[PATCH] ath9k: decrease size of ath9k.ko

[PATCH] ath9k: decrease size of ath9k.ko

[tom.leiming]

From: Ming Lei <tom.leiming@gmail.com>

The patch defines the fields of 'valid_single_stream' and 'valid' in
struct ath_rate_table as char type, so decrease the size of ath9k.ko
about 2KB.

old ath9k.ko
[tom@tom-lei ath9k]$ size ath9k.ko
   text    data     bss     dec     hex filename
  69344    3080     168   72592   11b90 ath9k.ko

new ath9k.ko
[tom@tom-lei ath9k]$ size ath9k.ko
   text    data     bss     dec     hex filename
  67304    3080     168   70552   11398 ath9k.ko

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 drivers/net/wireless/ath/ath9k/rc.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/rc.h b/drivers/net/wireless/ath/ath9k/rc.h
index 4f6d6fd..389168a 100644
--- a/drivers/net/wireless/ath/ath9k/rc.h
+++ b/drivers/net/wireless/ath/ath9k/rc.h
@@ -110,8 +110,8 @@ struct ath_rate_table {
 	int rate_cnt;
 	int mcs_start;
 	struct {
-		int valid;
-		int valid_single_stream;
+		char valid;
+		char valid_single_stream;
 		u8 phy;
 		u32 ratekbps;
 		u32 user_ratekbps;
-- 
1.6.2.5

Re: [PATCH] ath9k: decrease size of ath9k.ko

[Larry Finger]

On 02/27/2010 10:56 AM, tom.leiming@gmail.com wrote:
> From: Ming Lei <tom.leiming@gmail.com>
> 
> The patch defines the fields of 'valid_single_stream' and 'valid' in
> struct ath_rate_table as char type, so decrease the size of ath9k.ko
> about 2KB.
> 
> old ath9k.ko
> [tom@tom-lei ath9k]$ size ath9k.ko
>    text    data     bss     dec     hex filename
>   69344    3080     168   72592   11b90 ath9k.ko
> 
> new ath9k.ko
> [tom@tom-lei ath9k]$ size ath9k.ko
>    text    data     bss     dec     hex filename
>   67304    3080     168   70552   11398 ath9k.ko
> 
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
> ---
>  drivers/net/wireless/ath/ath9k/rc.h |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath9k/rc.h b/drivers/net/wireless/ath/ath9k/rc.h
> index 4f6d6fd..389168a 100644
> --- a/drivers/net/wireless/ath/ath9k/rc.h
> +++ b/drivers/net/wireless/ath/ath9k/rc.h
> @@ -110,8 +110,8 @@ struct ath_rate_table {
>  	int rate_cnt;
>  	int mcs_start;
>  	struct {
> -		int valid;
> -		int valid_single_stream;
> +		char valid;
> +		char valid_single_stream;
>  		u8 phy;
>  		u32 ratekbps;
>  		u32 user_ratekbps;

Why 'char' rather than 'u8'? To me, the latter implies a small integer, not
character data.

Larry

Re: [PATCH] ath9k: decrease size of ath9k.ko

[Ming Lei]

2010/2/28 Larry Finger <Larry.Finger@lwfinger.net>:
> On 02/27/2010 10:56 AM, tom.leiming@gmail.com wrote:
>> From: Ming Lei <tom.leiming@gmail.com>
>>
>> The patch defines the fields of 'valid_single_stream' and 'valid' in
>> struct ath_rate_table as char type, so decrease the size of ath9k.ko
>> about 2KB.
>>
>> old ath9k.ko
>> [tom@tom-lei ath9k]$ size ath9k.ko
>>    text    data     bss     dec     hex filename
>>   69344    3080     168   72592   11b90 ath9k.ko
>>
>> new ath9k.ko
>> [tom@tom-lei ath9k]$ size ath9k.ko
>>    text    data     bss     dec     hex filename
>>   67304    3080     168   70552   11398 ath9k.ko
>>
>> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
>> ---
>>  drivers/net/wireless/ath/ath9k/rc.h |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/rc.h b/drivers/net/wireless/ath/ath9k/rc.h
>> index 4f6d6fd..389168a 100644
>> --- a/drivers/net/wireless/ath/ath9k/rc.h
>> +++ b/drivers/net/wireless/ath/ath9k/rc.h
>> @@ -110,8 +110,8 @@ struct ath_rate_table {
>>       int rate_cnt;
>>       int mcs_start;
>>       struct {
>> -             int valid;
>> -             int valid_single_stream;
>> +             char valid;
>> +             char valid_single_stream;
>>               u8 phy;
>>               u32 ratekbps;
>>               u32 user_ratekbps;
>
> Why 'char' rather than 'u8'? To me, the latter implies a small integer, not
> character data.

Either 'char' or 'u8' is OK, since both has one-byte size, isn't it?

Thanks,

-- 
Lei Ming

Re: [PATCH] ath9k: decrease size of ath9k.ko

[Larry Finger]

On 02/27/2010 08:52 PM, Ming Lei wrote:
> 2010/2/28 Larry Finger <Larry.Finger@lwfinger.net>:
>> On 02/27/2010 10:56 AM, tom.leiming@gmail.com wrote:
>>> From: Ming Lei <tom.leiming@gmail.com>
>>>
>>> The patch defines the fields of 'valid_single_stream' and 'valid' in
>>> struct ath_rate_table as char type, so decrease the size of ath9k.ko
>>> about 2KB.
>>>
>>> old ath9k.ko
>>> [tom@tom-lei ath9k]$ size ath9k.ko
>>>    text    data     bss     dec     hex filename
>>>   69344    3080     168   72592   11b90 ath9k.ko
>>>
>>> new ath9k.ko
>>> [tom@tom-lei ath9k]$ size ath9k.ko
>>>    text    data     bss     dec     hex filename
>>>   67304    3080     168   70552   11398 ath9k.ko
>>>
>>> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
>>> ---
>>>  drivers/net/wireless/ath/ath9k/rc.h |    4 ++--
>>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath9k/rc.h b/drivers/net/wireless/ath/ath9k/rc.h
>>> index 4f6d6fd..389168a 100644
>>> --- a/drivers/net/wireless/ath/ath9k/rc.h
>>> +++ b/drivers/net/wireless/ath/ath9k/rc.h
>>> @@ -110,8 +110,8 @@ struct ath_rate_table {
>>>       int rate_cnt;
>>>       int mcs_start;
>>>       struct {
>>> -             int valid;
>>> -             int valid_single_stream;
>>> +             char valid;
>>> +             char valid_single_stream;
>>>               u8 phy;
>>>               u32 ratekbps;
>>>               u32 user_ratekbps;
>>
>> Why 'char' rather than 'u8'? To me, the latter implies a small integer, not
>> character data.
> 
> Either 'char' or 'u8' is OK, since both has one-byte size, isn't it?

It wasn't the size, but the content implied by the type.

Re: [PATCH] ath9k: decrease size of ath9k.ko

[Ming Lei]

>From e74b075cdb143d45be9b371ee8a8e2dcfc15ab34 Mon Sep 17 00:00:00 2001
From: Ming Lei <tom.leiming@gmail.com>
Date: Sat, 27 Feb 2010 23:50:54 +0800
Subject: [PATCH] ath9k: decrease size of ath9k.ko

The patch defines the fields of 'valid_single_stream' and 'valid' in
struct ath_rate_table as char type, so decrease the size of ath9k.ko
about 2KB.

old ath9k.ko
[tom@tom-lei ath9k]$ size ath9k.ko
   text    data     bss     dec     hex filename
  69344    3080     168   72592   11b90 ath9k.ko

new ath9k.ko
[tom@tom-lei ath9k]$ size ath9k.ko
   text    data     bss     dec     hex filename
  67304    3080     168   70552   11398 ath9k.ko

Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---

This version takes Larry's suggestion to define 'valid_single_stream'
and 'valid' as u8 instead of char.

---
 drivers/net/wireless/ath/ath9k/rc.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/rc.h b/drivers/net/wireless/ath/ath9k/rc.h
index 4f6d6fd..aeaaa88 100644
--- a/drivers/net/wireless/ath/ath9k/rc.h
+++ b/drivers/net/wireless/ath/ath9k/rc.h
@@ -110,8 +110,8 @@ struct ath_rate_table {
 	int rate_cnt;
 	int mcs_start;
 	struct {
-		int valid;
-		int valid_single_stream;
+		u8 valid;
+		u8 valid_single_stream;
 		u8 phy;
 		u32 ratekbps;
 		u32 user_ratekbps;
-- 
1.6.2.5



On Sat, 27 Feb 2010 21:19:51 -0600
Larry Finger <Larry.Finger@lwfinger.net> wrote:

> On 02/27/2010 08:52 PM, Ming Lei wrote:
> > 2010/2/28 Larry Finger <Larry.Finger@lwfinger.net>:
> >> On 02/27/2010 10:56 AM, tom.leiming@gmail.com wrote:
> >>> From: Ming Lei <tom.leiming@gmail.com>
> > 
> > Either 'char' or 'u8' is OK, since both has one-byte size, isn't it?
> 
> It wasn't the size, but the content implied by the type.

Re: [PATCH] ath9k: decrease size of ath9k.ko

[Pavel Roskin]

On Mon, 2010-03-01 at 23:13 +0800, Ming Lei wrote:
> -		int valid;
> -		int valid_single_stream;
> +		u8 valid;
> +		u8 valid_single_stream;

You can use bool instead, and that would give the same size saving while
being even more descriptive.  I think using bool could be safer, as the
compiler would be able to detect some misuses and the values.

But I could get even more saving by using bool with the field width:

bool valid:1;
bool valid_single_stream:1;

That would place both variables into one byte.  It may be ineffective
for speed, but it's more effective for storage.

In my configuration, I get following sizes:

original (int):    2792138
your patch (u8):   2790186
bool:              2790186
bool (1 bit):      2789218

-- 
Regards,
Pavel Roskin

Re: [PATCH] ath9k: decrease size of ath9k.ko

[Luis R. Rodriguez]

On Tue, Mar 02, 2010 at 09:02:44PM -0800, Pavel Roskin wrote:
> On Mon, 2010-03-01 at 23:13 +0800, Ming Lei wrote:
> > -		int valid;
> > -		int valid_single_stream;
> > +		u8 valid;
> > +		u8 valid_single_stream;
> 
> You can use bool instead, and that would give the same size saving while
> being even more descriptive.  I think using bool could be safer, as the
> compiler would be able to detect some misuses and the values.
> 
> But I could get even more saving by using bool with the field width:
> 
> bool valid:1;
> bool valid_single_stream:1;
> 
> That would place both variables into one byte.  It may be ineffective
> for speed, but it's more effective for storage.
> 
> In my configuration, I get following sizes:
> 
> original (int):    2792138
> your patch (u8):   2790186
> bool:              2790186
> bool (1 bit):      2789218

Or use flags, and use BIT(1), BIT(2), etc for #defines for each stream mode.
Would save even more.

  Luis

Re: [PATCH] ath9k: decrease size of ath9k.ko

[Ming Lei]

2010/3/3 Pavel Roskin <proski@gnu.org>:
> On Mon, 2010-03-01 at 23:13 +0800, Ming Lei wrote:
>> -             int valid;
>> -             int valid_single_stream;
>> +             u8 valid;
>> +             u8 valid_single_stream;
>
> You can use bool instead, and that would give the same size saving while
> being even more descriptive.  I think using bool could be safer, as the
> compiler would be able to detect some misuses and the values.
>
> But I could get even more saving by using bool with the field width:
>
> bool valid:1;
> bool valid_single_stream:1;
>
> That would place both variables into one byte.  It may be ineffective
> for speed, but it's more effective for storage.
>
> In my configuration, I get following sizes:
>
> original (int):    2792138
> your patch (u8):   2790186
> bool:              2790186
> bool (1 bit):      2789218

1bit is not enough, at least 3bit is required for each one.

We does not save more using bit field since size of the least type
is 1 byte.

-- 
Lei Ming

Re: [PATCH] ath9k: decrease size of ath9k.ko

[Holger Schurig]

> Either 'char' or 'u8' is OK, since both has one-byte size, isn't it?

Besides Johannes' comment: char is signed, u8 is unsigned. This can sometimes 
byte you.

-- 
http://www.holgerschurig.de

Re: [stable] [PATCH] ath9k: decrease size of ath9k.ko

[Greg KH]

On Sun, Feb 28, 2010 at 12:56:43AM +0800, tom.leiming@gmail.com wrote:
> From: Ming Lei <tom.leiming@gmail.com>
> 
> The patch defines the fields of 'valid_single_stream' and 'valid' in
> struct ath_rate_table as char type, so decrease the size of ath9k.ko
> about 2KB.

Why would this be a -stable patch?

confused,

greg k-h

Re: [stable] [PATCH] ath9k: decrease size of ath9k.ko

[Luis R. Rodriguez]

On Wed, Mar 10, 2010 at 03:52:20PM -0800, Greg KH wrote:
> On Sun, Feb 28, 2010 at 12:56:43AM +0800, tom.leiming@gmail.com wrote:
> > From: Ming Lei <tom.leiming@gmail.com>
> > 
> > The patch defines the fields of 'valid_single_stream' and 'valid' in
> > struct ath_rate_table as char type, so decrease the size of ath9k.ko
> > about 2KB.
> 
> Why would this be a -stable patch?
> 
> confused,

Its not, oh he e-mailed stable, yeah ignore.

  Luis