All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ath6kl: Use vmalloc to allocate ar->fw for api1 method
@ 2015-12-01  5:41 ` Brent Taylor
  0 siblings, 0 replies; 12+ messages in thread
From: Brent Taylor @ 2015-12-01  5:41 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, netdev, linux-kernel, ath6kl, Brent Taylor

Since commit 8437754c83351d6213c1a47ff029c1126d6042a7, ar->fw is expected to be pointing to memory allocated by vmalloc.  If the api1 method (via ath6kl_fetch_fw_api1) is used to allocate memory for ar->fw, then kmemdup is used.  This patch checks if the firmware being loaded is the 'fw' image, then use vmalloc, otherwise use kmalloc.

Signed-off-by: Brent Taylor <motobud@gmail.com>
---
 drivers/net/wireless/ath/ath6kl/init.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath6kl/init.c b/drivers/net/wireless/ath/ath6kl/init.c
index 6ae0734..4f2b124d 100644
--- a/drivers/net/wireless/ath/ath6kl/init.c
+++ b/drivers/net/wireless/ath/ath6kl/init.c
@@ -673,10 +673,15 @@ static int ath6kl_get_fw(struct ath6kl *ar, const char *filename,
 		return ret;
 
 	*fw_len = fw_entry->size;
-	*fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL);
+   if (&ar->fw == fw)
+		*fw = vmalloc(fw_entry->size);
+   else
+		*fw = kmalloc(fw_entry->size, GFP_KERNEL);
 
 	if (*fw == NULL)
 		ret = -ENOMEM;
+	else
+		memcpy(*fw, fw_entry->data, fw_entry->size);
 
 	release_firmware(fw_entry);
 
-- 
2.6.3


^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH] ath6kl: Use vmalloc to allocate ar->fw for api1 method
@ 2015-12-01  5:41 ` Brent Taylor
  0 siblings, 0 replies; 12+ messages in thread
From: Brent Taylor @ 2015-12-01  5:41 UTC (permalink / raw)
  To: kvalo-A+ZNKFmMK5xy9aJCnZT0Uw
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	ath6kl-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Brent Taylor

Since commit 8437754c83351d6213c1a47ff029c1126d6042a7, ar->fw is expected to be pointing to memory allocated by vmalloc.  If the api1 method (via ath6kl_fetch_fw_api1) is used to allocate memory for ar->fw, then kmemdup is used.  This patch checks if the firmware being loaded is the 'fw' image, then use vmalloc, otherwise use kmalloc.

Signed-off-by: Brent Taylor <motobud-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/net/wireless/ath/ath6kl/init.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath6kl/init.c b/drivers/net/wireless/ath/ath6kl/init.c
index 6ae0734..4f2b124d 100644
--- a/drivers/net/wireless/ath/ath6kl/init.c
+++ b/drivers/net/wireless/ath/ath6kl/init.c
@@ -673,10 +673,15 @@ static int ath6kl_get_fw(struct ath6kl *ar, const char *filename,
 		return ret;
 
 	*fw_len = fw_entry->size;
-	*fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL);
+   if (&ar->fw == fw)
+		*fw = vmalloc(fw_entry->size);
+   else
+		*fw = kmalloc(fw_entry->size, GFP_KERNEL);
 
 	if (*fw == NULL)
 		ret = -ENOMEM;
+	else
+		memcpy(*fw, fw_entry->data, fw_entry->size);
 
 	release_firmware(fw_entry);
 
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* Re: [PATCH] ath6kl: Use vmalloc to allocate ar->fw for api1 method
  2015-12-01  5:41 ` Brent Taylor
  (?)
@ 2015-12-01 13:08 ` Sergei Shtylyov
  -1 siblings, 0 replies; 12+ messages in thread
From: Sergei Shtylyov @ 2015-12-01 13:08 UTC (permalink / raw)
  To: Brent Taylor, kvalo; +Cc: linux-wireless, netdev, linux-kernel, ath6kl

Hello.

On 12/1/2015 8:41 AM, Brent Taylor wrote:

> Since commit 8437754c83351d6213c1a47ff029c1126d6042a7,

    The scripts/checkpatch.pl now enforces certain format of the commit citing.

> ar->fw is expected to be pointing to memory allocated by vmalloc.  If the api1 method (via ath6kl_fetch_fw_api1) is used to allocate memory for ar->fw, then kmemdup is used.  This patch checks if the firmware being loaded is the 'fw' image, then use vmalloc, otherwise use kmalloc.

    This script also checks for unwrapped changelogs. Please run your patches 
thru it before posting.

> Signed-off-by: Brent Taylor <motobud@gmail.com>
[...]

MBR, Sergei


