LinuxLists.cc - [PATCH] removal of "static foo = 0" from drivers/ide (test11)

[permalink] [raw]

Subject: Re: [PATCH] removal of "static foo = 0" from drivers/ide (test11)

On Tue, 21 Nov 2000 22:25:01 Bartlomiej Zolnierkiewicz wrote:
>
> Quick removal of unnecessary initialization to 0.
>
>
> -static int basePort = 0; /* base port address */
> -static int regPort = 0; /* port for register number */
> -static int dataPort = 0; /* port for register data */
> +static int basePort; /* base port address */
> +static int regPort; /* port for register number */
> +static int dataPort; /* port for register data */

That is not too much confidence on the ANSI-ness of the compiler ???

--
Juan Antonio Magallon Lacarta #> cd /pub
mailto:[email protected] #> more beer

Linux 2.2.18-pre22-vm #7 SMP Sun Nov 19 03:29:20 CET 2000 i686 unknown

2000-11-21 23:35:07

[permalink] [raw]

Subject: Re: [PATCH] removal of "static foo = 0" from drivers/ide (test11)

On Tue, 21 Nov 2000, J . A . Magallon wrote:

>
> On Tue, 21 Nov 2000 22:25:01 Bartlomiej Zolnierkiewicz wrote:
> >
> > Quick removal of unnecessary initialization to 0.
> >
> >
> > -static int basePort = 0; /* base port address */
> > -static int regPort = 0; /* port for register number */
> > -static int dataPort = 0; /* port for register data */
> > +static int basePort; /* base port address */
> > +static int regPort; /* port for register number */
> > +static int dataPort; /* port for register data */
>
> That is not too much confidence on the ANSI-ness of the compiler ???
>

Quite the contrary. The patch seems correct and useful to me. What do you
think is wrong with it? (Linus accepted megabytes worth of the above in
the past...)

(see arch/i386/kernel/head.S and look for surprised. This is becoming FAQ,
Richard, how about putting it into your FAQ mentioned at the bottom of
this message?)

Regards,
Tigran

2000-11-21 23:48:48

[permalink] [raw]

Subject: Re: [PATCH] removal of "static foo = 0" from drivers/ide (test11)

On Wed, 22 Nov 2000 00:04:53 Tigran Aivazian wrote:
> On Tue, 21 Nov 2000, J . A . Magallon wrote:
>
> Quite the contrary. The patch seems correct and useful to me. What do you
> think is wrong with it? (Linus accepted megabytes worth of the above in
> the past...)
>

Sorry, i should look at the rest of the code. Seeing only that, is seems like
that variables have to hold an initial value of zero, and the patch relies
on the ANSI behaviour of the compiler that auto-initializes them to 0.
I have seen many compilers break ANSI rules in optimized mode. Typical
runs-fine-in-debug-mode-but-breaks-on-production-release.
One other point for info would be gcc specs.

--
Juan Antonio Magallon Lacarta #> cd /pub
mailto:[email protected] #> more beer

Linux 2.2.18-pre22-vm #7 SMP Sun Nov 19 03:29:20 CET 2000 i686 unknown

2000-11-21 23:56:18

[permalink] [raw]

Subject: Re: [PATCH] removal of "static foo = 0" from drivers/ide (test11)

On Wed, 22 Nov 2000, J . A . Magallon wrote:

>
> On Wed, 22 Nov 2000 00:04:53 Tigran Aivazian wrote:
> > On Tue, 21 Nov 2000, J . A . Magallon wrote:
> >
> > Quite the contrary. The patch seems correct and useful to me. What do you
> > think is wrong with it? (Linus accepted megabytes worth of the above in
> > the past...)
> >
>
> Sorry, i should look at the rest of the code. Seeing only that, is seems like
> that variables have to hold an initial value of zero, and the patch relies
> on the ANSI behaviour of the compiler that auto-initializes them to 0.
> I have seen many compilers break ANSI rules in optimized mode. Typical
> runs-fine-in-debug-mode-but-breaks-on-production-release.
> One other point for info would be gcc specs.

