linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* brcmfmac: Possible memleak brcmf_sdiod_sgtable_alloc
@ 2017-07-22 13:18 Stefan Wahren
  2017-07-22 19:40 ` Arend van Spriel
  0 siblings, 1 reply; 5+ messages in thread
From: Stefan Wahren @ 2017-07-22 13:18 UTC (permalink / raw)
  To: arend.vanspriel, hante.meuleman, pieter-paul.giesberts,
	franky.lin, kvalo
  Cc: brcm80211-dev-list.pdl, linux-wireless

Hi,

with enabled memleak detector on 4.13-rc1 (Raspberry Pi Zero W) i get the following:

root@raspberrypi:/sys/kernel/debug# cat kmemleak
unreferenced object 0xd824e400 (size 1024):
  comm "kworker/0:0", pid 3, jiffies 4294939822 (age 873.420s)
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<c07f741c>] kmemleak_alloc+0x60/0xc8
    [<c024cb44>] __kmalloc+0x184/0x2f4
    [<c03dc1c8>] sg_kmalloc+0x48/0x4c
    [<c03dc0dc>] __sg_alloc_table+0x78/0x11c
    [<c03dc5d0>] sg_alloc_table+0x2c/0x5c
    [<bf07d7e8>] brcmf_sdiod_sgtable_alloc+0xc0/0x110 [brcmfmac]
    [<bf07b814>] brcmf_sdio_probe+0x24c/0x970 [brcmfmac]
    [<bf07c6fc>] brcmf_ops_sdio_probe+0x17c/0x244 [brcmfmac]
    [<c057d5b4>] sdio_bus_probe+0xb4/0x124
    [<c04add08>] driver_probe_device+0x1d8/0x438
    [<c04ae00c>] __driver_attach+0xa4/0x108
    [<c04abe20>] bus_for_each_dev+0x84/0x98
    [<c04ad644>] driver_attach+0x28/0x30
    [<c04ad084>] bus_add_driver+0xe4/0x24c
    [<c04ae834>] driver_register+0xac/0xf0
    [<c057d7dc>] sdio_register_driver+0x2c/0x34

Regards
Stefan

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: brcmfmac: Possible memleak brcmf_sdiod_sgtable_alloc
  2017-07-22 13:18 brcmfmac: Possible memleak brcmf_sdiod_sgtable_alloc Stefan Wahren
@ 2017-07-22 19:40 ` Arend van Spriel
  2017-07-23  0:24   ` Stefan Wahren
  0 siblings, 1 reply; 5+ messages in thread
From: Arend van Spriel @ 2017-07-22 19:40 UTC (permalink / raw)
  To: Stefan Wahren, hante.meuleman, pieter-paul.giesberts, franky.lin,
	kvalo
  Cc: brcm80211-dev-list.pdl, linux-wireless

On 22-07-17 15:18, Stefan Wahren wrote:
> Hi,
> 
> with enabled memleak detector on 4.13-rc1 (Raspberry Pi Zero W) i get the following:
> 
> root@raspberrypi:/sys/kernel/debug# cat kmemleak
> unreferenced object 0xd824e400 (size 1024):
>   comm "kworker/0:0", pid 3, jiffies 4294939822 (age 873.420s)
>   hex dump (first 32 bytes):
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<c07f741c>] kmemleak_alloc+0x60/0xc8
>     [<c024cb44>] __kmalloc+0x184/0x2f4
>     [<c03dc1c8>] sg_kmalloc+0x48/0x4c
>     [<c03dc0dc>] __sg_alloc_table+0x78/0x11c
>     [<c03dc5d0>] sg_alloc_table+0x2c/0x5c
>     [<bf07d7e8>] brcmf_sdiod_sgtable_alloc+0xc0/0x110 [brcmfmac]
>     [<bf07b814>] brcmf_sdio_probe+0x24c/0x970 [brcmfmac]
>     [<bf07c6fc>] brcmf_ops_sdio_probe+0x17c/0x244 [brcmfmac]
>     [<c057d5b4>] sdio_bus_probe+0xb4/0x124
>     [<c04add08>] driver_probe_device+0x1d8/0x438
>     [<c04ae00c>] __driver_attach+0xa4/0x108
>     [<c04abe20>] bus_for_each_dev+0x84/0x98
>     [<c04ad644>] driver_attach+0x28/0x30
>     [<c04ad084>] bus_add_driver+0xe4/0x24c
>     [<c04ae834>] driver_register+0xac/0xf0
>     [<c057d7dc>] sdio_register_driver+0x2c/0x34

Hi Stefan,

Thanks for the report. Checking elixir it shows two call sites to
brcmf_sdiod_sgtable_alloc() [1]. This is rather unexpected. We did move
the call so this might be a merge issue.

4d7928959 sdio.c (Hante Meuleman   2016-02-17 11:27:07 +0100:
3867)     brcmf_sdiod_sgtable_alloc(sdiodev);

e0045bf80 sdio.c (Hante Meuleman   2016-01-19 12:39:24 +0100:
4180)     brcmf_sdiod_sgtable_alloc(bus->sdiodev);

The most recent change is:

commit 4d7928959832 ("brcmfmac: switch to new platform data") found in
patchwork [2], which shows the added call, but no removal so the merge
issue is probably internal with us (me :-( ).

Again thanks for the report. Below change should fix the issue.

Regards,
Arend

[1]
http://elixir.free-electrons.com/linux/latest/ident/brcmf_sdiod_sgtable_alloc
[2] https://patchwork.kernel.org/patch/8336231/

---
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index fbcbb43..ed2c693 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -4174,11 +4174,6 @@ struct brcmf_sdio *brcmf_sdio_probe(struct
brcmf_sdio_dev *sdiodev)
                goto fail;
        }

-       /* allocate scatter-gather table. sg support
-        * will be disabled upon allocation failure.
-        */
-       brcmf_sdiod_sgtable_alloc(bus->sdiodev);
-
        /* Query the F2 block size, set roundup accordingly */
        bus->blocksize = bus->sdiodev->func[2]->cur_blksize;
        bus->roundup = min(max_roundup, bus->blocksize);

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: brcmfmac: Possible memleak brcmf_sdiod_sgtable_alloc
  2017-07-22 19:40 ` Arend van Spriel
@ 2017-07-23  0:24   ` Stefan Wahren
  2017-07-26  6:12     ` Stefan Wahren
  0 siblings, 1 reply; 5+ messages in thread
From: Stefan Wahren @ 2017-07-23  0:24 UTC (permalink / raw)
  To: franky.lin, pieter-paul.giesberts, kvalo, Arend van Spriel,
	hante.meuleman
  Cc: brcm80211-dev-list.pdl, linux-wireless


> Arend van Spriel <arend.vanspriel@broadcom.com> hat am 22. Juli 2017 um 21:40 geschrieben:
> 
> 
> On 22-07-17 15:18, Stefan Wahren wrote:
> > Hi,
> > 
> > with enabled memleak detector on 4.13-rc1 (Raspberry Pi Zero W) i get the following:
> > 
> > root@raspberrypi:/sys/kernel/debug# cat kmemleak
> > unreferenced object 0xd824e400 (size 1024):
> >   comm "kworker/0:0", pid 3, jiffies 4294939822 (age 873.420s)
> >   hex dump (first 32 bytes):
> >     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> >     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> >   backtrace:
> >     [<c07f741c>] kmemleak_alloc+0x60/0xc8
> >     [<c024cb44>] __kmalloc+0x184/0x2f4
> >     [<c03dc1c8>] sg_kmalloc+0x48/0x4c
> >     [<c03dc0dc>] __sg_alloc_table+0x78/0x11c
> >     [<c03dc5d0>] sg_alloc_table+0x2c/0x5c
> >     [<bf07d7e8>] brcmf_sdiod_sgtable_alloc+0xc0/0x110 [brcmfmac]
> >     [<bf07b814>] brcmf_sdio_probe+0x24c/0x970 [brcmfmac]
> >     [<bf07c6fc>] brcmf_ops_sdio_probe+0x17c/0x244 [brcmfmac]
> >     [<c057d5b4>] sdio_bus_probe+0xb4/0x124
> >     [<c04add08>] driver_probe_device+0x1d8/0x438
> >     [<c04ae00c>] __driver_attach+0xa4/0x108
> >     [<c04abe20>] bus_for_each_dev+0x84/0x98
> >     [<c04ad644>] driver_attach+0x28/0x30
> >     [<c04ad084>] bus_add_driver+0xe4/0x24c
> >     [<c04ae834>] driver_register+0xac/0xf0
> >     [<c057d7dc>] sdio_register_driver+0x2c/0x34
> 
> Hi Stefan,
> 
> Thanks for the report. Checking elixir it shows two call sites to
> brcmf_sdiod_sgtable_alloc() [1]. This is rather unexpected. We did move
> the call so this might be a merge issue.
> 
> 4d7928959 sdio.c (Hante Meuleman   2016-02-17 11:27:07 +0100:
> 3867)     brcmf_sdiod_sgtable_alloc(sdiodev);
> 
> e0045bf80 sdio.c (Hante Meuleman   2016-01-19 12:39:24 +0100:
> 4180)     brcmf_sdiod_sgtable_alloc(bus->sdiodev);
> 
> The most recent change is:
> 
> commit 4d7928959832 ("brcmfmac: switch to new platform data") found in
> patchwork [2], which shows the added call, but no removal so the merge
> issue is probably internal with us (me :-( ).
> 
> Again thanks for the report. Below change should fix the issue.

Yes, this fixed the leak. Thanks for you fast reply.

> 
> Regards,
> Arend
> 
> [1]
> http://elixir.free-electrons.com/linux/latest/ident/brcmf_sdiod_sgtable_alloc
> [2] https://patchwork.kernel.org/patch/8336231/
> 
> ---
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> index fbcbb43..ed2c693 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> @@ -4174,11 +4174,6 @@ struct brcmf_sdio *brcmf_sdio_probe(struct
> brcmf_sdio_dev *sdiodev)
>                 goto fail;
>         }
> 
> -       /* allocate scatter-gather table. sg support
> -        * will be disabled upon allocation failure.
> -        */
> -       brcmf_sdiod_sgtable_alloc(bus->sdiodev);
> -
>         /* Query the F2 block size, set roundup accordingly */
>         bus->blocksize = bus->sdiodev->func[2]->cur_blksize;
>         bus->roundup = min(max_roundup, bus->blocksize);

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: brcmfmac: Possible memleak brcmf_sdiod_sgtable_alloc
  2017-07-23  0:24   ` Stefan Wahren
@ 2017-07-26  6:12     ` Stefan Wahren
  2017-07-26  7:59       ` Arend van Spriel
  0 siblings, 1 reply; 5+ messages in thread
From: Stefan Wahren @ 2017-07-26  6:12 UTC (permalink / raw)
  To: franky.lin, pieter-paul.giesberts, kvalo, Arend van Spriel,
	hante.meuleman
  Cc: brcm80211-dev-list.pdl, linux-wireless

Hi Arend,

> Stefan Wahren <stefan.wahren@i2se.com> hat am 23. Juli 2017 um 02:24 geschrieben:
> 
> 
> 
> > Arend van Spriel <arend.vanspriel@broadcom.com> hat am 22. Juli 2017 um 21:40 geschrieben:
> > 
> > 
> > On 22-07-17 15:18, Stefan Wahren wrote:
> > > Hi,
> > > 
> > > with enabled memleak detector on 4.13-rc1 (Raspberry Pi Zero W) i get the following:
> > > 
> > > root@raspberrypi:/sys/kernel/debug# cat kmemleak
> > > unreferenced object 0xd824e400 (size 1024):
> > >   comm "kworker/0:0", pid 3, jiffies 4294939822 (age 873.420s)
> > >   hex dump (first 32 bytes):
> > >     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > >     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > >   backtrace:
> > >     [<c07f741c>] kmemleak_alloc+0x60/0xc8
> > >     [<c024cb44>] __kmalloc+0x184/0x2f4
> > >     [<c03dc1c8>] sg_kmalloc+0x48/0x4c
> > >     [<c03dc0dc>] __sg_alloc_table+0x78/0x11c
> > >     [<c03dc5d0>] sg_alloc_table+0x2c/0x5c
> > >     [<bf07d7e8>] brcmf_sdiod_sgtable_alloc+0xc0/0x110 [brcmfmac]
> > >     [<bf07b814>] brcmf_sdio_probe+0x24c/0x970 [brcmfmac]
> > >     [<bf07c6fc>] brcmf_ops_sdio_probe+0x17c/0x244 [brcmfmac]
> > >     [<c057d5b4>] sdio_bus_probe+0xb4/0x124
> > >     [<c04add08>] driver_probe_device+0x1d8/0x438
> > >     [<c04ae00c>] __driver_attach+0xa4/0x108
> > >     [<c04abe20>] bus_for_each_dev+0x84/0x98
> > >     [<c04ad644>] driver_attach+0x28/0x30
> > >     [<c04ad084>] bus_add_driver+0xe4/0x24c
> > >     [<c04ae834>] driver_register+0xac/0xf0
> > >     [<c057d7dc>] sdio_register_driver+0x2c/0x34
> > 
> > Hi Stefan,
> > 
> > Thanks for the report. Checking elixir it shows two call sites to
> > brcmf_sdiod_sgtable_alloc() [1]. This is rather unexpected. We did move
> > the call so this might be a merge issue.
> > 
> > 4d7928959 sdio.c (Hante Meuleman   2016-02-17 11:27:07 +0100:
> > 3867)     brcmf_sdiod_sgtable_alloc(sdiodev);
> > 
> > e0045bf80 sdio.c (Hante Meuleman   2016-01-19 12:39:24 +0100:
> > 4180)     brcmf_sdiod_sgtable_alloc(bus->sdiodev);
> > 
> > The most recent change is:
> > 
> > commit 4d7928959832 ("brcmfmac: switch to new platform data") found in
> > patchwork [2], which shows the added call, but no removal so the merge
> > issue is probably internal with us (me :-( ).
> > 
> > Again thanks for the report. Below change should fix the issue.
> 
> Yes, this fixed the leak. Thanks for you fast reply.

do you plan to send a proper patch for this?

> 
> > 
> > Regards,
> > Arend
> > 
> > [1]
> > http://elixir.free-electrons.com/linux/latest/ident/brcmf_sdiod_sgtable_alloc
> > [2] https://patchwork.kernel.org/patch/8336231/
> > 
> > ---
> > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> > b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> > index fbcbb43..ed2c693 100644
> > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> > @@ -4174,11 +4174,6 @@ struct brcmf_sdio *brcmf_sdio_probe(struct
> > brcmf_sdio_dev *sdiodev)
> >                 goto fail;
> >         }
> > 
> > -       /* allocate scatter-gather table. sg support
> > -        * will be disabled upon allocation failure.
> > -        */
> > -       brcmf_sdiod_sgtable_alloc(bus->sdiodev);
> > -
> >         /* Query the F2 block size, set roundup accordingly */
> >         bus->blocksize = bus->sdiodev->func[2]->cur_blksize;
> >         bus->roundup = min(max_roundup, bus->blocksize);

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: brcmfmac: Possible memleak brcmf_sdiod_sgtable_alloc
  2017-07-26  6:12     ` Stefan Wahren
