* [PATCH] powerpc: msi: mark bitmap with kmemleak_not_leak()
@ 2015-09-13 18:36 Denis Kirjanov
2015-09-14 6:04 ` Michael Ellerman
0 siblings, 1 reply; 6+ messages in thread
From: Denis Kirjanov @ 2015-09-13 18:36 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Michael Ellerman, Catalin Marinas, Denis Kirjanov
During the MSI bitmap test on boot kmemleak spews the following trace:
unreferenced object 0xc00000016e86c900 (size 64):
comm "swapper/0", pid 1, jiffies 4294893173 (age 518.024s)
hex dump (first 32 bytes):
00 00 01 ff 7f ff 7f 37 00 00 00 00 00 00 00 00
.......7........
ff ff ff ff ff ff ff ff 01 ff ff ff ff ff ff ff
................
backtrace:
[<c00000000003eebc>] .zalloc_maybe_bootmem+0x3c/0x380
[<c000000000042d6c>] .msi_bitmap_alloc+0x3c/0xb0
[<c000000000a9aff8>]
.msi_bitmap_selftest+0x30/0x2b4
[<c0000000000090f4>]
.do_one_initcall+0xd4/0x270
[<c000000000a8e250>]
.kernel_init_freeable+0x1a0/0x280
[<c000000000009b5c>]
.kernel_init+0x1c/0x120
[<c000000000007fbc>]
.ret_from_kernel_thread+0x58/0x9c
The comment in msi_bitmap_free() states that we can't free
the bitmap so mark it with the kmemleak_not_leak().
Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
---
arch/powerpc/sysdev/msi_bitmap.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/sysdev/msi_bitmap.c b/arch/powerpc/sysdev/msi_bitmap.c
index 73b64c7..bd9fa54 100644
--- a/arch/powerpc/sysdev/msi_bitmap.c
+++ b/arch/powerpc/sysdev/msi_bitmap.c
@@ -139,6 +139,7 @@ int msi_bitmap_alloc(struct msi_bitmap *bmp, unsigned int irq_count,
void msi_bitmap_free(struct msi_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;
}
--
2.4.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc: msi: mark bitmap with kmemleak_not_leak()
2015-09-13 18:36 [PATCH] powerpc: msi: mark bitmap with kmemleak_not_leak() Denis Kirjanov
@ 2015-09-14 6:04 ` Michael Ellerman
2015-09-14 9:15 ` Catalin Marinas
0 siblings, 1 reply; 6+ messages in thread
From: Michael Ellerman @ 2015-09-14 6:04 UTC (permalink / raw)
To: Denis Kirjanov; +Cc: linuxppc-dev, Catalin Marinas
On Sun, 2015-09-13 at 21:36 +0300, Denis Kirjanov wrote:
> During the MSI bitmap test on boot kmemleak spews the following trace:
>
> unreferenced object 0xc00000016e86c900 (size 64):
> comm "swapper/0", pid 1, jiffies 4294893173 (age 518.024s)
> hex dump (first 32 bytes):
> 00 00 01 ff 7f ff 7f 37 00 00 00 00 00 00 00 00
> .......7........
> ff ff ff ff ff ff ff ff 01 ff ff ff ff ff ff ff
> ................
> backtrace:
> [<c00000000003eebc>] .zalloc_maybe_bootmem+0x3c/0x380
> [<c000000000042d6c>] .msi_bitmap_alloc+0x3c/0xb0
> [<c000000000a9aff8>]
> .msi_bitmap_selftest+0x30/0x2b4
> [<c0000000000090f4>]
> .do_one_initcall+0xd4/0x270
> [<c000000000a8e250>]
> .kernel_init_freeable+0x1a0/0x280
> [<c000000000009b5c>]
> .kernel_init+0x1c/0x120
> [<c000000000007fbc>]
> .ret_from_kernel_thread+0x58/0x9c
>
> 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?
cheers
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc: msi: mark bitmap with kmemleak_not_leak()
2015-09-14 6:04 ` Michael Ellerman
@ 2015-09-14 9:15 ` Catalin Marinas
2015-09-14 9:36 ` Michael Ellerman
0 siblings, 1 reply; 6+ messages in thread
From: Catalin Marinas @ 2015-09-14 9:15 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Denis Kirjanov, linuxppc-dev
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:
> > 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc: msi: mark bitmap with kmemleak_not_leak()
2015-09-14 9:15 ` Catalin Marinas
@ 2015-09-14 9:36 ` Michael Ellerman
2015-09-14 10:00 ` Catalin Marinas
0 siblings, 1 reply; 6+ messages in thread
From: Michael Ellerman @ 2015-09-14 9:36 UTC (permalink / raw)
To: Catalin Marinas; +Cc: Denis Kirjanov, linuxppc-dev
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:
> > On Sun, 2015-09-13 at 21:36 +0300, Denis Kirjanov wrote:
> > > 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc: msi: mark bitmap with kmemleak_not_leak()
2015-09-14 9:36 ` Michael Ellerman
@ 2015-09-14 10:00 ` Catalin Marinas
2015-09-14 10:42 ` Denis Kirjanov
0 siblings, 1 reply; 6+ messages in thread
From: Catalin Marinas @ 2015-09-14 10:00 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Denis Kirjanov, linuxppc-dev
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:
> > 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc: msi: mark bitmap with kmemleak_not_leak()
2015-09-14 10:00 ` Catalin Marinas
@ 2015-09-14 10:42 ` Denis Kirjanov
0 siblings, 0 replies; 6+ messages in thread
From: Denis Kirjanov @ 2015-09-14 10:42 UTC (permalink / raw)
To: Catalin Marinas; +Cc: Michael Ellerman, linuxppc-dev
On 9/14/15, Catalin Marinas <catalin.marinas@arm.com> wrote:
> 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:
>> > 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).
>
>> 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
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-09-14 10:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-13 18:36 [PATCH] powerpc: msi: mark bitmap with kmemleak_not_leak() Denis Kirjanov
2015-09-14 6:04 ` Michael Ellerman
2015-09-14 9:15 ` Catalin Marinas
2015-09-14 9:36 ` Michael Ellerman
2015-09-14 10:00 ` Catalin Marinas
2015-09-14 10:42 ` Denis Kirjanov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).