In the case of kernel, we have to do many things manually, can't rely on
some compiler (sometimes :). So, the code I pointed you at
arch/i386/kernel/head.S (look for "Clear BSS") is in fact what clears the
BSS; without it you will end up with uninitialized garbage in what you
think "ANSI C compiler arranged" for you.

Regards,
Tigran

2000-11-22 00:00:38

[permalink] [raw]

Subject: Re: [PATCH] removal of "static foo = 0" from drivers/ide (test11)

On Wed, 22 Nov 2000 00:26:23 Tigran Aivazian wrote:
> On Wed, 22 Nov 2000, J . A . Magallon wrote:
>
> In the case of kernel, we have to do many things manually, can't rely on
> some compiler (sometimes :). So, the code I pointed you at
> arch/i386/kernel/head.S (look for "Clear BSS") is in fact what clears the
> BSS; without it you will end up with uninitialized garbage in what you
> think "ANSI C compiler arranged" for you.
>

Thanks, that makes everything clear...I'm very suspicious on compilers.
Last thing I saw was VisualC++ skipping constructors...but that is
off-topic, we talked about 'compilers'...

--
Juan Antonio Magallon Lacarta #> cd /pub
mailto:[email protected] #> more beer

Linux 2.2.18-pre22-vm #7 SMP Sun Nov 19 03:29:20 CET 2000 i686 unknown

2000-11-22 00:40:05

by Jeff Garzik

[permalink] [raw]

Subject: Re: [PATCH] removal of "static foo = 0" from drivers/ide (test11)

"J . A . Magallon" wrote:
> On Tue, 21 Nov 2000 22:25:01 Bartlomiej Zolnierkiewicz wrote:
> > -static int dataPort = 0; /* port for register data */
> > +static int dataPort; /* port for register data */
>
> That is not too much confidence on the ANSI-ness of the compiler ???

There is nothing wrong with that change. Standard kernel style cleanup,
which saves a few bytes in the output kernel image.

Jeff

--
Jeff Garzik |
Building 1024 | The chief enemy of creativity is "good" sense
MandrakeSoft | -- Picasso

2000-11-22 13:18:44

[permalink] [raw]

Subject: Re: [PATCH] removal of "static foo = 0" from drivers/ide (test11)

J . A . Magallon writes:
> On Tue, 21 Nov 2000 22:25:01 Bartlomiej Zolnierkiewicz wrote:
> > -static int basePort = 0; /* base port address */
> > -static int regPort = 0; /* port for register number */
> > -static int dataPort = 0; /* port for register data */
> > +static int basePort; /* base port address */
> > +static int regPort; /* port for register number */
> > +static int dataPort; /* port for register data */
>
> That is not too much confidence on the ANSI-ness of the compiler ???

Its got nothing to do with the ANSI-ness of the compiler, but more to do
with the C startup code - does the C startup code initialise BSS to zero
or does it not?

In this particular example, we have full control over it; in the kernel
has our own start up code which does initialise the BSS to zero.

In the dim and distant past (about 7-8 years ago), it was true that we
didn't init the BSS and everything had to be initialised, but this is
long gone.
_____
|_____| ------------------------------------------------- ---+---+-
| | Russell King [email protected] --- ---
| | | | http://www.arm.linux.org.uk/personal/aboutme.html / / |
| +-+-+ --- -+-
/ | THE developer of ARM Linux |+| /|\
/ | | | --- |
+-+-+ ------------------------------------------------- /\\\ |

2000-11-24 21:14:15

by Rusty Russell

[permalink] [raw]

Subject: Re: [PATCH] removal of "static foo = 0" from drivers/ide (test11)

In message <[email protected]> you write:
> > On Tue, 21 Nov 2000 22:25:01 Bartlomiej Zolnierkiewicz wrote:
> > >
> > > Quick removal of unnecessary initialization to 0.
>
> Quite the contrary. The patch seems correct and useful to me. What do you
> think is wrong with it? (Linus accepted megabytes worth of the above in
> the past...)

What irritates about these monkey-see-monkey-do patches is that if I
initialize a variable to NULL, it's because my code actually relies on
it; I don't want that information eliminated.

Rusty.
--
Hacking time.

2000-11-24 22:10:41

by Vojtech Pavlik

[permalink] [raw]

Subject: Re: [PATCH] removal of "static foo = 0" from drivers/ide (test11)