^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH] ath6kl: Use vmalloc to allocate ar->fw for api1 method
  2015-12-01  5:41 ` Brent Taylor
  (?)
  (?)
@ 2015-12-13  0:52 ` Andy Shevchenko
  -1 siblings, 0 replies; 12+ messages in thread
From: Andy Shevchenko @ 2015-12-13  0:52 UTC (permalink / raw)
  To: Brent Taylor
  Cc: Kalle Valo, open list:TI WILINK WIRELES..., netdev, linux-kernel, ath6kl

On Tue, Dec 1, 2015 at 7:41 AM, Brent Taylor <motobud@gmail.com> wrote:
> Since commit 8437754c83351d6213c1a47ff029c1126d6042a7, ar->fw is expected to be pointing to memory allocated by vmalloc.  If the api1 method (via ath6kl_fetch_fw_api1) is used to allocate memory for ar->fw, then kmemdup is used.  This patch checks if the firmware being loaded is the 'fw' image, then use vmalloc, otherwise use kmalloc.
>

I think I already told someone that this kind of fixes miss the
symmetric kvfree() calls. Please, fix.


> Signed-off-by: Brent Taylor <motobud@gmail.com>
> ---
>  drivers/net/wireless/ath/ath6kl/init.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath6kl/init.c b/drivers/net/wireless/ath/ath6kl/init.c
> index 6ae0734..4f2b124d 100644
> --- a/drivers/net/wireless/ath/ath6kl/init.c
> +++ b/drivers/net/wireless/ath/ath6kl/init.c
> @@ -673,10 +673,15 @@ static int ath6kl_get_fw(struct ath6kl *ar, const char *filename,
>                 return ret;
>
>         *fw_len = fw_entry->size;
> -       *fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL);
> +   if (&ar->fw == fw)
> +               *fw = vmalloc(fw_entry->size);
> +   else
> +               *fw = kmalloc(fw_entry->size, GFP_KERNEL);
>
>         if (*fw == NULL)
>                 ret = -ENOMEM;
> +       else
> +               memcpy(*fw, fw_entry->data, fw_entry->size);
>
>         release_firmware(fw_entry);
>
> --
> 2.6.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH] ath6kl: Use vmalloc to allocate ar->fw for api1 method
  2015-12-01  5:41 ` Brent Taylor
                   ` (2 preceding siblings ...)
  (?)
@ 2015-12-21 19:23 ` Souptick Joarder
  2015-12-21 21:53   ` Brent Taylor
  2015-12-21 22:00     ` Dan Kephart
  -1 siblings, 2 replies; 12+ messages in thread
From: Souptick Joarder @ 2015-12-21 19:23 UTC (permalink / raw)
  To: Brent Taylor; +Cc: Kalle Valo, linux-wireless, netdev, linux-kernel, ath6kl

Hi Brent,

On Tue, Dec 1, 2015 at 11:11 AM, Brent Taylor <motobud@gmail.com> wrote:
> Since commit 8437754c83351d6213c1a47ff029c1126d6042a7, ar->fw is expected to be pointing to memory allocated by vmalloc.  If the api1 method (via ath6kl_fetch_fw_api1) is used to allocate memory for ar->fw, then kmemdup is used.  This patch checks if the firmware being loaded is the 'fw' image, then use vmalloc, otherwise use kmalloc.
>
> Signed-off-by: Brent Taylor <motobud@gmail.com>
> ---
>  drivers/net/wireless/ath/ath6kl/init.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath6kl/init.c b/drivers/net/wireless/ath/ath6kl/init.c
> index 6ae0734..4f2b124d 100644
> --- a/drivers/net/wireless/ath/ath6kl/init.c
> +++ b/drivers/net/wireless/ath/ath6kl/init.c
> @@ -673,10 +673,15 @@ static int ath6kl_get_fw(struct ath6kl *ar, const char *filename,
>                 return ret;
>
>         *fw_len = fw_entry->size;
> -       *fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL);
> +   if (&ar->fw == fw)
> +               *fw = vmalloc(fw_entry->size);
> +   else
> +               *fw = kmalloc(fw_entry->size, GFP_KERNEL);

          Why vmalloc and kmalloc both are required? can't use either
vmalloc or kmalloc?
>
>         if (*fw == NULL)
>                 ret = -ENOMEM;
> +       else
> +               memcpy(*fw, fw_entry->data, fw_entry->size);
>
>         release_firmware(fw_entry);
>
> --
> 2.6.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-Souptick

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH] ath6kl: Use vmalloc to allocate ar->fw for api1 method
  2015-12-21 19:23 ` Souptick Joarder
@ 2015-12-21 21:53   ` Brent Taylor
  2015-12-22  9:05     ` Souptick Joarder
  2015-12-21 22:00     ` Dan Kephart
  1 sibling, 1 reply; 12+ messages in thread
From: Brent Taylor @ 2015-12-21 21:53 UTC (permalink / raw)
  To: Souptick Joarder; +Cc: Kalle Valo, linux-wireless, netdev, linux-kernel, ath6kl

On Mon, Dec 21, 2015 at 1:23 PM, Souptick Joarder <jrdr.linux@gmail.com> wrote:
> Hi Brent,
>
> On Tue, Dec 1, 2015 at 11:11 AM, Brent Taylor <motobud@gmail.com> wrote:
>> Since commit 8437754c83351d6213c1a47ff029c1126d6042a7, ar->fw is expected to be pointing to memory allocated by vmalloc.  If the api1 method (via ath6kl_fetch_fw_api1) is used to allocate memory for ar->fw, then kmemdup is used.  This patch checks if the firmware being loaded is the 'fw' image, then use vmalloc, otherwise use kmalloc.
>>
>> Signed-off-by: Brent Taylor <motobud@gmail.com>
>> ---
>>  drivers/net/wireless/ath/ath6kl/init.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath6kl/init.c b/drivers/net/wireless/ath/ath6kl/init.c
>> index 6ae0734..4f2b124d 100644
>> --- a/drivers/net/wireless/ath/ath6kl/init.c
>> +++ b/drivers/net/wireless/ath/ath6kl/init.c
>> @@ -673,10 +673,15 @@ static int ath6kl_get_fw(struct ath6kl *ar, const char *filename,
>>                 return ret;
>>
>>         *fw_len = fw_entry->size;
>> -       *fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL);
>> +   if (&ar->fw == fw)
>> +               *fw = vmalloc(fw_entry->size);
>> +   else
>> +               *fw = kmalloc(fw_entry->size, GFP_KERNEL);
>
>           Why vmalloc and kmalloc both are required? can't use either
> vmalloc or kmalloc?

My original problem was that kmemdup (which uses kmalloc) could not
allocate enough memory
to hold the firmware that is placed into "ar->fw".  In the function
ath6kl_core_cleanup (in core.c),
the "ar->fw" pointer is the only one that uses vfree which was changed in commit
8437754c83351d6213c1a47ff029c1126d6042a7.

I was trying to change as little as possible and I wasn't sure if
there was a reason that any of the
other firmware items needed to be allocated with kmalloc or if they
could be changed to use vmalloc.

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH] ath6kl: Use vmalloc to allocate ar->fw for api1 method
  2015-12-21 19:23 ` Souptick Joarder
  2015-12-21 21:53   ` Brent Taylor
@ 2015-12-21 22:00     ` Dan Kephart
  1 sibling, 0 replies; 12+ messages in thread
From: Dan Kephart @ 2015-12-21 22:00 UTC (permalink / raw)
  To: Souptick Joarder, Brent Taylor
  Cc: netdev, Kalle Valo, ath6kl, linux-wireless, linux-kernel

