[KJ] [PATCH] dm: uninitialize static variables

[KJ] [PATCH] dm: uninitialize static variables

[Nicolas Kaiser]

[-- Attachment #1: Type: text/plain, Size: 1036 bytes --]

uninitialize static variables initialized to 0, to make them go
to .bss instead of .data.

Signed-off-by: Nicolas Kaiser <nikai@nikai.net>
---
 dm.c     |    4 ++--
 kcopyd.c |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff -urpN a/drivers/md/dm.c b/drivers/md/dm.c
--- a/drivers/md/dm.c	2005-10-02 15:53:35.000000000 +0200
+++ b/drivers/md/dm.c	2005-12-05 00:01:26.000000000 +0100
@@ -20,8 +20,8 @@
 
 static const char *_name = DM_NAME;
 
-static unsigned int major = 0;
-static unsigned int _major = 0;
+static unsigned int major; /* = 0 */
+static unsigned int _major; /* = 0 */
 
 /*
  * One of these is allocated per bio.
diff -urpN a/drivers/md/kcopyd.c b/drivers/md/kcopyd.c
--- a/drivers/md/kcopyd.c	2005-10-02 15:53:35.000000000 +0200
+++ b/drivers/md/kcopyd.c	2005-12-05 00:00:33.000000000 +0100
@@ -588,7 +588,7 @@ static void client_del(struct kcopyd_cli
 }
 
 static DECLARE_MUTEX(kcopyd_init_lock);
-static int kcopyd_clients = 0;
+static int kcopyd_clients; /* = 0 */
 
 static int kcopyd_init(void)
 {

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

[KJ] [PATCH] dm: uninitialize static variables

[Nicolas Kaiser]

[-- Attachment #1: Type: text/plain, Size: 1036 bytes --]

uninitialize static variables initialized to 0, to make them go
to .bss instead of .data.

Signed-off-by: Nicolas Kaiser <nikai@nikai.net>
---
 dm.c     |    4 ++--
 kcopyd.c |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff -urpN a/drivers/md/dm.c b/drivers/md/dm.c
--- a/drivers/md/dm.c	2005-10-02 15:53:35.000000000 +0200
+++ b/drivers/md/dm.c	2005-12-05 00:01:26.000000000 +0100
@@ -20,8 +20,8 @@
 
 static const char *_name = DM_NAME;
 
-static unsigned int major = 0;
-static unsigned int _major = 0;
+static unsigned int major; /* = 0 */
+static unsigned int _major; /* = 0 */
 
 /*
  * One of these is allocated per bio.
diff -urpN a/drivers/md/kcopyd.c b/drivers/md/kcopyd.c
--- a/drivers/md/kcopyd.c	2005-10-02 15:53:35.000000000 +0200
+++ b/drivers/md/kcopyd.c	2005-12-05 00:00:33.000000000 +0100
@@ -588,7 +588,7 @@ static void client_del(struct kcopyd_cli
 }
 
 static DECLARE_MUTEX(kcopyd_init_lock);
-static int kcopyd_clients = 0;
+static int kcopyd_clients; /* = 0 */
 
 static int kcopyd_init(void)
 {

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] dm: uninitialize static variables

[Carlos Manuel Duclos Vergara]

Hi,

> uninitialize static variables initialized to 0, to make them go
> to .bss instead of .data.
>

if you want that why not explicitely put them in .bss?
something like:

> -static unsigned int major = 0;
> +static unsigned int major; /* = 0 */

static unsigned int major = 0 __attribute__ ((section(".bss"));

Even when crt should take care of initializing bss to zero, so I think
that you're trying to save some cpu cycles

my $1

-- 
Carlos Manuel Duclos Vergara
http://www.toolchains.com/personal/blog

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] dm: uninitialize static variables

[Carlos Manuel Duclos Vergara]

Hi,

> uninitialize static variables initialized to 0, to make them go
> to .bss instead of .data.
>

if you want that why not explicitely put them in .bss?
something like:

> -static unsigned int major = 0;
> +static unsigned int major; /* = 0 */

static unsigned int major = 0 __attribute__ ((section(".bss"));

Even when crt should take care of initializing bss to zero, so I think
that you're trying to save some cpu cycles

my $1

-- 
Carlos Manuel Duclos Vergara
http://www.toolchains.com/personal/blog

Re: [KJ] [PATCH] dm: uninitialize static variables

[Nicolas Kaiser]

[-- Attachment #1: Type: text/plain, Size: 1847 bytes --]

* "Carlos Manuel Duclos Vergara" <carlos@embedded.cl>:
> > uninitialize static variables initialized to 0, to make them go
> > to .bss instead of .data.
> >
> 
> if you want that why not explicitely put them in .bss?
> something like:
> 
> > -static unsigned int major = 0;
> > +static unsigned int major; /* = 0 */
> 
> static unsigned int major = 0 __attribute__ ((section(".bss"));

I personally like the idea of explicitly expressing this intention,
but I didn't manage to achieve this together with initialization.
I tried

 static unsigned int __attribute__((__section__(".bss"))) major = 0;
 static unsigned int __attribute__((__section__(".bss"))) _major = 0;

but they appear not to end up in .bss nevertheless, compared with the
uninitialized version:

--- dm-mod.ko1.txt      2005-12-05 18:40:00.000000000 +0100
+++ dm-mod.ko2.txt      2005-12-05 18:39:52.000000000 +0100
@@ -27,7 +27,7 @@
 [22] .rel.init.data       REL          00000000 00b174 000028  8       29  21  4
 [23] .gnu.linkonce.this_module PROGBITS     00000000 0087c0 000160  0 WA     0   0 32
 [24] .rel.gnu.linkonce.this_module REL          00000000 00b19c 000010  8       29  23  4
-[25] .bss                 NOBITS       00000000 008920 00049c  0 WA     0   0 32
+[25] .bss                 NOBITS       00000000 008920 000498  0 WA     0   0 32
 [26] .comment             PROGBITS     00000000 008920 00022e  0        0   0  1
 [27] .note.GNU-stack      PROGBITS     00000000 008b4e 000000  0        0   0  1
 [28] .shstrtab            STRTAB       00000000 008b4e 00011e  0        0   0  1

If you could tell me how to achieve this in a fancy way which also works,
I'm all ears. Maybe the janitorial steering committee might also like the
idea enough to adjust their current TODO list.
( http://janitor.kernelnewbies.org/TODO - 10th task in 6th section)

Cheers,
n.

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] dm: uninitialize static variables

[Nicolas Kaiser]

* "Carlos Manuel Duclos Vergara" <carlos@embedded.cl>:
> > uninitialize static variables initialized to 0, to make them go
> > to .bss instead of .data.
> >
> 
> if you want that why not explicitely put them in .bss?
> something like:
> 
> > -static unsigned int major = 0;
> > +static unsigned int major; /* = 0 */
> 
> static unsigned int major = 0 __attribute__ ((section(".bss"));

I personally like the idea of explicitly expressing this intention,
but I didn't manage to achieve this together with initialization.
I tried

 static unsigned int __attribute__((__section__(".bss"))) major = 0;
 static unsigned int __attribute__((__section__(".bss"))) _major = 0;

but they appear not to end up in .bss nevertheless, compared with the
uninitialized version:

--- dm-mod.ko1.txt      2005-12-05 18:40:00.000000000 +0100
+++ dm-mod.ko2.txt      2005-12-05 18:39:52.000000000 +0100
@@ -27,7 +27,7 @@
 [22] .rel.init.data       REL          00000000 00b174 000028  8       29  21  4
 [23] .gnu.linkonce.this_module PROGBITS     00000000 0087c0 000160  0 WA     0   0 32
 [24] .rel.gnu.linkonce.this_module REL          00000000 00b19c 000010  8       29  23  4
-[25] .bss                 NOBITS       00000000 008920 00049c  0 WA     0   0 32
+[25] .bss                 NOBITS       00000000 008920 000498  0 WA     0   0 32
 [26] .comment             PROGBITS     00000000 008920 00022e  0        0   0  1
 [27] .note.GNU-stack      PROGBITS     00000000 008b4e 000000  0        0   0  1
 [28] .shstrtab            STRTAB       00000000 008b4e 00011e  0        0   0  1

If you could tell me how to achieve this in a fancy way which also works,
I'm all ears. Maybe the janitorial steering committee might also like the
idea enough to adjust their current TODO list.
( http://janitor.kernelnewbies.org/TODO - 10th task in 6th section)

Cheers,
n.

Re: [KJ] [PATCH] dm: uninitialize static variables

[Carlos Manuel Duclos Vergara]

>  static unsigned int __attribute__((__section__(".bss"))) major = 0;
>  static unsigned int __attribute__((__section__(".bss"))) _major = 0;

it should be:

static unsigned int major __attribute__ ((section(".bss"))) = 0;

> If you could tell me how to achieve this in a fancy way which also works,
> I'm all ears. Maybe the janitorial steering committee might also like the
> idea enough to adjust their current TODO list.

yup, maybe it's worth of a discussion on the list

-- 
Carlos Manuel Duclos Vergara
http://www.toolchains.com/personal/blog

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] dm: uninitialize static variables

[Carlos Manuel Duclos Vergara]

>  static unsigned int __attribute__((__section__(".bss"))) major = 0;
>  static unsigned int __attribute__((__section__(".bss"))) _major = 0;

it should be:

static unsigned int major __attribute__ ((section(".bss"))) = 0;

> If you could tell me how to achieve this in a fancy way which also works,
> I'm all ears. Maybe the janitorial steering committee might also like the
> idea enough to adjust their current TODO list.

yup, maybe it's worth of a discussion on the list

-- 
Carlos Manuel Duclos Vergara
http://www.toolchains.com/personal/blog

Re: [KJ] [PATCH] dm: uninitialize static variables

[Alexey Dobriyan]

[-- Attachment #1: Type: text/plain, Size: 686 bytes --]

On Mon, Dec 05, 2005 at 01:28:00PM +0100, Nicolas Kaiser wrote:
> uninitialize static variables initialized to 0, to make them go
> to .bss instead of .data.

>  dm.c     |    4 ++--
>  kcopyd.c |    2 +-

diffstat -p1 ;-)

> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -20,8 +20,8 @@
>
>  static const char *_name = DM_NAME;
>
> -static unsigned int major = 0;
> -static unsigned int _major = 0;
> +static unsigned int major; /* = 0 */
> +static unsigned int _major; /* = 0 */

Drop the comments. major goes into .bss. .bss is initialized to 0.

> --- a/drivers/md/kcopyd.c
> +++ b/drivers/md/kcopyd.c

> -static int kcopyd_clients = 0;
> +static int kcopyd_clients; /* = 0 */


[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] dm: uninitialize static variables

[Alexey Dobriyan]

[-- Attachment #1: Type: text/plain, Size: 686 bytes --]

On Mon, Dec 05, 2005 at 01:28:00PM +0100, Nicolas Kaiser wrote:
> uninitialize static variables initialized to 0, to make them go
> to .bss instead of .data.

>  dm.c     |    4 ++--
>  kcopyd.c |    2 +-

diffstat -p1 ;-)

> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -20,8 +20,8 @@
>
>  static const char *_name = DM_NAME;
>
> -static unsigned int major = 0;
> -static unsigned int _major = 0;
> +static unsigned int major; /* = 0 */
> +static unsigned int _major; /* = 0 */

Drop the comments. major goes into .bss. .bss is initialized to 0.

> --- a/drivers/md/kcopyd.c
> +++ b/drivers/md/kcopyd.c

> -static int kcopyd_clients = 0;
> +static int kcopyd_clients; /* = 0 */


[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] dm: uninitialize static variables

[Alexey Dobriyan]

[-- Attachment #1: Type: text/plain, Size: 677 bytes --]

On Mon, Dec 05, 2005 at 07:00:29PM +0100, Nicolas Kaiser wrote:
> * "Carlos Manuel Duclos Vergara" <carlos@embedded.cl>:
> > > uninitialize static variables initialized to 0, to make them go
> > > to .bss instead of .data.
> > >
> >
> > if you want that why not explicitely put them in .bss?
> > something like:
> >
> > > -static unsigned int major = 0;
> > > +static unsigned int major; /* = 0 */
> >
> > static unsigned int major = 0 __attribute__ ((section(".bss"));
>
> I personally like the idea of explicitly expressing this intention,
> but I didn't manage to achieve this together with initialization.

Not only you use gcc'ism, you also make code needlessly verbose.


[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] dm: uninitialize static variables

[Alexey Dobriyan]

[-- Attachment #1: Type: text/plain, Size: 677 bytes --]

On Mon, Dec 05, 2005 at 07:00:29PM +0100, Nicolas Kaiser wrote:
> * "Carlos Manuel Duclos Vergara" <carlos@embedded.cl>:
> > > uninitialize static variables initialized to 0, to make them go
> > > to .bss instead of .data.
> > >
> >
> > if you want that why not explicitely put them in .bss?
> > something like:
> >
> > > -static unsigned int major = 0;
> > > +static unsigned int major; /* = 0 */
> >
> > static unsigned int major = 0 __attribute__ ((section(".bss"));
>
> I personally like the idea of explicitly expressing this intention,
> but I didn't manage to achieve this together with initialization.

Not only you use gcc'ism, you also make code needlessly verbose.


[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

[KJ] [PATCH] dm: uninitialize static variables v2

[Nicolas Kaiser]

[-- Attachment #1: Type: text/plain, Size: 1028 bytes --]

uninitialize static variables initialized to 0, to make them go
to .bss instead of .data.

Signed-off-by: Nicolas Kaiser <nikai@nikai.net>
---
 drivers/md/dm.c     |    4 ++--
 drivers/md/kcopyd.c |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff -urpN a/drivers/md/dm.c b/drivers/md/dm.c
--- a/drivers/md/dm.c	2005-10-02 15:53:35.000000000 +0200
+++ b/drivers/md/dm.c	2005-12-10 20:24:13.000000000 +0100
@@ -20,8 +20,8 @@
 
 static const char *_name = DM_NAME;
 
-static unsigned int major = 0;
-static unsigned int _major = 0;
+static unsigned int major;
+static unsigned int _major;
 
 /*
  * One of these is allocated per bio.
diff -urpN a/drivers/md/kcopyd.c b/drivers/md/kcopyd.c
--- a/drivers/md/kcopyd.c	2005-10-02 15:53:35.000000000 +0200
+++ b/drivers/md/kcopyd.c	2005-12-10 20:26:16.000000000 +0100
@@ -588,7 +588,7 @@ static void client_del(struct kcopyd_cli
 }
 
 static DECLARE_MUTEX(kcopyd_init_lock);
-static int kcopyd_clients = 0;
+static int kcopyd_clients;
 
 static int kcopyd_init(void)
 {

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

[KJ] [PATCH] dm: uninitialize static variables v2

[Nicolas Kaiser]

[-- Attachment #1: Type: text/plain, Size: 1028 bytes --]

uninitialize static variables initialized to 0, to make them go
to .bss instead of .data.

Signed-off-by: Nicolas Kaiser <nikai@nikai.net>
---
 drivers/md/dm.c     |    4 ++--
 drivers/md/kcopyd.c |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff -urpN a/drivers/md/dm.c b/drivers/md/dm.c
--- a/drivers/md/dm.c	2005-10-02 15:53:35.000000000 +0200
+++ b/drivers/md/dm.c	2005-12-10 20:24:13.000000000 +0100
@@ -20,8 +20,8 @@
 
 static const char *_name = DM_NAME;
 
-static unsigned int major = 0;
-static unsigned int _major = 0;
+static unsigned int major;
+static unsigned int _major;
 
 /*
  * One of these is allocated per bio.
diff -urpN a/drivers/md/kcopyd.c b/drivers/md/kcopyd.c
--- a/drivers/md/kcopyd.c	2005-10-02 15:53:35.000000000 +0200
+++ b/drivers/md/kcopyd.c	2005-12-10 20:26:16.000000000 +0100
@@ -588,7 +588,7 @@ static void client_del(struct kcopyd_cli
 }
 
 static DECLARE_MUTEX(kcopyd_init_lock);
-static int kcopyd_clients = 0;
+static int kcopyd_clients;
 
 static int kcopyd_init(void)
 {

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] dm: uninitialize static variables

[Jaco Kroon]

In reply to Nicolas Kaiser
> * "Carlos Manuel Duclos Vergara" <carlos@embedded.cl>:
>> > uninitialize static variables initialized to 0, to make them go
>> > to .bss instead of .data.
>> >
>>
>> if you want that why not explicitely put them in .bss?
>> something like:
>>
>> > -static unsigned int major = 0;
>> > +static unsigned int major; /* = 0 */
>>
>> static unsigned int major = 0 __attribute__ ((section(".bss"));
>
> I personally like the idea of explicitly expressing this intention,
> but I didn't manage to achieve this together with initialization.
> I tried

From http://janitor.kernelnewbies.org/TODO:

- uninitialize static variables initialized to 0, to make it go to the .bss,
  instead of .data.
  __initdata inside functions has to be initialized, even if to 0 though.

Therefor I would go with

static unsigned int major; /* = 0 */

Let the compiler make the decision, it will make the correct one.  The
comment is just there to indicate that we haven't forgotten to initialise
it.  As soon as you explicitly initialise you force the compiler to place
it somewhere other than bss.


-- 
There are 10 kinds of people in the world, those who understand binary,
and those who don't.
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] dm: uninitialize static variables

[Jaco Kroon]

In reply to Nicolas Kaiser
> * "Carlos Manuel Duclos Vergara" <carlos@embedded.cl>:
>> > uninitialize static variables initialized to 0, to make them go
>> > to .bss instead of .data.
>> >
>>
>> if you want that why not explicitely put them in .bss?
>> something like:
>>
>> > -static unsigned int major = 0;
>> > +static unsigned int major; /* = 0 */
>>
>> static unsigned int major = 0 __attribute__ ((section(".bss"));
>
> I personally like the idea of explicitly expressing this intention,
> but I didn't manage to achieve this together with initialization.
> I tried

From http://janitor.kernelnewbies.org/TODO:

- uninitialize static variables initialized to 0, to make it go to the .bss,
  instead of .data.
  __initdata inside functions has to be initialized, even if to 0 though.

Therefor I would go with

static unsigned int major; /* = 0 */

Let the compiler make the decision, it will make the correct one.  The
comment is just there to indicate that we haven't forgotten to initialise
it.  As soon as you explicitly initialise you force the compiler to place
it somewhere other than bss.


-- 
There are 10 kinds of people in the world, those who understand binary,
and those who don't.

Re: [KJ] [PATCH] dm: uninitialize static variables

[Jesse Millan]

[-- Attachment #1: Type: text/plain, Size: 2098 bytes --]


On Dec 11, 2005, at 11:17 PM, Jaco Kroon wrote:

>
> In reply to Nicolas Kaiser
>> * "Carlos Manuel Duclos Vergara" <carlos@embedded.cl>:
>>>> uninitialize static variables initialized to 0, to make them go
>>>> to .bss instead of .data.
>>>>
>>>
>>> if you want that why not explicitely put them in .bss?
>>> something like:
>>>
>>>> -static unsigned int major = 0;
>>>> +static unsigned int major; /* = 0 */
>>>
>>> static unsigned int major = 0 __attribute__ ((section(".bss"));
>>
>> I personally like the idea of explicitly expressing this intention,
>> but I didn't manage to achieve this together with initialization.
>> I tried
>
>> From http://janitor.kernelnewbies.org/TODO:
>
> - uninitialize static variables initialized to 0, to make it go to  
> the .bss,
>   instead of .data.
>   __initdata inside functions has to be initialized, even if to 0  
> though.
>
> Therefor I would go with
>
> static unsigned int major; /* = 0 */
>
> Let the compiler make the decision, it will make the correct one.  The
> comment is just there to indicate that we haven't forgotten to  
> initialise
> it.  As soon as you explicitly initialise you force the compiler to  
> place
> it somewhere other than bss.
>

The comment only indicates that you purposely did not initialize the  
variable. However, it does not indicate *why* you chose to do so like  
the section attribute would. What about:

static unsigned int major; /* __attribute__ ((section(".bss")) */

or

static unsigned int major; /* section(".bss") */

which is more explicit about your intentions, lets the compiler  
ultimately choose, and is not tied to gcc.


>
> -- 
> There are 10 kinds of people in the world, those who understand  
> binary,
> and those who don't.
> _______________________________________________
> Kernel-janitors mailing list
> Kernel-janitors@lists.osdl.org
> https://lists.osdl.org/mailman/listinfo/kernel-janitors

--
Jesse Millan
CNS Unix/Windows Team
Portland State University
Phone: (503) 725-3285
Homepage: www.system-calls.com

grep --recursive --ignore-case 'SHOULD WORK' /usr/src/linux/* | wc



[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors