From: Ming Lei <[email protected]>
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 <[email protected]>
---
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
2010/2/28 Larry Finger <[email protected]>:
> On 02/27/2010 10:56 AM, [email protected] wrote:
>> From: Ming Lei <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>> ?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
On 02/27/2010 08:52 PM, Ming Lei wrote:
> 2010/2/28 Larry Finger <[email protected]>:
>> On 02/27/2010 10:56 AM, [email protected] wrote:
>>> From: Ming Lei <[email protected]>
>>>
>>> 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 <[email protected]>
>>> ---
>>> 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.
On 02/27/2010 10:56 AM, [email protected] wrote:
> From: Ming Lei <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
> 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
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
>From e74b075cdb143d45be9b371ee8a8e2dcfc15ab34 Mon Sep 17 00:00:00 2001
From: Ming Lei <[email protected]>
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 <[email protected]>
Signed-off-by: Ming Lei <[email protected]>
---
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 <[email protected]> wrote:
> On 02/27/2010 08:52 PM, Ming Lei wrote:
> > 2010/2/28 Larry Finger <[email protected]>:
> >> On 02/27/2010 10:56 AM, [email protected] wrote:
> >>> From: Ming Lei <[email protected]>
> >
> > 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.
2010/3/3 Pavel Roskin <[email protected]>:
> 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
On Sun, Feb 28, 2010 at 12:56:43AM +0800, [email protected] wrote:
> From: Ming Lei <[email protected]>
>
> 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
On Wed, Mar 10, 2010 at 03:52:20PM -0800, Greg KH wrote:
> On Sun, Feb 28, 2010 at 12:56:43AM +0800, [email protected] wrote:
> > From: Ming Lei <[email protected]>
> >
> > 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
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