SGkgQnJlbnQgYW5kIFNvdXB0aWNrLA0KDQoNCg0KDQpPbiAxMi8yMS8xNSwgMjoyMyBQTSwgImF0
aDZrbCBvbiBiZWhhbGYgb2YgU291cHRpY2sgSm9hcmRlciIgPGF0aDZrbC1ib3VuY2VzQGxpc3Rz
LmluZnJhZGVhZC5vcmcgb24gYmVoYWxmIG9mIGpyZHIubGludXhAZ21haWwuY29tPiB3cm90ZToN
Cg0KPkhpIEJyZW50LA0KPg0KPk9uIFR1ZSwgRGVjIDEsIDIwMTUgYXQgMTE6MTEgQU0sIEJyZW50
IFRheWxvciA8bW90b2J1ZEBnbWFpbC5jb20+IHdyb3RlOg0KPj4gU2luY2UgY29tbWl0IDg0Mzc3
NTRjODMzNTFkNjIxM2MxYTQ3ZmYwMjljMTEyNmQ2MDQyYTcsIGFyLT5mdyBpcyBleHBlY3RlZCB0
byBiZSBwb2ludGluZyB0byBtZW1vcnkgYWxsb2NhdGVkIGJ5IHZtYWxsb2MuICBJZiB0aGUgYXBp
MSBtZXRob2QgKHZpYSBhdGg2a2xfZmV0Y2hfZndfYXBpMSkgaXMgdXNlZCB0byBhbGxvY2F0ZSBt
ZW1vcnkgZm9yIGFyLT5mdywgdGhlbiBrbWVtZHVwIGlzIHVzZWQuICBUaGlzIHBhdGNoIGNoZWNr
cyBpZiB0aGUgZmlybXdhcmUgYmVpbmcgbG9hZGVkIGlzIHRoZSAnZncnIGltYWdlLCB0aGVuIHVz
ZSB2bWFsbG9jLCBvdGhlcndpc2UgdXNlIGttYWxsb2MuDQo+Pg0KPj4gU2lnbmVkLW9mZi1ieTog
QnJlbnQgVGF5bG9yIDxtb3RvYnVkQGdtYWlsLmNvbT4NCj4+IC0tLQ0KPj4gIGRyaXZlcnMvbmV0
L3dpcmVsZXNzL2F0aC9hdGg2a2wvaW5pdC5jIHwgNyArKysrKystDQo+PiAgMSBmaWxlIGNoYW5n
ZWQsIDYgaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbigtKQ0KPj4NCj4+IGRpZmYgLS1naXQgYS9k
cml2ZXJzL25ldC93aXJlbGVzcy9hdGgvYXRoNmtsL2luaXQuYyBiL2RyaXZlcnMvbmV0L3dpcmVs
ZXNzL2F0aC9hdGg2a2wvaW5pdC5jDQo+PiBpbmRleCA2YWUwNzM0Li40ZjJiMTI0ZCAxMDA2NDQN
Cj4+IC0tLSBhL2RyaXZlcnMvbmV0L3dpcmVsZXNzL2F0aC9hdGg2a2wvaW5pdC5jDQo+PiArKysg
Yi9kcml2ZXJzL25ldC93aXJlbGVzcy9hdGgvYXRoNmtsL2luaXQuYw0KPj4gQEAgLTY3MywxMCAr
NjczLDE1IEBAIHN0YXRpYyBpbnQgYXRoNmtsX2dldF9mdyhzdHJ1Y3QgYXRoNmtsICphciwgY29u
c3QgY2hhciAqZmlsZW5hbWUsDQo+PiAgICAgICAgICAgICAgICAgcmV0dXJuIHJldDsNCj4+DQo+
PiAgICAgICAgICpmd19sZW4gPSBmd19lbnRyeS0+c2l6ZTsNCj4+IC0gICAgICAgKmZ3ID0ga21l
bWR1cChmd19lbnRyeS0+ZGF0YSwgZndfZW50cnktPnNpemUsIEdGUF9LRVJORUwpOw0KPj4gKyAg
IGlmICgmYXItPmZ3ID09IGZ3KQ0KPj4gKyAgICAgICAgICAgICAgICpmdyA9IHZtYWxsb2MoZndf
ZW50cnktPnNpemUpOw0KPj4gKyAgIGVsc2UNCj4+ICsgICAgICAgICAgICAgICAqZncgPSBrbWFs
bG9jKGZ3X2VudHJ5LT5zaXplLCBHRlBfS0VSTkVMKTsNCj4NCj4gICAgICAgICAgV2h5IHZtYWxs
b2MgYW5kIGttYWxsb2MgYm90aCBhcmUgcmVxdWlyZWQ/IGNhbid0IHVzZSBlaXRoZXINCj52bWFs
bG9jIG9yIGttYWxsb2M/DQoNCk15IGd1ZXNzIGlzIHRoZSByZWFzb24gdG8gdXNlIGJvdGggdm1h
bGxvYyBhbmQga21hbGxvYyBpcyB0aGF0IHRoZSBmaXJtd2FyZSBibG9iIGNhbiBiZSBuZWFyIDEy
OEtCLiAgSSBrbm93IG91ciBhcjYwMDMgZmlybXdhcmVzIGFwcHJvYWNoIHRoYXQuICBTbyB2bWFs
bG9jIG11c3QgaGF2ZSBiZWVuIGNob3NlbiB0byBhdm9pZCBhbnkgaXNzdWVzIGlmIGl0IHdhcyBn
cmVhdGVyIHRoYW4gMTI4S0IuICBTbyBrbWFsbG9jIGlzIHVzZWQgZm9yIGFsbCB0aGUgc21hbGwg
ZmlybXdhcmUgcGllY2VzIChib2FyZCBkYXRhLCBvdHAuYmluLCBldGMpIGJ1dCB2bWFsbG9jIGZv
ciB0aGUgZmlybXdhcmUgaXRzZWxmLiAgDQoNCkkgcGVyc29uYWxseSBmaXhlZCB0aGlzIGlzc3Vl
IGZvciBsb2FkaW5nIHRoZSBmaXJtd2FyZSAoYnV0IG5vdCBib2FyZCBkYXRhLCBvcHQuYmluLCBl
dGMpIGluIHRoZSBhcGkxIGFuZCB0ZXN0bW9kZSBmdW5jdGlvbnMgYnkgaGF2ZSBpdCBjYWxsIGEg
bmV3IGhlbHBlciBmdW5jdGlvbjoNCg0Kc3RhdGljIGludCBhdGg2a2xfZ2V0X2Z3X3ZtKHN0cnVj
dCBhdGg2a2wgKmFyLCBjb25zdCBjaGFyICpmaWxlbmFtZSwNCgkJCSB1OCAqKmZ3LCBzaXplX3Qg
KmZ3X2xlbikNCnsNCgljb25zdCBzdHJ1Y3QgZmlybXdhcmUgKmZ3X2VudHJ5Ow0KCWludCByZXQ7
DQoNCglyZXQgPSByZXF1ZXN0X2Zpcm13YXJlKCZmd19lbnRyeSwgZmlsZW5hbWUsIGFyLT5kZXYp
Ow0KCWlmIChyZXQpDQoJcmV0dXJuIHJldDsNCg0KCSpmd19sZW4gPSBmd19lbnRyeS0+c2l6ZTsN
CgkqZncgPSB2bWFsbG9jKCpmd19sZW4pOw0KDQoJaWYgKCpmdyA9PSBOVUxMKQ0KCXJldCA9IC1F
Tk9NRU07DQoNCgltZW1jcHkoKmZ3LCBmd19lbnRyeS0+ZGF0YSwgKmZ3X2xlbik7DQoNCglyZWxl
YXNlX2Zpcm13YXJlKGZ3X2VudHJ5KTsNCg0KCXJldHVybiByZXQ7DQp9DQoNCg0KDQo+Pg0KPj4g
ICAgICAgICBpZiAoKmZ3ID09IE5VTEwpDQo+PiAgICAgICAgICAgICAgICAgcmV0ID0gLUVOT01F
TTsNCj4+ICsgICAgICAgZWxzZQ0KPj4gKyAgICAgICAgICAgICAgIG1lbWNweSgqZncsIGZ3X2Vu
dHJ5LT5kYXRhLCBmd19lbnRyeS0+c2l6ZSk7DQo+Pg0KPj4gICAgICAgICByZWxlYXNlX2Zpcm13
YXJlKGZ3X2VudHJ5KTsNCj4+DQo+PiAtLQ0KPj4gMi42LjMNCj4+DQo+PiAtLQ0KPj4gVG8gdW5z
dWJzY3JpYmUgZnJvbSB0aGlzIGxpc3Q6IHNlbmQgdGhlIGxpbmUgInVuc3Vic2NyaWJlIGxpbnV4
LXdpcmVsZXNzIiBpbg0KPj4gdGhlIGJvZHkgb2YgYSBtZXNzYWdlIHRvIG1ham9yZG9tb0B2Z2Vy
Lmtlcm5lbC5vcmcNCj4+IE1vcmUgbWFqb3Jkb21vIGluZm8gYXQgIGh0dHA6Ly92Z2VyLmtlcm5l
bC5vcmcvbWFqb3Jkb21vLWluZm8uaHRtbA0KPg0KPi1Tb3VwdGljaw0KPg0KPl9fX19fX19fX19f
X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fDQo+YXRoNmtsIG1haWxpbmcgbGlz
dA0KPmF0aDZrbEBsaXN0cy5pbmZyYWRlYWQub3JnDQo+aHR0cDovL2xpc3RzLmluZnJhZGVhZC5v
cmcvbWFpbG1hbi9saXN0aW5mby9hdGg2a2wNCg0KLSBEYW4gS2VwaGFydA0K

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH] ath6kl: Use vmalloc to allocate ar->fw for api1 method
@ 2015-12-21 22:00     ` Dan Kephart
  0 siblings, 0 replies; 12+ messages in thread
From: Dan Kephart @ 2015-12-21 22:00 UTC (permalink / raw)
  To: Souptick Joarder, Brent Taylor
  Cc: netdev, Kalle Valo, ath6kl, linux-wireless, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3139 bytes --]

Hi Brent and Souptick,




On 12/21/15, 2:23 PM, "ath6kl on behalf of Souptick Joarder" <ath6kl-bounces@lists.infradead.org on behalf of jrdr.linux@gmail.com> wrote:

>Hi Brent,
>
>On Tue, Dec 1, 2015 at 11:11 AM, Brent Taylor <motobud@gmail.com> wrote:
>> Since commit 8437754c83351d6213c1a47ff029c1126d6042a7, ar->fw is expected to be pointing to memory allocated by vmalloc.  If the api1 method (via ath6kl_fetch_fw_api1) is used to allocate memory for ar->fw, then kmemdup is used.  This patch checks if the firmware being loaded is the 'fw' image, then use vmalloc, otherwise use kmalloc.
>>
>> Signed-off-by: Brent Taylor <motobud@gmail.com>
>> ---
>>  drivers/net/wireless/ath/ath6kl/init.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath6kl/init.c b/drivers/net/wireless/ath/ath6kl/init.c
>> index 6ae0734..4f2b124d 100644
>> --- a/drivers/net/wireless/ath/ath6kl/init.c
>> +++ b/drivers/net/wireless/ath/ath6kl/init.c
>> @@ -673,10 +673,15 @@ static int ath6kl_get_fw(struct ath6kl *ar, const char *filename,
>>                 return ret;
>>
>>         *fw_len = fw_entry->size;
>> -       *fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL);
>> +   if (&ar->fw == fw)
>> +               *fw = vmalloc(fw_entry->size);
>> +   else
>> +               *fw = kmalloc(fw_entry->size, GFP_KERNEL);
>
>          Why vmalloc and kmalloc both are required? can't use either
>vmalloc or kmalloc?

My guess is the reason to use both vmalloc and kmalloc is that the firmware blob can be near 128KB.  I know our ar6003 firmwares approach that.  So vmalloc must have been chosen to avoid any issues if it was greater than 128KB.  So kmalloc is used for all the small firmware pieces (board data, otp.bin, etc) but vmalloc for the firmware itself.  

I personally fixed this issue for loading the firmware (but not board data, opt.bin, etc) in the api1 and testmode functions by have it call a new helper function:

static int ath6kl_get_fw_vm(struct ath6kl *ar, const char *filename,
			 u8 **fw, size_t *fw_len)
{
	const struct firmware *fw_entry;
	int ret;

	ret = request_firmware(&fw_entry, filename, ar->dev);
	if (ret)
	return ret;

	*fw_len = fw_entry->size;
	*fw = vmalloc(*fw_len);

	if (*fw == NULL)
	ret = -ENOMEM;

	memcpy(*fw, fw_entry->data, *fw_len);

	release_firmware(fw_entry);

	return ret;
}



>>
>>         if (*fw == NULL)
>>                 ret = -ENOMEM;
>> +       else
>> +               memcpy(*fw, fw_entry->data, fw_entry->size);
>>
>>         release_firmware(fw_entry);
>>
>> --
>> 2.6.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>-Souptick
>
>_______________________________________________
>ath6kl mailing list
>ath6kl@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/ath6kl

- Dan Kephart
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH] ath6kl: Use vmalloc to allocate ar->fw for api1 method
@ 2015-12-21 22:00     ` Dan Kephart
  0 siblings, 0 replies; 12+ messages in thread
From: Dan Kephart @ 2015-12-21 22:00 UTC (permalink / raw)
  To: Souptick Joarder, Brent Taylor
  Cc: netdev, Kalle Valo, ath6kl, linux-wireless, linux-kernel

Hi Brent and Souptick,




On 12/21/15, 2:23 PM, "ath6kl on behalf of Souptick Joarder" <ath6kl-bounces@lists.infradead.org on behalf of jrdr.linux@gmail.com> wrote:

>Hi Brent,
>
>On Tue, Dec 1, 2015 at 11:11 AM, Brent Taylor <motobud@gmail.com> wrote:
>> Since commit 8437754c83351d6213c1a47ff029c1126d6042a7, ar->fw is expected to be pointing to memory allocated by vmalloc.  If the api1 method (via ath6kl_fetch_fw_api1) is used to allocate memory for ar->fw, then kmemdup is used.  This patch checks if the firmware being loaded is the 'fw' image, then use vmalloc, otherwise use kmalloc.
>>
>> Signed-off-by: Brent Taylor <motobud@gmail.com>
>> ---
>>  drivers/net/wireless/ath/ath6kl/init.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath6kl/init.c b/drivers/net/wireless/ath/ath6kl/init.c
>> index 6ae0734..4f2b124d 100644
>> --- a/drivers/net/wireless/ath/ath6kl/init.c
>> +++ b/drivers/net/wireless/ath/ath6kl/init.c
>> @@ -673,10 +673,15 @@ static int ath6kl_get_fw(struct ath6kl *ar, const char *filename,
>>                 return ret;
>>
>>         *fw_len = fw_entry->size;
>> -       *fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL);
>> +   if (&ar->fw == fw)
>> +               *fw = vmalloc(fw_entry->size);
>> +   else
>> +               *fw = kmalloc(fw_entry->size, GFP_KERNEL);
>
>          Why vmalloc and kmalloc both are required? can't use either
>vmalloc or kmalloc?