On Thu, Nov 23, 2000 at 10:01:53PM +1100, Rusty Russell wrote:
> In message <[email protected]> you write:
> > > On Tue, 21 Nov 2000 22:25:01 Bartlomiej Zolnierkiewicz wrote:
> > > >
> > > > Quick removal of unnecessary initialization to 0.
> >
> > Quite the contrary. The patch seems correct and useful to me. What do you
> > think is wrong with it? (Linus accepted megabytes worth of the above in
> > the past...)
>
> What irritates about these monkey-see-monkey-do patches is that if I
> initialize a variable to NULL, it's because my code actually relies on
> it; I don't want that information eliminated.

Yes, but if it generates a bigger (== worse) binary?

--
Vojtech Pavlik
SuSE Labs

2000-11-24 23:44:23

[permalink] [raw]

Subject: Re: [PATCH] removal of "static foo = 0" from drivers/ide (test11)

On Thu, 23 Nov 2000 12:01:53 Rusty Russell wrote:
>
> What irritates about these monkey-see-monkey-do patches is that if I
> initialize a variable to NULL, it's because my code actually relies on
> it; I don't want that information eliminated.
>

What I understood from the previous answer from Tigran is that you can
avoid initializations to ZERO or NULL, because the BSS is zeroed (ie, all
variables you declare static os global in a module are zeroed) at
kernel start.

As I understand from compiler working, if you put a statement like
int a = 0;
int b = 0;
all variables that have an initial value are stored contiguous in the
data segment of the executable and an image of their initial values has
to be stored with the binary. So if a program like

int a[16384];
int main() {}

gives a binary of 13k, if you write it as

int a[16384] = {0};
int main() {}

it has to store the 64k of a, just to put a 0 in the first place and
make the exec size to 78k.

ANSI rules for C say that uninitialized vars get a 0, but you can't trust
on the ANSI behaviour of a compiler.

Obviuosly, you have to leave your initializations to 7 or -1 in place.

--
Juan Antonio Magallon Lacarta #> cd /pub
mailto:[email protected] #> more beer

Linux 2.2.18-pre23-vm #3 SMP Wed Nov 22 22:33:53 CET 2000 i686 unknown

2000-11-25 12:35:20

[permalink] [raw]

Subject: Re: [PATCH] removal of "static foo = 0" from drivers/ide (test11)

J . A . Magallon writes:
> ANSI rules for C say that uninitialized vars get a 0, but you can't trust
> on the ANSI behaviour of a compiler.

