@@ -139,6 +139,7 @@ int msi_bitmap_alloc(struct msi_bitmap *bmp, unsigned int irq_count,voidmsi_bitmap_free(structmsi_bitmap*bmp){/* we can't free the bitmap we don't know if it's bootmem etc. */+kmemleak_not_leak(bmp->bitmap);of_node_put(bmp->of_node);bmp->bitmap=NULL;}
On Mon, Sep 14, 2015 at 04:04:37PM +1000, Michael Ellerman wrote:
On Sun, 2015-09-13 at 21:36 +0300, Denis Kirjanov wrote:
quoted
During the MSI bitmap test on boot kmemleak spews the following trace:
=20
unreferenced object 0xc00000016e86c900 (size 64):
comm "swapper/0", pid 1, jiffies 4294893173 (age 518.024s)
hex dump (first 32 bytes):
=0900 00 01 ff 7f ff 7f 37 00 00 00 00 00 00 00 00
=09.......7........
=09ff ff ff ff ff ff ff ff 01 ff ff ff ff ff ff ff
=09................
=09backtrace:
=09[<c00000000003eebc>] .zalloc_maybe_bootmem+0x3c/0x380
=09[<c000000000042d6c>] .msi_bitmap_alloc+0x3c/0xb0
=09[<c000000000a9aff8>]
=09.msi_bitmap_selftest+0x30/0x2b4
=09[<c0000000000090f4>]
=09.do_one_initcall+0xd4/0x270
=09[<c000000000a8e250>]
=09.kernel_init_freeable+0x1a0/0x280
=09[<c000000000009b5c>]
=09.kernel_init+0x1c/0x120
=09[<c000000000007fbc>]
=09.ret_from_kernel_thread+0x58/0x9c
=20
The comment in msi_bitmap_free() states that we can't free
the bitmap so mark it with the kmemleak_not_leak().
=20
Yeah, and I've always hated that comment :)
=20
I assume it's still true now that we don't use bootmem?
If it's not bootmem, it's probably memblock. Looking at the code, it
seems that msi_bitmap_free() cannot make the distinction between a slab
allocation and a bootmem/memblock one (allocated via
zalloc_maybe_bootmem).
You could add some flag to struct msi_bitmap based on mem_init_done to
be able to reclaim some slab memory later. If the bitmap is small and
such allocation doesn't happen outside boot, it may not be worth the
effort.
--=20
Catalin
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2015-09-14 09:36:50
On Mon, 2015-09-14 at 10:15 +0100, Catalin Marinas wrote:
On Mon, Sep 14, 2015 at 04:04:37PM +1000, Michael Ellerman wrote:
quoted
On Sun, 2015-09-13 at 21:36 +0300, Denis Kirjanov wrote:
quoted
The comment in msi_bitmap_free() states that we can't free
the bitmap so mark it with the kmemleak_not_leak().
Yeah, and I've always hated that comment :)
I assume it's still true now that we don't use bootmem?
If it's not bootmem, it's probably memblock. Looking at the code, it
seems that msi_bitmap_free() cannot make the distinction between a slab
allocation and a bootmem/memblock one (allocated via
zalloc_maybe_bootmem).
Yeah it is memblock these days.
You could add some flag to struct msi_bitmap based on mem_init_done to
be able to reclaim some slab memory later. If the bitmap is small and
such allocation doesn't happen outside boot, it may not be worth the
effort.
Right, I think that's the only solution, and it's not quite worth the trouble
because it's generally not freed at all, except via error paths.
Still I think it would be better to move the kmemleak annotation into the msi
bitmap test code, or maybe a wrapper that's called by the test code.
That way if someone starts calling alloc/free from a hotplug path for example,
kmemleak will still notice that free isn't really freeing.
cheers
On Mon, Sep 14, 2015 at 07:36:49PM +1000, Michael Ellerman wrote:
On Mon, 2015-09-14 at 10:15 +0100, Catalin Marinas wrote:
quoted
You could add some flag to struct msi_bitmap based on mem_init_done to
be able to reclaim some slab memory later. If the bitmap is small and
such allocation doesn't happen outside boot, it may not be worth the
effort.
=20
Right, I think that's the only solution, and it's not quite worth the tro=
uble
because it's generally not freed at all, except via error paths.
=20
Still I think it would be better to move the kmemleak annotation into the=
msi
bitmap test code, or maybe a wrapper that's called by the test code.
Other kmemleak annotations throughout the kernel are usually added
immediately after the allocation place (since that's what kmemleak
reports as a leak).
That way if someone starts calling alloc/free from a hotplug path for exa=
mple,
kmemleak will still notice that free isn't really freeing.
Kmemleak would only notice the moment you clear the last reference to
the allocated memory block (like bmp->bitmap =3D NULL), so this patch
should work as along as "freeing" is done via msi_bitmap_free().
BTW, you can even use kmemleak_ignore(). The difference is that the
bitmap won't be scanned by kmemleak and that's fine since it doesn't
contain any pointers.
--=20
Catalin
On Mon, Sep 14, 2015 at 07:36:49PM +1000, Michael Ellerman wrote:
quoted
On Mon, 2015-09-14 at 10:15 +0100, Catalin Marinas wrote:
quoted
You could add some flag to struct msi_bitmap based on mem_init_done to
be able to reclaim some slab memory later. If the bitmap is small and
such allocation doesn't happen outside boot, it may not be worth the
effort.
Right, I think that's the only solution, and it's not quite worth the
trouble
because it's generally not freed at all, except via error paths.
Still I think it would be better to move the kmemleak annotation into the
msi
bitmap test code, or maybe a wrapper that's called by the test code.
Other kmemleak annotations throughout the kernel are usually added
immediately after the allocation place (since that's what kmemleak
reports as a leak).
quoted
That way if someone starts calling alloc/free from a hotplug path for
example,
kmemleak will still notice that free isn't really freeing.
Kmemleak would only notice the moment you clear the last reference to
the allocated memory block (like bmp->bitmap = NULL), so this patch
should work as along as "freeing" is done via msi_bitmap_free().
Ok, I'll move kmemleak annotaion for the bitmap inside the test_basics()
plus add a flag into msi_bitmap to test for the type of allocation
though the slab_is_available()
Thank you.
BTW, you can even use kmemleak_ignore(). The difference is that the
bitmap won't be scanned by kmemleak and that's fine since it doesn't
contain any pointers.
--
Catalin