My guess is the reason to use both vmalloc and kmalloc is that the firmware blob can be near 128KB.  I know our ar6003 firmwares approach that.  So vmalloc must have been chosen to avoid any issues if it was greater than 128KB.  So kmalloc is used for all the small firmware pieces (board data, otp.bin, etc) but vmalloc for the firmware itself.  

I personally fixed this issue for loading the firmware (but not board data, opt.bin, etc) in the api1 and testmode functions by have it call a new helper function:

static int ath6kl_get_fw_vm(struct ath6kl *ar, const char *filename,
			 u8 **fw, size_t *fw_len)
{
	const struct firmware *fw_entry;
	int ret;

	ret = request_firmware(&fw_entry, filename, ar->dev);
	if (ret)
	return ret;

	*fw_len = fw_entry->size;
	*fw = vmalloc(*fw_len);

	if (*fw == NULL)
	ret = -ENOMEM;

	memcpy(*fw, fw_entry->data, *fw_len);

	release_firmware(fw_entry);

	return ret;
}



>>
>>         if (*fw == NULL)
>>                 ret = -ENOMEM;
>> +       else
>> +               memcpy(*fw, fw_entry->data, fw_entry->size);
>>
>>         release_firmware(fw_entry);
>>
>> --
>> 2.6.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>-Souptick
>
>_______________________________________________
>ath6kl mailing list
>ath6kl@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/ath6kl