It has nothing to do with the compiler, but everything to do with the
C startup code. In the Linux kernel, we have complete control over the
C startup code - it is in arch/*/kernel/head.S.

The only way a compiler can break this is if it creates a new section
.bss_im_not_going_to_allow_anyone_to_initialise_this and places all
the variables in there. Hardly likely, don't you think?

The initialisation of .bss is a run-time issue, NOT a compiler issue.
_____
|_____| ------------------------------------------------- ---+---+-
| | Russell King [email protected] --- ---
| | | | http://www.arm.linux.org.uk/personal/aboutme.html / / |
| +-+-+ --- -+-
/ | THE developer of ARM Linux |+| /|\
/ | | | --- |
+-+-+ ------------------------------------------------- /\\\ |

2000-11-25 12:35:20

[permalink] [raw]

Subject: Re: [PATCH] removal of "static foo = 0" from drivers/ide (test11)

Rusty Russell writes:
> What irritates about these monkey-see-monkey-do patches is that if I
> initialize a variable to NULL, it's because my code actually relies on
> it; I don't want that information eliminated.

What information is lost? Unless you're working on a really strange
machine which does not zero bss, the following means the same from the
codes point of view:

static int foo = 0;
static int foo;

Both are initialised to zero by the time the code sees them for the
first time. Therefore there is no difference to the code in its reliance
on whether foo is zero. foo will be zero in both cases.

Also, any good programmer worth their skin should know this, and should
realise it. Therefore, there is no information loss about what value
that variable is expected to hold at initialisation time.

The only difference is the size on disk; if we go around setting every
bss variable to zero, the kernel/module data size will unnecessarily
huge.

We already argue about the extra couple of bytes that xx change to the
kernel/a module would cost. With these change, we save kilo-bytes in
disk space (which is important on some systems).
_____
|_____| ------------------------------------------------- ---+---+-
| | Russell King [email protected] --- ---
| | | | http://www.arm.linux.org.uk/personal/aboutme.html / / |
| +-+-+ --- -+-
/ | THE developer of ARM Linux |+| /|\
/ | | | --- |
+-+-+ ------------------------------------------------- /\\\ |

2000-11-26 00:29:17

by Bartlomiej Zolnierkiewicz

[permalink] [raw]

Subject: Re: [PATCH] removal of "static foo = 0" from drivers/ide (test11)

On Thu, 23 Nov 2000, Rusty Russell wrote:

> In message <[email protected]> you write:
> > > On Tue, 21 Nov 2000 22:25:01 Bartlomiej Zolnierkiewicz wrote:
> > > >
> > > > Quick removal of unnecessary initialization to 0.
> >
> > Quite the contrary. The patch seems correct and useful to me. What do you
> > think is wrong with it? (Linus accepted megabytes worth of the above in
> > the past...)
>
> What irritates about these monkey-see-monkey-do patches is that if I
> initialize a variable to NULL, it's because my code actually relies on
> it; I don't want that information eliminated.

What irritates about these monkey-do-not-read-monkey-write e-mails
is that I know what I am doing :) My patch removed *REALLY* unnecessary
initialization, no code depended on beginning value so don't quote
my words...

--
Bartlomiej Zolnierkiewicz
<[email protected]>

2000-11-28 12:00:29

by Vojtech Pavlik

[permalink] [raw]

Subject: Re: [PATCH] removal of "static foo = 0" from drivers/ide (test11)

On Tue, Nov 28, 2000 at 02:19:23PM +1100, Rusty Russell wrote:
> In message <[email protected]> you write:
> > On Thu, Nov 23, 2000 at 10:01:53PM +1100, Rusty Russell wrote:
> > > What irritates about these monkey-see-monkey-do patches is that if I
> > > initialize a variable to NULL, it's because my code actually relies on
> > > it; I don't want that information eliminated.
> >
> > Yes, but if it generates a bigger (== worse) binary?
>
> We're talking about a few bytes, here. If you're prepared to make my
> code less clear to save bytes, you can do much better than that...

Perhaps in your case you had just an

int a = 0;

then it's really just a few bytes, but many sources have for example

int a[1024] = { 0, 0, /* .... */ };

Which in turn is a big wastage.

On the other hand, if you save "just" a few bytes in every driver, in a
way that is safe and simple (and commenting out the = 0 is a safe way),
you get a lot of space saved in the sum.

--
Vojtech Pavlik
SuSE Labs

2000-11-28 21:27:47

by David Hinds

[permalink] [raw]

Subject: Re: [PATCH] removal of "static foo = 0" from drivers/ide (test11)

> What information is lost? Unless you're working on a really strange
> machine which does not zero bss, the following means the same from the
> codes point of view:
>
> static int foo = 0;
> static int foo;

I think the argument is that "static int foo;" implies you don't
actually care how "foo" is initialized, but adding the "= 0" is
revealing that the code actually relies on the default value. The
code is obviously equivalent. It is a readability issue, not an issue
of what the code does.

I would contend that it is a compiler bug in gcc if it treats the two
statements differently, since they are trivially equivalent. I guess
that it has been decided that linux kernel coding style dictates no
zero initializers, so that's that. Personally, I prefer symmetry: if
I have a list of static variables initialized to various things, I
don't have to use a different form for ones that are zero initialized.

Did the savings really work out to be measured in kb's of space? I
would have expected compression to eliminate most of the savings.

-- Dave

2000-11-28 21:36:42

[permalink] [raw]

Subject: Re: [PATCH] removal of "static foo = 0" from drivers/ide (test11)

On Tue, 28 Nov 2000, David Hinds wrote:
> I would contend that it is a compiler bug in gcc if it treats the two
> statements differently, since they are trivially equivalent. I guess
> that it has been decided that linux kernel coding style dictates no
> zero initializers, so that's that. Personally, I prefer symmetry: if
> I have a list of static variables initialized to various things, I
> don't have to use a different form for ones that are zero initialized.

I would contend that you are wrong -- it is not a compiler bug.

First, they are not trivially equivalent. In fact, they are not equivalent
at all. Any good C book should tell you that one places data in "data
segment" and another in "bss segment" (with a footnote explaining
historical meaning of "block started by symbol")

Second, If compiler did the ".data -> .bss" move on the fly then it would
violate the principle of least surprize. For example a programmer could
expect the variable to be in .data for binary patching the object (this
reason is not applicable to the Linux kernel because section information
is lost when converting vmlinux to raw binary and then onto bzImage).

>
> Did the savings really work out to be measured in kb's of space? I
> would have expected compression to eliminate most of the savings.
>

if one doesn't count compression then -- hundreds of Kbs (K=1024 and b =
byte, not bit!). Even Mbs if you count multiple architectures.

Use Keith Owens' famous perl script to do the counting.

Regards,
Tigran

2000-11-29 00:24:38

by Peter Samuelson

[permalink] [raw]

Subject: Re: [PATCH] removal of "static foo = 0" from drivers/ide (test11)

[Tigran Aivazian]
> First, they are not trivially equivalent. In fact, they are not
> equivalent at all. Any good C book should tell you that one places
> data in "data segment" and another in "bss segment" (with a footnote
> explaining historical meaning of "block started by symbol")

Do you have an ISO C standard to refer to? I don't, but my guess is
that the standard does not even *mention* discreet data sections. It
*does* say that globals and statics are implicitly initialized to 0 --
the rest is a mere implementation issue.

So yes, the two statements *are* trivially equivalent, unless I'm wrong
and the standard *does* talk about data sections.

> For example a programmer could expect the variable to be in .data for
> binary patching the object

Binary patching? If you are binary patching something you need to get
the exact location, one way or another. Whatever tool you use to
extract the location of a symbol in an object file, that same tool
should tell you which section it is in. If the tool only looks in
'.data', it is flawed.

[David Hinds]
> > Did the savings really work out to be measured in kb's of space? I
> > would have expected compression to eliminate most of the savings.

Some boot targets are not compressed. And the variables are scattered
through the .data section rather than all in one place, so the 0s may
not compress as well as you'd think.

Anyway, by some estimates 'vmlinux' has gone down by several kilobytes
due to these patches. Obviously the exact savings depend on .config .

Peter

2000-11-29 03:53:48

by Keith Owens

[permalink] [raw]

Subject: Re: [PATCH] removal of "static foo = 0" from drivers/ide (test11)

On Tue, 28 Nov 2000 17:53:48 -0600,
Peter Samuelson <[email protected]> wrote:
>Binary patching? If you are binary patching something you need to get
>the exact location, one way or another. Whatever tool you use to
>extract the location of a symbol in an object file, that same tool
>should tell you which section it is in. If the tool only looks in
>'.data', it is flawed.

The whole point of bss is that it does not have any space allocated in
the object on disk. bss is just a section entry saying "this size, at
this location", the area is allocated and zeroed at load time. Binary
patches against bss on disk cannot work, there is nothing to patch.

2000-11-29 04:06:30

by Peter Samuelson

[permalink] [raw]

Subject: Re: [PATCH] removal of "static foo = 0" from drivers/ide (test11)

[Keith Owens]
> Binary patches against bss on disk cannot work, there is nothing to
> patch.

OK, me dumkopf.

Peter

2000-11-29 08:17:48

[permalink] [raw]

Subject: Re: [PATCH] removal of "static foo = 0" from drivers/ide (test11)

On Tue, 28 Nov 2000, Peter Samuelson wrote:
> [Tigran Aivazian]
> > First, they are not trivially equivalent. In fact, they are not
> > equivalent at all. Any good C book should tell you that one places
> > data in "data segment" and another in "bss segment" (with a footnote
> > explaining historical meaning of "block started by symbol")
>
> Do you have an ISO C standard to refer to? I don't, but my guess is
> that the standard does not even *mention* discreet data sections. It
> *does* say that globals and statics are implicitly initialized to 0 --
> the rest is a mere implementation issue.
>
> So yes, the two statements *are* trivially equivalent, unless I'm wrong
> and the standard *does* talk about data sections.