@ 2017-07-26  7:59       ` Arend van Spriel
  0 siblings, 0 replies; 5+ messages in thread
From: Arend van Spriel @ 2017-07-26  7:59 UTC (permalink / raw)
  To: Stefan Wahren
  Cc: franky.lin, pieter-paul.giesberts, kvalo, hante.meuleman,
	brcm80211-dev-list.pdl, linux-wireless

On 7/26/2017 8:12 AM, Stefan Wahren wrote:
> Hi Arend,
>
>> Stefan Wahren <stefan.wahren@i2se.com> hat am 23. Juli 2017 um 02:24 geschrieben:
>>
>>
>>
>>> Arend van Spriel <arend.vanspriel@broadcom.com> hat am 22. Juli 2017 um 21:40 geschrieben:
>>>
>>>
>>> On 22-07-17 15:18, Stefan Wahren wrote:
>>>> Hi,
>>>>
>>>> with enabled memleak detector on 4.13-rc1 (Raspberry Pi Zero W) i get the following:
>>>>
>>>> root@raspberrypi:/sys/kernel/debug# cat kmemleak
>>>> unreferenced object 0xd824e400 (size 1024):
>>>>    comm "kworker/0:0", pid 3, jiffies 4294939822 (age 873.420s)
>>>>    hex dump (first 32 bytes):
>>>>      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>>>>      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>>>>    backtrace:
>>>>      [<c07f741c>] kmemleak_alloc+0x60/0xc8
>>>>      [<c024cb44>] __kmalloc+0x184/0x2f4
>>>>      [<c03dc1c8>] sg_kmalloc+0x48/0x4c
>>>>      [<c03dc0dc>] __sg_alloc_table+0x78/0x11c
>>>>      [<c03dc5d0>] sg_alloc_table+0x2c/0x5c
>>>>      [<bf07d7e8>] brcmf_sdiod_sgtable_alloc+0xc0/0x110 [brcmfmac]
>>>>      [<bf07b814>] brcmf_sdio_probe+0x24c/0x970 [brcmfmac]
>>>>      [<bf07c6fc>] brcmf_ops_sdio_probe+0x17c/0x244 [brcmfmac]
>>>>      [<c057d5b4>] sdio_bus_probe+0xb4/0x124
>>>>      [<c04add08>] driver_probe_device+0x1d8/0x438
>>>>      [<c04ae00c>] __driver_attach+0xa4/0x108
>>>>      [<c04abe20>] bus_for_each_dev+0x84/0x98
>>>>      [<c04ad644>] driver_attach+0x28/0x30
>>>>      [<c04ad084>] bus_add_driver+0xe4/0x24c
>>>>      [<c04ae834>] driver_register+0xac/0xf0
>>>>      [<c057d7dc>] sdio_register_driver+0x2c/0x34
>>>
>>> Hi Stefan,
>>>
>>> Thanks for the report. Checking elixir it shows two call sites to
>>> brcmf_sdiod_sgtable_alloc() [1]. This is rather unexpected. We did move
>>> the call so this might be a merge issue.
>>>
>>> 4d7928959 sdio.c (Hante Meuleman   2016-02-17 11:27:07 +0100:
>>> 3867)     brcmf_sdiod_sgtable_alloc(sdiodev);
>>>
>>> e0045bf80 sdio.c (Hante Meuleman   2016-01-19 12:39:24 +0100:
>>> 4180)     brcmf_sdiod_sgtable_alloc(bus->sdiodev);
>>>
>>> The most recent change is:
>>>
>>> commit 4d7928959832 ("brcmfmac: switch to new platform data") found in
>>> patchwork [2], which shows the added call, but no removal so the merge
>>> issue is probably internal with us (me :-( ).
>>>
>>> Again thanks for the report. Below change should fix the issue.
>>
>> Yes, this fixed the leak. Thanks for you fast reply.
>
> do you plan to send a proper patch for this?

Sure. Catching up after my vacation.

Regards,
Arend

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-07-26  7:59 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-22 13:18 brcmfmac: Possible memleak brcmf_sdiod_sgtable_alloc Stefan Wahren
2017-07-22 19:40 ` Arend van Spriel
2017-07-23  0:24   ` Stefan Wahren
2017-07-26  6:12     ` Stefan Wahren
2017-07-26  7:59       ` Arend van Spriel

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).