- Dan Kephart

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH] ath6kl: Use vmalloc to allocate ar->fw for api1 method
  2015-12-21 21:53   ` Brent Taylor
@ 2015-12-22  9:05     ` Souptick Joarder
  2015-12-22  9:12       ` Kalle Valo
  0 siblings, 1 reply; 12+ messages in thread
From: Souptick Joarder @ 2015-12-22  9:05 UTC (permalink / raw)
  To: Brent Taylor; +Cc: Kalle Valo, linux-wireless, netdev, linux-kernel, ath6kl

Hi Brent,

On Tue, Dec 22, 2015 at 3:23 AM, Brent Taylor <motobud@gmail.com> wrote:
> On Mon, Dec 21, 2015 at 1:23 PM, Souptick Joarder <jrdr.linux@gmail.com> wrote:
>> Hi Brent,
>>
>> On Tue, Dec 1, 2015 at 11:11 AM, Brent Taylor <motobud@gmail.com> wrote:
>>> Since commit 8437754c83351d6213c1a47ff029c1126d6042a7, ar->fw is expected to be pointing to memory allocated by vmalloc.  If the api1 method (via ath6kl_fetch_fw_api1) is used to allocate memory for ar->fw, then kmemdup is used.  This patch checks if the firmware being loaded is the 'fw' image, then use vmalloc, otherwise use kmalloc.
>>>
>>> Signed-off-by: Brent Taylor <motobud@gmail.com>
>>> ---
>>>  drivers/net/wireless/ath/ath6kl/init.c | 7 ++++++-
>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath6kl/init.c b/drivers/net/wireless/ath/ath6kl/init.c
>>> index 6ae0734..4f2b124d 100644
>>> --- a/drivers/net/wireless/ath/ath6kl/init.c
>>> +++ b/drivers/net/wireless/ath/ath6kl/init.c
>>> @@ -673,10 +673,15 @@ static int ath6kl_get_fw(struct ath6kl *ar, const char *filename,
>>>                 return ret;
>>>
>>>         *fw_len = fw_entry->size;
>>> -       *fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL);
>>> +   if (&ar->fw == fw)
>>> +               *fw = vmalloc(fw_entry->size);
>>> +   else
>>> +               *fw = kmalloc(fw_entry->size, GFP_KERNEL)
>>
>>           Why vmalloc and kmalloc both are required? can't use either
>> vmalloc or kmalloc?
>
> My original problem was that kmemdup (which uses kmalloc) could not
> allocate enough memory

If kmemdump ( which uses kmalloc) could not allocate memory then
using kmalloc again can lead to same problem.
I guess it will be correct to use
            *fw = vmalloc(fw_entry->size);
Correct me if i am wrong.

> to hold the firmware that is placed into "ar->fw".  In the function
> ath6kl_core_cleanup (in core.c),
> the "ar->fw" pointer is the only one that uses vfree which was changed in commit
> 8437754c83351d6213c1a47ff029c1126d6042a7.
>
> I was trying to change as little as possible and I wasn't sure if
> there was a reason that any of the
> other firmware items needed to be allocated with kmalloc or if they
> could be changed to use vmalloc.

-Souptick

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH] ath6kl: Use vmalloc to allocate ar->fw for api1 method
  2015-12-22  9:05     ` Souptick Joarder
@ 2015-12-22  9:12       ` Kalle Valo
  2015-12-29 18:16         ` Souptick Joarder
  0 siblings, 1 reply; 12+ messages in thread
From: Kalle Valo @ 2015-12-22  9:12 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: Brent Taylor, netdev, ath6kl, linux-wireless, linux-kernel

Souptick Joarder <jrdr.linux@gmail.com> writes:

