2018-03-16 15:41:57

by Takashi Iwai

Subject: [PATCH] brcmsmac: allocate ucode with GFP_KERNEL

The brcms_ucode_init_buf() duplicates the ucode chunks via kmemdup()
with GFP_ATOMIC as a precondition of wl->lock acquired. This caused
allocation failures sometimes as reported in the bugzilla below.

When looking at the the real usage, one can find that it's called
solely from brcms_request_fw(), and it's obviously outside the lock.
Hence we can use GFP_KERNEL there safely for avoiding such allocation
errors.

Bugzilla: http://bugzilla.suse.com/show_bug.cgi?id=1085174
Signed-off-by: Takashi Iwai <[email protected]>

---
drivers/net/wireless/broadcom/brcm80211/brcmsmac/mac80211_if.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/mac80211_if.c b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/mac80211_if.c
index ddfdfe177e24..a14fccf8a902 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/mac80211_if.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/mac80211_if.c
@@ -1563,7 +1563,7 @@ void brcms_free_timer(struct brcms_timer *t)
}

/*
- * precondition: perimeter lock has been acquired
+ * precondition: no locking required
*/
int brcms_ucode_init_buf(struct brcms_info *wl, void **pbuf, u32 idx)
{
@@ -1578,7 +1578,7 @@ int brcms_ucode_init_buf(struct brcms_info *wl, void **pbuf, u32 idx)
if (le32_to_cpu(hdr->idx) == idx) {
pdata = wl->fw.fw_bin[i]->data +
le32_to_cpu(hdr->offset);
- *pbuf = kmemdup(pdata, len, GFP_ATOMIC);
+ *pbuf = kmemdup(pdata, len, GFP_KERNEL);
if (*pbuf == NULL)
goto fail;

--
2.16.2

2018-03-27 09:09:38

by Kalle Valo

Subject: Re: brcmsmac: allocate ucode with GFP_KERNEL

Takashi Iwai <[email protected]> wrote:

> The brcms_ucode_init_buf() duplicates the ucode chunks via kmemdup()
> with GFP_ATOMIC as a precondition of wl->lock acquired. This caused
> allocation failures sometimes as reported in the bugzilla below.
>
> When looking at the the real usage, one can find that it's called
> solely from brcms_request_fw(), and it's obviously outside the lock.
> Hence we can use GFP_KERNEL there safely for avoiding such allocation
> errors.
>
> Bugzilla: http://bugzilla.suse.com/show_bug.cgi?id=1085174
> Signed-off-by: Takashi Iwai <[email protected]>
> Acked-by: Arend van Spriel <[email protected]>

Patch applied to wireless-drivers-next.git, thanks.

b1c2d0f2507b brcmsmac: allocate ucode with GFP_KERNEL

--
https://patchwork.kernel.org/patch/10288881/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2018-03-16 20:50:45

by Arend Van Spriel

Subject: Re: [PATCH] brcmsmac: allocate ucode with GFP_KERNEL

On 3/16/2018 4:41 PM, Takashi Iwai wrote:
> The brcms_ucode_init_buf() duplicates the ucode chunks via kmemdup()
> with GFP_ATOMIC as a precondition of wl->lock acquired. This caused
> allocation failures sometimes as reported in the bugzilla below.
>
> When looking at the the real usage, one can find that it's called
> solely from brcms_request_fw(), and it's obviously outside the lock.
> Hence we can use GFP_KERNEL there safely for avoiding such allocation
> errors.
>
> Bugzilla: http://bugzilla.suse.com/show_bug.cgi?id=1085174

Acked-by: Arend van Spriel <[email protected]>
> Signed-off-by: Takashi Iwai <[email protected]>
>
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmsmac/mac80211_if.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)