no, I was not talking about ISO C standards but about the normal expected
C environment under any UNIX. I guess, we just mean different things by
"trivially equivalent" since neither of us said anything about what that
actually meant. What I meant by that was "not only imply the same logic
for the program but correspond to the same physical locations (if any) on
the resulting object file". In this sense, obviously, they are not
"trivially equivalent" since in one case there is no physical location at
all -- that is the whole point of having bss at all.

>
> > For example a programmer could expect the variable to be in .data for
> > binary patching the object
>
> Binary patching? If you are binary patching something you need to get
> the exact location, one way or another. Whatever tool you use to
> extract the location of a symbol in an object file, that same tool
> should tell you which section it is in. If the tool only looks in
> '.data', it is flawed.

Keith already pointed out why you are wrong there.

>
> [David Hinds]
> > > Did the savings really work out to be measured in kb's of space? I
> > > would have expected compression to eliminate most of the savings.
>
> Some boot targets are not compressed. And the variables are scattered
> through the .data section rather than all in one place, so the 0s may
> not compress as well as you'd think.
>
> Anyway, by some estimates 'vmlinux' has gone down by several kilobytes
> due to these patches. Obviously the exact savings depend on .config .

"several" kilobytes? _My_ patches alone (accepted by Linus ages
ago) reduced it by >280K.

Regards,
Tigran

2000-11-29 14:32:13

by Peter Samuelson

[permalink] [raw]

Subject: Re: [PATCH] removal of "static foo = 0" from drivers/ide (test11)

[Tigran Aivazian <[email protected]>]
> no, I was not talking about ISO C standards but about the normal
> expected C environment under any UNIX. I guess, we just mean
> different things by "trivially equivalent" since neither of us said
> anything about what that actually meant. What I meant by that was
> "not only imply the same logic for the program but correspond to the
> same physical locations (if any) on the resulting object file".

OK, so we do mean two different things. As far as standard C is
concerned, the location of your global variables is none of your
business, as long as they can be addressed with a pointer of the
appropriate type. By the time you get to caring about what section
something goes in, you're way beyond standard C, and you need to start
looking at the gcc __attribute__(("section")) extension, and possibly
an ld link script.

So I guess to me "trivially equivalent" meant that a standards-
conforming C program would never notice the difference. As such, I
think -fassume-bss-zero would be an entirely reasonable default for a C
compiler, just as reasonable as reordering globals within a section
(and nobody seems to be questioning gcc's right to do *that*).

It would probably also be reasonable to document it and provide an
option to switch it off.

Peter

2000-11-29 14:37:15

[permalink] [raw]

Subject: Re: [PATCH] removal of "static foo = 0" from drivers/ide (test11)

On Wed, 29 Nov 2000, Peter Samuelson wrote:
> It would probably also be reasonable to document it and provide an
> option to switch it off.

yes, that is perfectly fine by me. Now, who is going to do the _work_? :)

Regards,
Tigran

2000-11-29 16:47:59

by Darryl L. Miles

[permalink] [raw]

Subject: Re: [PATCH] removal of "static foo = 0" from drivers/ide (test11)

Russell King <[email protected]> writes:
>The only difference is the size on disk; if we go around setting every
>bss variable to zero, the kernel/module data size will unnecessarily
>huge.

Hmm, what about common symbol generation? i.e. the linker looses the
ability
to throw out "multiply defined symbol" errors where you fail to
initialise it
to a value.

Okay extern global variables in the kernel need to be controlled and it
is not
like many get added, however it is possible that one developer may never
know
it is already in use by another part of the kernel, when their oh-no-new
driver
is added. Since the linkers assistance in this issue has just been
disabled.

Is 'gas' able to be configured to never emit common symbols, but emit
BBS
symbols instead, or is 'ld' able to be configured to never merge common
symbols but throw up "multiply defined symbol" errors. Then everyone is
safe.

>We already argue about the extra couple of bytes that xx change to the
>kernel/a module would cost. With these change, we save kilo-bytes in
>disk space (which is important on some systems).

PDAs!!! :) Excellent work Russell.

--
Darryl Miles

2000-11-30 00:56:29