> Hi Brent,
>
> On Tue, Dec 22, 2015 at 3:23 AM, Brent Taylor <motobud@gmail.com> wrote:
>> On Mon, Dec 21, 2015 at 1:23 PM, Souptick Joarder <jrdr.linux@gmail.com> wrote:
>>> Hi Brent,
>>>
>>> On Tue, Dec 1, 2015 at 11:11 AM, Brent Taylor <motobud@gmail.com> wrote:
>>>
>>>> --- a/drivers/net/wireless/ath/ath6kl/init.c
>>>> +++ b/drivers/net/wireless/ath/ath6kl/init.c
>>>> @@ -673,10 +673,15 @@ static int ath6kl_get_fw(struct ath6kl *ar, const char *filename,
>>>>                 return ret;
>>>>
>>>>         *fw_len = fw_entry->size;
>>>> -       *fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL);
>>>> +   if (&ar->fw == fw)
>>>> +               *fw = vmalloc(fw_entry->size);
>>>> +   else
>>>> +               *fw = kmalloc(fw_entry->size, GFP_KERNEL)
>>>
>>>           Why vmalloc and kmalloc both are required? can't use either
>>> vmalloc or kmalloc?
>>
>> My original problem was that kmemdup (which uses kmalloc) could not
>> allocate enough memory
>
> If kmemdump ( which uses kmalloc) could not allocate memory then
> using kmalloc again can lead to same problem.
> I guess it will be correct to use
>             *fw = vmalloc(fw_entry->size);
> Correct me if i am wrong.

That sounds best. But remember take into account DMA requirements, IIRC
you cannot DMA from vmalloc memory on all platforms.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH] ath6kl: Use vmalloc to allocate ar->fw for api1 method
  2015-12-22  9:12       ` Kalle Valo
@ 2015-12-29 18:16         ` Souptick Joarder
  0 siblings, 0 replies; 12+ messages in thread
From: Souptick Joarder @ 2015-12-29 18:16 UTC (permalink / raw)
  To: Brent Taylor; +Cc: netdev, ath6kl, linux-wireless, linux-kernel, Kalle Valo

Brent,

On Tue, Dec 22, 2015 at 2:42 PM, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> Souptick Joarder <jrdr.linux@gmail.com> writes:
>
>> Hi Brent,
>>
>> On Tue, Dec 22, 2015 at 3:23 AM, Brent Taylor <motobud@gmail.com> wrote:
>>> On Mon, Dec 21, 2015 at 1:23 PM, Souptick Joarder <jrdr.linux@gmail.com> wrote:
>>>> Hi Brent,
>>>>
>>>> On Tue, Dec 1, 2015 at 11:11 AM, Brent Taylor <motobud@gmail.com> wrote:
>>>>
>>>>> --- a/drivers/net/wireless/ath/ath6kl/init.c
>>>>> +++ b/drivers/net/wireless/ath/ath6kl/init.c
>>>>> @@ -673,10 +673,15 @@ static int ath6kl_get_fw(struct ath6kl *ar, const char *filename,
>>>>>                 return ret;
>>>>>
>>>>>         *fw_len = fw_entry->size;
>>>>> -       *fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL);
>>>>> +   if (&ar->fw == fw)
>>>>> +               *fw = vmalloc(fw_entry->size);
>>>>> +   else
>>>>> +               *fw = kmalloc(fw_entry->size, GFP_KERNEL)
>>>>
>>>>           Why vmalloc and kmalloc both are required? can't use either
>>>> vmalloc or kmalloc?
>>>
>>> My original problem was that kmemdup (which uses kmalloc) could not
>>> allocate enough memory
>>
>> If kmemdump ( which uses kmalloc) could not allocate memory then
>> using kmalloc again can lead to same problem.
>> I guess it will be correct to use
>>             *fw = vmalloc(fw_entry->size);
>> Correct me if i am wrong.
>
> That sounds best. But remember take into account DMA requirements, IIRC
> you cannot DMA from vmalloc memory on all platforms.

Is it possible to modify the patch as per feedback from Kalle.

>
> --
> Kalle Valo

-Souptick

^ permalink raw reply	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-12-29 18:16 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-01  5:41 [PATCH] ath6kl: Use vmalloc to allocate ar->fw for api1 method Brent Taylor
2015-12-01  5:41 ` Brent Taylor
2015-12-01 13:08 ` Sergei Shtylyov
2015-12-13  0:52 ` Andy Shevchenko
2015-12-21 19:23 ` Souptick Joarder
2015-12-21 21:53   ` Brent Taylor
2015-12-22  9:05     ` Souptick Joarder
2015-12-22  9:12       ` Kalle Valo
2015-12-29 18:16         ` Souptick Joarder
2015-12-21 22:00   ` Dan Kephart
2015-12-21 22:00     ` Dan Kephart
2015-12-21 22:00     ` Dan Kephart

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.