* Re: [PATCH] removal of "static foo = 0"
@ 2000-11-27 5:56 Adam J. Richter
2000-11-27 8:41 ` Werner Almesberger
0 siblings, 1 reply; 23+ messages in thread
From: Adam J. Richter @ 2000-11-27 5:56 UTC (permalink / raw)
To: linux-kernel, meissner
Michael Meissner wrote:
>On Sat, Nov 25, 2000 at 11:55:11PM +0000, Tim Waugh wrote:
>> On Sat, Nov 25, 2000 at 10:53:00PM +0000, James A Sutherland wrote:
>>
>> > Which is silly. The variable is explicitly defined to be zero
>> > anyway, whether you put this in your code or not.
>>
>> Why doesn't the compiler just leave out explicit zeros from the
>> 'initial data' segment then? Seems like it ought to be tought to..
>
>Because sometimes it matters. For example, in kernel mode (and certainly for
>embedded programs that I'm more familiar with), the kernel does go through and
>zero out the so called BSS segment, so that normally uninitialized static
>variables will follow the rules as laid out under the C standards (both C89 and
>C99). I can imagine however, that the code that is executed before the BSS
>area is zeroed out needs to be extra careful in terms of statics that it
>references, and those must be hand initialized.
Since that code is already careful to hand initialize what
it needs and explicitly zeroes the BSS, that sounds like an argument
that it *is* safe to change gcc to move data that is intialized to
all zeroes into bss, either as a compiler option or even not
optionally.
I am not a gcc hacker, but it looks to me like one could
copy the code from output_constant and the functions that it
calls (in gcc-2.95.2/gcc/gcc/varasm.c) to walk the tree to figure
out if the data was all zeroes. I even started writing a routine
for assemble_variable to call to try to test just for the integer case
(basically just by cutting and pasting code). I include it here just
to illustrate. Note: this code doesn't even type check properly when
I try to compile it, so I know it's very wrong, but it's as good as
posting pseudo code to explain my thinking).
void
clear_zero_initialization(tree decl)
{
tree exp = DECL_INITIAL(decl);
enum tree_code code;
if (exp == NULL)
return;
code = TREE_CODE (TREE_TYPE (exp));
if (lang_expand_constant)
exp = (*lang_expand_constant) (exp);
while ((TREE_CODE (exp) == NOP_EXPR
&& (TREE_TYPE (exp) == TREE_TYPE (TREE_OPERAND (exp, 0))
|| AGGREGATE_TYPE_P (TREE_TYPE (exp))))
|| TREE_CODE (exp) == NON_LVALUE_EXPR)
exp = TREE_OPERAND (exp, 0);
if (code == INTEGER_TYPE && exp == const0_rtx)
DECL_INITIAL(decl) = NULL;
}
At the moment, I have started daydreaming about instead
writing an "elf squeezer" to do this and other space optimizations
by modifying objdump. However, I do think that such an improvement
to gcc would be at least a bit useful to the larger user base than
just those people who use binutils-based systems.
Adam J. Richter __ ______________ 4880 Stevens Creek Blvd, Suite 104
adam@yggdrasil.com \ / San Jose, California 95129-1034
+1 408 261-6630 | g g d r a s i l United States of America
fax +1 408 261-6631 "Free Software For The Rest Of Us."
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH] removal of "static foo = 0" 2000-11-27 5:56 [PATCH] removal of "static foo = 0" Adam J. Richter @ 2000-11-27 8:41 ` Werner Almesberger 2000-11-27 8:39 ` David S. Miller 2000-11-27 18:01 ` Michael Meissner 0 siblings, 2 replies; 23+ messages in thread From: Werner Almesberger @ 2000-11-27 8:41 UTC (permalink / raw) To: Adam J. Richter; +Cc: linux-kernel Adam J. Richter wrote: > At the moment, I have started daydreaming about instead > writing an "elf squeezer" to do this and other space optimizations > by modifying objdump. Hmm, this would require that gcc never calculates the location of an explicitly initialized static variable based on the address of another one. E.g. in static int a = 0, b = 0, c = 0, d = 0; ... ... a+b+c+d ... ... egcs-2.91.66 indeed doesn't seem to make this optimization on i386. (Maybe the pointer increment or pointer offset solution would actually be slower - didn't check.) But I'm not sure if this is also true for other versions/architectures, or other code constructs. - Werner -- _________________________________________________________________________ / Werner Almesberger, ICA, EPFL, CH Werner.Almesberger@epfl.ch / /_IN_N_032__Tel_+41_21_693_6621__Fax_+41_21_693_6610_____________________/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] removal of "static foo = 0" 2000-11-27 8:41 ` Werner Almesberger @ 2000-11-27 8:39 ` David S. Miller 2000-11-27 9:08 ` Werner Almesberger 2000-11-27 17:21 ` Andrea Arcangeli 2000-11-27 18:01 ` Michael Meissner 1 sibling, 2 replies; 23+ messages in thread From: David S. Miller @ 2000-11-27 8:39 UTC (permalink / raw) To: Werner.Almesberger; +Cc: adam, linux-kernel Date: Mon, 27 Nov 2000 09:41:39 +0100 From: Werner Almesberger <Werner.Almesberger@epfl.ch> egcs-2.91.66 indeed doesn't seem to make this optimization on i386. (Maybe the pointer increment or pointer offset solution would actually be slower - didn't check.) But I'm not sure if this is also true for other versions/architectures, or other code constructs. There is no guarentee that contiguous data or bss section members will appear contiguous and in the same order, in the final object. In fact, a specific optimization done on MIPS and other platforms is to place all data members under a certain size in a special ".small.data" section. So for example: static int a; static struct foo b; static int b; Would not place 'b' at "&a + sizeof(a) + sizeof(b)" Also I believe linkers are allowed to arbitrarily reorder members in the data and bss sections. I could be wrong on this one though. Later, David S. Miller davem@redhat.com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] removal of "static foo = 0" 2000-11-27 8:39 ` David S. Miller @ 2000-11-27 9:08 ` Werner Almesberger 2000-11-27 17:21 ` Andrea Arcangeli 1 sibling, 0 replies; 23+ messages in thread From: Werner Almesberger @ 2000-11-27 9:08 UTC (permalink / raw) To: David S. Miller; +Cc: adam, linux-kernel David S. Miller wrote: > There is no guarentee that contiguous data or bss section members > will appear contiguous and in the same order, in the final object. That's a different issue and actually okay in this case. What I meant to show is an example where the compiler happens to allocate the variables in sequence, and where it could access them either by referencing each by absolute address, with relocation (so that objdump-patcher could change that), or by generating a pointer and using pointer-relative addressing or pointer increment (so we only get one relocation and never know what may go wrong with the other variables). - Werner -- _________________________________________________________________________ / Werner Almesberger, ICA, EPFL, CH Werner.Almesberger@epfl.ch / /_IN_N_032__Tel_+41_21_693_6621__Fax_+41_21_693_6610_____________________/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] removal of "static foo = 0" 2000-11-27 8:39 ` David S. Miller 2000-11-27 9:08 ` Werner Almesberger @ 2000-11-27 17:21 ` Andrea Arcangeli 2000-11-27 17:36 ` Michael Meissner 2000-11-27 18:11 ` [PATCH] removal of "static foo = 0" Richard B. Johnson 1 sibling, 2 replies; 23+ messages in thread From: Andrea Arcangeli @ 2000-11-27 17:21 UTC (permalink / raw) To: David S. Miller; +Cc: Werner.Almesberger, adam, linux-kernel On Mon, Nov 27, 2000 at 12:39:55AM -0800, David S. Miller wrote: > Also I believe linkers are allowed to arbitrarily reorder members in > the data and bss sections. I could be wrong on this one though. I'm not sure either, but we certainly rely on that behaviour somewhere. Just to make an example fs/dquot.c: int nr_dquots, nr_free_dquots; kernel/sysctl.c: {FS_NRDQUOT, "dquot-nr", &nr_dquots, 2*sizeof(int), The above is ok also on mips in practice though. In 2.2.x there was more of them. Regardless if we're allowed to rely on the ordering the above is bad coding practice because somebody could forget about the dependency on the ordering and put something between nr_dquotes and nr_free_dquotes :), so such dependency should be avoided anyways... Andrea - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] removal of "static foo = 0" 2000-11-27 17:21 ` Andrea Arcangeli @ 2000-11-27 17:36 ` Michael Meissner 2000-11-27 19:06 ` Andrea Arcangeli 2000-11-27 21:27 ` Marcus Sundberg 2000-11-27 18:11 ` [PATCH] removal of "static foo = 0" Richard B. Johnson 1 sibling, 2 replies; 23+ messages in thread From: Michael Meissner @ 2000-11-27 17:36 UTC (permalink / raw) To: Andrea Arcangeli; +Cc: David S. Miller, Werner.Almesberger, adam, linux-kernel On Mon, Nov 27, 2000 at 06:21:13PM +0100, Andrea Arcangeli wrote: > On Mon, Nov 27, 2000 at 12:39:55AM -0800, David S. Miller wrote: > > Also I believe linkers are allowed to arbitrarily reorder members in > > the data and bss sections. I could be wrong on this one though. > > I'm not sure either, but we certainly rely on that behaviour somewhere. > Just to make an example fs/dquot.c: > > int nr_dquots, nr_free_dquots; > > kernel/sysctl.c: > > {FS_NRDQUOT, "dquot-nr", &nr_dquots, 2*sizeof(int), > > The above is ok also on mips in practice though. That needs to be fixed ASAP to use an array (not a structure). It is simply wrong to depend on two variables winding up in at adjacent offsets. -- Michael Meissner, Red Hat, Inc. PMB 198, 174 Littleton Road #3, Westford, Massachusetts 01886, USA Work: meissner@redhat.com phone: +1 978-486-9304 Non-work: meissner@spectacle-pond.org fax: +1 978-692-4482 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] removal of "static foo = 0" 2000-11-27 17:36 ` Michael Meissner @ 2000-11-27 19:06 ` Andrea Arcangeli 2000-11-27 19:34 ` Richard B. Johnson 2000-11-28 3:10 ` kumon 2000-11-27 21:27 ` Marcus Sundberg 1 sibling, 2 replies; 23+ messages in thread From: Andrea Arcangeli @ 2000-11-27 19:06 UTC (permalink / raw) To: Michael Meissner; +Cc: David S. Miller, Werner.Almesberger, adam, linux-kernel On Mon, Nov 27, 2000 at 12:36:55PM -0500, Michael Meissner wrote: > wrong to depend on two variables winding up in at adjacent offsets. I'd like if it will be written explicitly in the specs that it's forbidden to rely on that. I grepped the specs and I didn't find anything. So I wasn't sure if I missed the information in the specs or not. I never investigated on it because I always considered it bad coding regardless the fact it's guaranteed to generate the right asm with the _current_ tools. Andrea - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] removal of "static foo = 0" 2000-11-27 19:06 ` Andrea Arcangeli @ 2000-11-27 19:34 ` Richard B. Johnson 2000-11-28 0:28 ` Andrea Arcangeli 2000-11-28 3:10 ` kumon 1 sibling, 1 reply; 23+ messages in thread From: Richard B. Johnson @ 2000-11-27 19:34 UTC (permalink / raw) To: Andrea Arcangeli Cc: Michael Meissner, David S. Miller, Werner.Almesberger, adam, linux-kernel On Mon, 27 Nov 2000, Andrea Arcangeli wrote: > On Mon, Nov 27, 2000 at 12:36:55PM -0500, Michael Meissner wrote: > > wrong to depend on two variables winding up in at adjacent offsets. > > I'd like if it will be written explicitly in the specs that it's forbidden to > rely on that. I grepped the specs and I didn't find anything. So I wasn't sure > if I missed the information in the specs or not. I never investigated on it > because I always considered it bad coding regardless the fact it's guaranteed > to generate the right asm with the _current_ tools. > > Andrea > - The following shell-script shows that gcc-2.8.1 produces code with data allocations adjacent. However, they are reversed! cat - <<EOF >x.c int a, b; EOF gcc -c -o x.o x.c cat - <<EOF >y.c extern int a; extern int b; int main() { printf("a=%p\n", &a); printf("b=%p\n", &b); return 0; } EOF gcc -o y y.c x.o ./y a=0x804a42c b=0x804a428 The output shows variable 'a' as being in the higher address. So, it's not good to assume anything about so-called adjacent variables. Cheers, Dick Johnson Penguin : Linux version 2.4.0 on an i686 machine (799.54 BogoMips). "Memory is like gasoline. You use it up when you are running. Of course you get it all back when you reboot..."; Actual explanation obtained from the Micro$oft help desk. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] removal of "static foo = 0" 2000-11-27 19:34 ` Richard B. Johnson @ 2000-11-28 0:28 ` Andrea Arcangeli 2000-11-28 11:25 ` Horst von Brand 0 siblings, 1 reply; 23+ messages in thread From: Andrea Arcangeli @ 2000-11-28 0:28 UTC (permalink / raw) To: Richard B. Johnson Cc: Michael Meissner, David S. Miller, Werner.Almesberger, adam, linux-kernel On Mon, Nov 27, 2000 at 02:34:45PM -0500, Richard B. Johnson wrote: > The following shell-script shows that gcc-2.8.1 produces code with > data allocations adjacent. However, they are reversed! same with 2.95.* :). Andrea - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] removal of "static foo = 0" 2000-11-28 0:28 ` Andrea Arcangeli @ 2000-11-28 11:25 ` Horst von Brand 0 siblings, 0 replies; 23+ messages in thread From: Horst von Brand @ 2000-11-28 11:25 UTC (permalink / raw) To: Andrea Arcangeli; +Cc: linux-kernel Andrea Arcangeli <andrea@suse.de> said: > On Mon, Nov 27, 2000 at 02:34:45PM -0500, Richard B. Johnson wrote: > > The following shell-script shows that gcc-2.8.1 produces code with > > data allocations adjacent. However, they are reversed! > same with 2.95.* :). The point was if gcc did use the fact that variables are adyacent in memory to generate better code, and this degenerated into the current discussion about where they are from the programmers perspective. - If gcc is going to use the fact that some variables are nearby for some optimization purposes, I do trust the gcc hackers to set stuff up so that they use it for variables that are nearby in VM, not just where defined together. If variables defined together end up adyacent or not is completely irrelevant. The compiler might even rearrange them to optimize for the access pattern observed. - As a C programmer, you are only entitled to assume that pieces of the same object (array or struct) are laid out in memory in the order given. With segmented VM different objects would probably end up in different segments anyway. -- Dr. Horst H. von Brand mailto:vonbrand@inf.utfsm.cl Departamento de Informatica Fono: +56 32 654431 Universidad Tecnica Federico Santa Maria +56 32 654239 Casilla 110-V, Valparaiso, Chile Fax: +56 32 797513 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] removal of "static foo = 0" 2000-11-27 19:06 ` Andrea Arcangeli 2000-11-27 19:34 ` Richard B. Johnson @ 2000-11-28 3:10 ` kumon 2000-11-28 3:28 ` Andrea Arcangeli 1 sibling, 1 reply; 23+ messages in thread From: kumon @ 2000-11-28 3:10 UTC (permalink / raw) To: Andrea Arcangeli Cc: Michael Meissner, David S. Miller, Werner.Almesberger, adam, linux-kernel, kumon Andrea Arcangeli writes: > I'd like if it will be written explicitly in the specs that it's forbidden to > rely on that. I grepped the specs and I didn't find anything. So I wasn't sure > if I missed the information in the specs or not. I never investigated on it If you have two files: test1.c: int a,b,c; test2.c: int a,c; Which is _stronger_? If somebody adds such a file to the kernel tree, the layout is changed by link orderling, irrelevant option on/off or other magical environments. Spec doesn't say anything about the layout of the variables. -- Computer Systems Laboratory, Fujitsu Labs. kumon@flab.fujitsu.co.jp - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] removal of "static foo = 0" 2000-11-28 3:10 ` kumon @ 2000-11-28 3:28 ` Andrea Arcangeli 2000-11-28 3:35 ` Alexander Viro 0 siblings, 1 reply; 23+ messages in thread From: Andrea Arcangeli @ 2000-11-28 3:28 UTC (permalink / raw) To: kumon; +Cc: linux-kernel On Tue, Nov 28, 2000 at 12:10:33PM +0900, kumon@flab.fujitsu.co.jp wrote: > If you have two files: > test1.c: > int a,b,c; > > test2.c: > int a,c; > > Which is _stronger_? Those won't link together as they aren't declared static. If they would been static they could be ordered file-per-file (note: I'm not suggesting anything like that and I'm more than happy the compiler is allowed to do sane optimizations with none downside :). Andrea - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] removal of "static foo = 0" 2000-11-28 3:28 ` Andrea Arcangeli @ 2000-11-28 3:35 ` Alexander Viro 2000-11-28 4:15 ` Michael Meissner 2000-11-28 9:55 ` Andreas Schwab 0 siblings, 2 replies; 23+ messages in thread From: Alexander Viro @ 2000-11-28 3:35 UTC (permalink / raw) To: Andrea Arcangeli; +Cc: kumon, linux-kernel On Tue, 28 Nov 2000, Andrea Arcangeli wrote: > On Tue, Nov 28, 2000 at 12:10:33PM +0900, kumon@flab.fujitsu.co.jp wrote: > > If you have two files: > > test1.c: > > int a,b,c; > > > > test2.c: > > int a,c; > > > > Which is _stronger_? > > Those won't link together as they aren't declared static. Try it. They _will_ link together. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] removal of "static foo = 0" 2000-11-28 3:35 ` Alexander Viro @ 2000-11-28 4:15 ` Michael Meissner 2000-11-28 9:55 ` Andreas Schwab 1 sibling, 0 replies; 23+ messages in thread From: Michael Meissner @ 2000-11-28 4:15 UTC (permalink / raw) To: Alexander Viro; +Cc: Andrea Arcangeli, kumon, linux-kernel On Mon, Nov 27, 2000 at 10:35:45PM -0500, Alexander Viro wrote: > > > On Tue, 28 Nov 2000, Andrea Arcangeli wrote: > > > On Tue, Nov 28, 2000 at 12:10:33PM +0900, kumon@flab.fujitsu.co.jp wrote: > > > If you have two files: > > > test1.c: > > > int a,b,c; > > > > > > test2.c: > > > int a,c; > > > > > > Which is _stronger_? > > > > Those won't link together as they aren't declared static. > > Try it. They _will_ link together. This is a GCC extension (actually it is a pretty common UNIX extension, but the official standard says you can only have one definition of a global variable). Off the top of my head, here are some reasons variables could be put in different orders: 1) The compilation system has the concept of a small data pointer, which is a register that is assumed by the compiler to point to a small region of memory (it is never allocated by the compiler and setup in the initialization modules). The compiler decides to put some variables into the small data region and other variables outside of it. Typically the choice is based on size of the variable. Small data pointers are typically used when the machine has plenty of registers and it takes 2 or more instructions to build the address of a random variable in memory with load high/load low type instructions, and the small data pointer has the upper half already loaded, and uses special relocations to access the variable based off of the difference of a special symbol. 2) Even without a small data pointer, a compiler might decide to sort the variables emitted based on either size or number of accesses to take advantage of instructions with smaller offsets. 3) The above mentioned global, non-initialized variables (ie, the so-called 'common' variables). Where the linker puts the variables into the bss section in any order it chooses. For example, the VMS linker used to sort common variables alphabetically. 4) For static variables, the compilation system might decide to omit the variable until it sees a reference to the variable, and if the first variable is referenced in one function, and the second is referenced several functions later. 5) At some point in the future, on machines with many more registers than the normal 32, the linker might see all references to a variable, and decide to put it in a static register rather than memory. 6) A checkout compiler could randomly order things specifically to catch these type of errors (the problem with the normal checkout compilers that I'm aware of, is that the kernel uses structs to talk to real devices and interact with system calls with fixed layouts). -- Michael Meissner, Red Hat, Inc. PMB 198, 174 Littleton Road #3, Westford, Massachusetts 01886, USA Work: meissner@redhat.com phone: +1 978-486-9304 Non-work: meissner@spectacle-pond.org fax: +1 978-692-4482 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] removal of "static foo = 0" 2000-11-28 3:35 ` Alexander Viro 2000-11-28 4:15 ` Michael Meissner @ 2000-11-28 9:55 ` Andreas Schwab 2000-11-28 15:16 ` Andrea Arcangeli 1 sibling, 1 reply; 23+ messages in thread From: Andreas Schwab @ 2000-11-28 9:55 UTC (permalink / raw) To: Alexander Viro; +Cc: Andrea Arcangeli, kumon, linux-kernel Alexander Viro <viro@math.psu.edu> writes: |> On Tue, 28 Nov 2000, Andrea Arcangeli wrote: |> |> > On Tue, Nov 28, 2000 at 12:10:33PM +0900, kumon@flab.fujitsu.co.jp wrote: |> > > If you have two files: |> > > test1.c: |> > > int a,b,c; |> > > |> > > test2.c: |> > > int a,c; |> > > |> > > Which is _stronger_? |> > |> > Those won't link together as they aren't declared static. |> |> Try it. They _will_ link together. Not if you compile with -fno-common, which should actually be the default some day, IMHO. Andreas. -- Andreas Schwab "And now for something SuSE Labs completely different." Andreas.Schwab@suse.de SuSE GmbH, Schanzäckerstr. 10, D-90443 Nürnberg - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] removal of "static foo = 0" 2000-11-28 9:55 ` Andreas Schwab @ 2000-11-28 15:16 ` Andrea Arcangeli 2000-11-28 16:09 ` Andreas Schwab 2000-11-28 16:44 ` Michael Meissner 0 siblings, 2 replies; 23+ messages in thread From: Andrea Arcangeli @ 2000-11-28 15:16 UTC (permalink / raw) To: Andreas Schwab; +Cc: Alexander Viro, kumon, linux-kernel On Tue, Nov 28, 2000 at 10:55:06AM +0100, Andreas Schwab wrote: > Alexander Viro <viro@math.psu.edu> writes: > > |> On Tue, 28 Nov 2000, Andrea Arcangeli wrote: > |> > |> > On Tue, Nov 28, 2000 at 12:10:33PM +0900, kumon@flab.fujitsu.co.jp wrote: > |> > > If you have two files: > |> > > test1.c: > |> > > int a,b,c; > |> > > > |> > > test2.c: > |> > > int a,c; > |> > > > |> > > Which is _stronger_? > |> > > |> > Those won't link together as they aren't declared static. > |> > |> Try it. They _will_ link together. > > Not if you compile with -fno-common, which should actually be the default > some day, IMHO. I thought -fno-common was the default behaviour indeed, and I agree it should become the default since current behaviour can lead to sublte bugs. (better I discovered this gcc "extension" this way than after some day of debugging :) I'm all for gcc extensions when they're powerful and useful, but this one looks absolutely worthless. I don't see any advantage from the current behaviour (avoid an "extern" in some include file that we have/want to write anyways to write correct C code?), and at least in large project (like the kernel) where different part of the project are handled by different people using -fno-common is pretty much mandatory IMHO. Think at somebody writing a driver starting from another driver, maybe he renames most of the stuff but he forgets to rename an uninitialized global variable. This bug won't trigger for him because he's not using the other driver at the same time. It will trigger only when an unlucky user will happen to use both drivers at the same time because he owns both hardwares... I disagree with GCC documentation: `-fno-common' Allocate even uninitialized global variables in the bss section of the object file, rather than generating them as common blocks. This has the effect that if the same variable is declared (without `extern') in two different compilations, you will get an error when you link them. The only reason this might be useful is if ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ you wish to verify that the program will work on other systems ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ which always work this way. ^^^^^^^^^^^^^^^^^^^^^^^^^^ That's one reason, but it's really not the interesting one... Andrea - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] removal of "static foo = 0" 2000-11-28 15:16 ` Andrea Arcangeli @ 2000-11-28 16:09 ` Andreas Schwab 2000-11-28 19:29 ` Andrea Arcangeli 2000-11-28 16:44 ` Michael Meissner 1 sibling, 1 reply; 23+ messages in thread From: Andreas Schwab @ 2000-11-28 16:09 UTC (permalink / raw) To: Andrea Arcangeli; +Cc: Alexander Viro, kumon, linux-kernel Andrea Arcangeli <andrea@suse.de> writes: |> On Tue, Nov 28, 2000 at 10:55:06AM +0100, Andreas Schwab wrote: |> > Alexander Viro <viro@math.psu.edu> writes: |> > |> > |> On Tue, 28 Nov 2000, Andrea Arcangeli wrote: |> > |> |> > |> > On Tue, Nov 28, 2000 at 12:10:33PM +0900, kumon@flab.fujitsu.co.jp wrote: |> > |> > > If you have two files: |> > |> > > test1.c: |> > |> > > int a,b,c; |> > |> > > |> > |> > > test2.c: |> > |> > > int a,c; |> > |> > > |> > |> > > Which is _stronger_? |> > |> > |> > |> > Those won't link together as they aren't declared static. |> > |> |> > |> Try it. They _will_ link together. |> > |> > Not if you compile with -fno-common, which should actually be the default |> > some day, IMHO. |> |> I thought -fno-common was the default behaviour indeed, and I agree it should |> become the default since current behaviour can lead to sublte bugs. (better I |> discovered this gcc "extension" this way than after some day of debugging :) This is not really a gcc extension, but long Unix tradition. If you make -fno-common the default then many programs will not build any more, including the Linux kernel. :-) Andreas. -- Andreas Schwab "And now for something SuSE Labs completely different." Andreas.Schwab@suse.de SuSE GmbH, Schanzäckerstr. 10, D-90443 Nürnberg - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] removal of "static foo = 0" 2000-11-28 16:09 ` Andreas Schwab @ 2000-11-28 19:29 ` Andrea Arcangeli 0 siblings, 0 replies; 23+ messages in thread From: Andrea Arcangeli @ 2000-11-28 19:29 UTC (permalink / raw) To: Andreas Schwab, Linus Torvalds, Karsten Keil Cc: Alexander Viro, kumon, linux-kernel On Tue, Nov 28, 2000 at 05:09:48PM +0100, Andreas Schwab wrote: > including the Linux kernel. :-) As it's a worthless extension it's always trivial to fixup after its removal :). The fixup also shown that the sis_300 and sis_301 driver would break if used at the same time (probably unlikely to happen as they're FB drivers though). This patch compiles 2.4.0-test12-pre2 with -fno-common and it fixups some minor compilation problem around the kernel. Karsten note the lc_start_delay_check change I did to make it to compile, it's not implemented yet, it only compiles right now. Patch is verified to compile with almost everything linked into the kernel, and it boots with my normal configuration. --- 2.4.0-test12-pre2-fno-common/drivers/char/applicom.c.~1~ Thu Jul 13 06:58:42 2000 +++ 2.4.0-test12-pre2-fno-common/drivers/char/applicom.c Tue Nov 28 19:07:05 2000 @@ -63,8 +63,8 @@ #define PCI_DEVICE_ID_APPLICOM_PCIGENERIC 0x0001 #define PCI_DEVICE_ID_APPLICOM_PCI2000IBS_CAN 0x0002 #define PCI_DEVICE_ID_APPLICOM_PCI2000PFB 0x0003 -#define MAX_PCI_DEVICE_NUM 3 #endif +#define MAX_PCI_DEVICE_NUM 3 static char *applicom_pci_devnames[] = { "PCI board", --- 2.4.0-test12-pre2-fno-common/drivers/isdn/hisax/isdnl3.c.~1~ Tue Nov 28 18:40:29 2000 +++ 2.4.0-test12-pre2-fno-common/drivers/isdn/hisax/isdnl3.c Tue Nov 28 19:46:08 2000 @@ -522,6 +522,11 @@ l3ml3p(st, DL_RELEASE | CONFIRM); } +static void +lc_start_delay_check(struct FsmInst *fi, int event, void *arg) +{ + /* FIXME */ +} /* *INDENT-OFF* */ static struct FsmNode L3FnList[] __initdata = --- 2.4.0-test12-pre2-fno-common/drivers/isdn/hisax/nj_s.c.~1~ Tue Nov 28 18:40:29 2000 +++ 2.4.0-test12-pre2-fno-common/drivers/isdn/hisax/nj_s.c Tue Nov 28 19:47:21 2000 @@ -75,7 +75,7 @@ (cs->hw.njet.last_is0 & NETJET_IRQM0_READ)) /* we have a read dma int */ read_tiger(cs); - if (cs->hw.njet.irqstat0 & NETJET_IRQM0_WRITE) != + if ((cs->hw.njet.irqstat0 & NETJET_IRQM0_WRITE) != (cs->hw.njet.last_is0 & NETJET_IRQM0_WRITE)) /* we have a write dma int */ write_tiger(cs); --- 2.4.0-test12-pre2-fno-common/drivers/isdn/hisax/nj_u.c.~1~ Tue Nov 28 18:40:29 2000 +++ 2.4.0-test12-pre2-fno-common/drivers/isdn/hisax/nj_u.c Tue Nov 28 19:48:52 2000 @@ -11,6 +11,7 @@ #include <linux/pci.h> #include <linux/interrupt.h> #include <linux/ppp_defs.h> +#include <linux/init.h> #include "netjet.h" const char *NETjet_U_revision = "$Revision: 2.8 $"; @@ -131,7 +132,7 @@ return(0); } -static struct pci_dev *dev_netjet __initdata; +static struct pci_dev *dev_netjet __initdata = 0; int __init setup_netjet_u(struct IsdnCard *card) --- 2.4.0-test12-pre2-fno-common/drivers/isdn/hisax/config.c.~1~ Tue Nov 28 18:40:29 2000 +++ 2.4.0-test12-pre2-fno-common/drivers/isdn/hisax/config.c Tue Nov 28 20:04:59 2000 @@ -376,7 +376,7 @@ #endif /* IO0_IO1 */ #endif /* MODULE */ -static int nrcards; +int nrcards; extern char *l1_revision; extern char *l2_revision; --- 2.4.0-test12-pre2-fno-common/drivers/media/video/bttv.h.~1~ Tue Nov 28 18:50:21 2000 +++ 2.4.0-test12-pre2-fno-common/drivers/media/video/bttv.h Tue Nov 28 18:51:19 2000 @@ -179,9 +179,9 @@ /* i2c */ #define I2C_CLIENTS_MAX 8 -struct i2c_algo_bit_data bttv_i2c_algo_template; -struct i2c_adapter bttv_i2c_adap_template; -struct i2c_client bttv_i2c_client_template; +extern struct i2c_algo_bit_data bttv_i2c_algo_template; +extern struct i2c_adapter bttv_i2c_adap_template; +extern struct i2c_client bttv_i2c_client_template; void bttv_bit_setscl(void *data, int state); void bttv_bit_setsda(void *data, int state); void bttv_call_i2c_clients(struct bttv *btv, unsigned int cmd, void *arg); --- 2.4.0-test12-pre2-fno-common/drivers/net/arlan.h.~1~ Tue Nov 28 19:10:06 2000 +++ 2.4.0-test12-pre2-fno-common/drivers/net/arlan.h Tue Nov 28 19:11:44 2000 @@ -321,7 +321,7 @@ int tx_queue_len; }; -struct arlan_conf_stru arlan_conf[MAX_ARLANS]; +extern struct arlan_conf_stru arlan_conf[MAX_ARLANS]; struct TxParam { --- 2.4.0-test12-pre2-fno-common/drivers/usb/storage/usb.h.~1~ Tue Nov 28 19:25:37 2000 +++ 2.4.0-test12-pre2-fno-common/drivers/usb/storage/usb.h Tue Nov 28 19:40:11 2000 @@ -187,7 +187,7 @@ extern struct semaphore us_list_semaphore; /* The structure which defines our driver */ -struct usb_driver usb_storage_driver; +extern struct usb_driver usb_storage_driver; /* Function to fill an inquiry response. See usb.c for details */ extern void fill_inquiry_response(struct us_data *us, --- 2.4.0-test12-pre2-fno-common/drivers/video/sis/sis_300.h.~1~ Tue Nov 28 18:40:01 2000 +++ 2.4.0-test12-pre2-fno-common/drivers/video/sis/sis_300.h Tue Nov 28 19:28:42 2000 @@ -95,15 +95,6 @@ #endif -USHORT P3c4,P3d4,P3c0,P3ce,P3c2,P3ca,P3c6,P3c7,P3c8,P3c9,P3da; -USHORT CRT1VCLKLen; //VCLKData table length of bytes of each entry -USHORT flag_clearbuffer; //0: no clear frame buffer 1:clear frame buffer -int RAMType; -int ModeIDOffset,StandTable,CRT1Table,ScreenOffset,VCLKData,MCLKData, ECLKData; -int REFIndex,ModeType; -USHORT IF_DEF_LVDS,IF_DEF_TRUMPION; -USHORT VBInfo,LCDResInfo,LCDTypeInfo,LCDInfo; - //int init300(int,int,int); VOID SetMemoryClock(ULONG); VOID SetDRAMSize(PHW_DEVICE_EXTENSION); --- 2.4.0-test12-pre2-fno-common/drivers/video/sis/sis_301.c.~1~ Tue Nov 28 18:40:01 2000 +++ 2.4.0-test12-pre2-fno-common/drivers/video/sis/sis_301.c Tue Nov 28 19:39:25 2000 @@ -3,6 +3,14 @@ #include <linux/config.h> #include "sis_301.h" +static USHORT P3c4,P3d4,P3c0,P3ce,P3c2,P3ca,P3c6,P3c7,P3c8,P3c9,P3da; +static USHORT flag_clearbuffer; //0:no clear frame buffer 1:clear frame buffer +static int RAMType; +static int ModeIDOffset,StandTable,CRT1Table,ScreenOffset,VCLKData,MCLKData, ECLKData; +static int REFIndex,ModeType; +static USHORT VBInfo,LCDResInfo,LCDTypeInfo,LCDInfo; +static USHORT IF_DEF_LVDS,IF_DEF_TRUMPION; + #ifndef CONFIG_FB_SIS_LINUXBIOS BOOLEAN SetCRT2Group(USHORT BaseAddr,ULONG ROMAddr,USHORT ModeNo, --- 2.4.0-test12-pre2-fno-common/drivers/video/sis/sis_300.c.~1~ Tue Nov 28 18:40:01 2000 +++ 2.4.0-test12-pre2-fno-common/drivers/video/sis/sis_300.c Tue Nov 28 19:29:04 2000 @@ -7,6 +7,14 @@ #pragma alloc_text(PAGE,SiSInit300) #endif +static USHORT P3c4,P3d4,P3c0,P3ce,P3c2,P3ca,P3c6,P3c7,P3c8,P3c9,P3da; +static USHORT CRT1VCLKLen; //VCLKData table length of bytes of each entry +static USHORT flag_clearbuffer; //0: no clear frame buffer 1:clear frame buffer +static int RAMType; +static int ModeIDOffset,StandTable,CRT1Table,ScreenOffset,VCLKData,MCLKData, ECLKData; +static int REFIndex,ModeType; +static USHORT IF_DEF_LVDS,IF_DEF_TRUMPION; +static USHORT VBInfo,LCDResInfo,LCDTypeInfo,LCDInfo; #ifdef NOBIOS BOOLEAN SiSInit300(PHW_DEVICE_EXTENSION HwDeviceExtension) --- 2.4.0-test12-pre2-fno-common/drivers/video/sis/sis_301.h.~1~ Tue Nov 28 18:40:01 2000 +++ 2.4.0-test12-pre2-fno-common/drivers/video/sis/sis_301.h Tue Nov 28 19:31:11 2000 @@ -3,7 +3,6 @@ USHORT SetFlag,RVBHCFACT,RVBHCMAX,VGAVT,VGAHT,VT,HT,VGAVDE,VGAHDE; USHORT VDE,HDE,RVBHRS,NewFlickerMode,RY1COE,RY2COE,RY3COE,RY4COE; -;USHORT LCDResInfo,LCDTypeInfo,LCDInfo; USHORT VCLKLen; USHORT LCDHDES,LCDVDES; @@ -180,14 +179,6 @@ extern USHORT CGA_DAC[]; extern USHORT EGA_DAC[]; extern USHORT VGA_DAC[]; - -extern USHORT P3c4,P3d4,P3c0,P3ce,P3c2,P3ca,P3c6,P3c7,P3c8,P3c9,P3da; -extern USHORT flag_clearbuffer; //0:no clear frame buffer 1:clear frame buffer -extern int RAMType; -extern int ModeIDOffset,StandTable,CRT1Table,ScreenOffset,VCLKData,MCLKData, ECLKData; -extern int REFIndex,ModeType; -extern USHORT VBInfo,LCDResInfo,LCDTypeInfo,LCDInfo; -extern USHORT IF_DEF_LVDS,IF_DEF_TRUMPION; extern VOID SetMemoryClock(ULONG); extern VOID SetDRAMSize(PHW_DEVICE_EXTENSION); --- 2.4.0-test12-pre2-fno-common/include/linux/vt_kern.h.~1~ Tue Nov 28 18:44:22 2000 +++ 2.4.0-test12-pre2-fno-common/include/linux/vt_kern.h Tue Nov 28 18:46:26 2000 @@ -30,7 +30,7 @@ wait_queue_head_t paste_wait; } *vt_cons[MAX_NR_CONSOLES]; -void (*kd_mksound)(unsigned int hz, unsigned int ticks); +extern void (*kd_mksound)(unsigned int hz, unsigned int ticks); /* console.c */ --- 2.4.0-test12-pre2-fno-common/include/linux/if_frad.h.~1~ Tue Nov 28 19:08:18 2000 +++ 2.4.0-test12-pre2-fno-common/include/linux/if_frad.h Tue Nov 28 19:09:01 2000 @@ -192,7 +192,7 @@ int register_frad(const char *name); int unregister_frad(const char *name); -int (*dlci_ioctl_hook)(unsigned int, void *); +extern int (*dlci_ioctl_hook)(unsigned int, void *); #endif __KERNEL__ --- 2.4.0-test12-pre2-fno-common/net/atm/lec.h.~1~ Tue Nov 28 20:01:59 2000 +++ 2.4.0-test12-pre2-fno-common/net/atm/lec.h Tue Nov 28 20:02:45 2000 @@ -16,9 +16,9 @@ #if defined (CONFIG_BRIDGE) || defined(CONFIG_BRIDGE_MODULE) #include <linux/if_bridge.h> -struct net_bridge_fdb_entry *(*br_fdb_get_hook)(struct net_bridge *br, +extern struct net_bridge_fdb_entry *(*br_fdb_get_hook)(struct net_bridge *br, unsigned char *addr); -void (*br_fdb_put_hook)(struct net_bridge_fdb_entry *ent); +extern void (*br_fdb_put_hook)(struct net_bridge_fdb_entry *ent); #endif /* defined(CONFIG_BRIDGE) || defined(CONFIG_BRIDGE_MODULE) */ #define LEC_HEADER_LEN 16 --- 2.4.0-test12-pre2-fno-common/net/bridge/br_private.h.~1~ Tue Nov 28 19:57:02 2000 +++ 2.4.0-test12-pre2-fno-common/net/bridge/br_private.h Tue Nov 28 19:59:43 2000 @@ -112,8 +112,8 @@ int gc_interval; }; -struct notifier_block br_device_notifier; -unsigned char bridge_ula[6]; +extern struct notifier_block br_device_notifier; +extern unsigned char bridge_ula[6]; /* br.c */ void br_dec_use_count(void); --- 2.4.0-test12-pre2-fno-common/Makefile.~1~ Tue Nov 28 18:40:28 2000 +++ 2.4.0-test12-pre2-fno-common/Makefile Tue Nov 28 18:42:50 2000 @@ -16,7 +16,7 @@ FINDHPATH = $(HPATH)/asm $(HPATH)/linux $(HPATH)/scsi $(HPATH)/net HOSTCC = gcc -HOSTCFLAGS = -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer +HOSTCFLAGS = -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer -fno-common CROSS_COMPILE = @@ -87,7 +87,7 @@ CPPFLAGS := -D__KERNEL__ -I$(HPATH) -CFLAGS := $(CPPFLAGS) -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer -fno-strict-aliasing +CFLAGS := $(CPPFLAGS) -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer -fno-strict-aliasing -fno-common AFLAGS := -D__ASSEMBLY__ $(CPPFLAGS) # I'd suggest to include it into 2.4 until -fno-common will become the default in gcc. Patch is here too: ftp://ftp.us.kernel.org/pub/linux/kernel/people/andrea/patches/v2.4/2.4.0-test12-pre2/gcc-fno-common-1 Andrea - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] removal of "static foo = 0" 2000-11-28 15:16 ` Andrea Arcangeli 2000-11-28 16:09 ` Andreas Schwab @ 2000-11-28 16:44 ` Michael Meissner 1 sibling, 0 replies; 23+ messages in thread From: Michael Meissner @ 2000-11-28 16:44 UTC (permalink / raw) To: Andrea Arcangeli; +Cc: Andreas Schwab, Alexander Viro, kumon, linux-kernel On Tue, Nov 28, 2000 at 04:16:12PM +0100, Andrea Arcangeli wrote: > On Tue, Nov 28, 2000 at 10:55:06AM +0100, Andreas Schwab wrote: > > Alexander Viro <viro@math.psu.edu> writes: > > > > |> On Tue, 28 Nov 2000, Andrea Arcangeli wrote: > > |> > > |> > On Tue, Nov 28, 2000 at 12:10:33PM +0900, kumon@flab.fujitsu.co.jp wrote: > > |> > > If you have two files: > > |> > > test1.c: > > |> > > int a,b,c; > > |> > > > > |> > > test2.c: > > |> > > int a,c; > > |> > > > > |> > > Which is _stronger_? > > |> > > > |> > Those won't link together as they aren't declared static. > > |> > > |> Try it. They _will_ link together. > > > > Not if you compile with -fno-common, which should actually be the default > > some day, IMHO. > > I thought -fno-common was the default behaviour indeed, and I agree it should > become the default since current behaviour can lead to sublte bugs. (better I > discovered this gcc "extension" this way than after some day of debugging :) > > I'm all for gcc extensions when they're powerful and useful, but this > one looks absolutely worthless. I don't see any advantage from the current > behaviour (avoid an "extern" in some include file that we have/want to write > anyways to write correct C code?), and at least in large project (like the > kernel) where different part of the project are handled by different people > using -fno-common is pretty much mandatory IMHO. Umm, the original Ritchie C compiler on the PDP-11 and the Johnson 'Portable' C compiler both had this extension, as well as every other C compiler under UNIX (tm) I've ever used or read the documentation for. It is also mentioned in the standards as a common extension (I wrote the initial draft for this section in the C89 standard). When asked why the disconnect between what K&R said (ref/def model) and what their compilers actually did (common model), the AT&T representative at the time said that the ref/def model was put into K&R when they tried to make a C compiler for their IBM mainframes, using the standard linker, and discovered that linker would page align each common variable (CSECT in IBM terminology IIRC). The IBM linker is also the reason why the K&R and the C89 standard had the rule that global names must be unique to 6 characters, one case (which is another extension that just about everybody has and depends on). The default for C++ is -fno-common. -- Michael Meissner, Red Hat, Inc. PMB 198, 174 Littleton Road #3, Westford, Massachusetts 01886, USA Work: meissner@redhat.com phone: +1 978-486-9304 Non-work: meissner@spectacle-pond.org fax: +1 978-692-4482 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] removal of "static foo = 0" 2000-11-27 17:36 ` Michael Meissner 2000-11-27 19:06 ` Andrea Arcangeli @ 2000-11-27 21:27 ` Marcus Sundberg 2000-11-28 0:49 ` real_root_dev Andries Brouwer 1 sibling, 1 reply; 23+ messages in thread From: Marcus Sundberg @ 2000-11-27 21:27 UTC (permalink / raw) To: linux-kernel meissner@spectacle-pond.org (Michael Meissner) writes: > On Mon, Nov 27, 2000 at 06:21:13PM +0100, Andrea Arcangeli wrote: > > On Mon, Nov 27, 2000 at 12:39:55AM -0800, David S. Miller wrote: > > > Also I believe linkers are allowed to arbitrarily reorder members in > > > the data and bss sections. I could be wrong on this one though. > > > > I'm not sure either, but we certainly rely on that behaviour somewhere. > > Just to make an example fs/dquot.c: > > > > int nr_dquots, nr_free_dquots; > > > > kernel/sysctl.c: > > > > {FS_NRDQUOT, "dquot-nr", &nr_dquots, 2*sizeof(int), > > > > The above is ok also on mips in practice though. > > That needs to be fixed ASAP to use an array (not a structure). It is simply > wrong to depend on two variables winding up in at adjacent offsets. This reminded me of an old bug which apparently still hasn't been fixed (not in 2.2 at least). In init/main.c we have: extern int rd_image_start; /* starting block # of image */ #ifdef CONFIG_BLK_DEV_INITRD kdev_t real_root_dev; #endif #endif int root_mountflags = MS_RDONLY; and then in kernel/sysctl.c: #ifdef CONFIG_BLK_DEV_INITRD {KERN_REALROOTDEV, "real-root-dev", &real_root_dev, sizeof(int), 0644, NULL, &proc_dointvec}, #endif Because rd_image_start and root_mountflags are both int-aligned, this happens to work on little endian platforms. On big endian platforms however writing a value in the range 0-65535 to /proc/sys/kernel/real-root-dev will place 0 in real_root_dev, and the actual value in the two padding bytes... (Yes, I'm one of the few that have actually used this feature. ;-) Unfortunately proc_dointvec() doesn't support shorts, so what is the correct fix? Changing: kdev_t real_root_dev; into int real_root-dev; is a perfectly working solution, but is it acceptable? //Marcus -- -------------------------------+----------------------------------- Marcus Sundberg | Phone: +46 707 452062 Embedded Systems Consultant | Email: marcus@cendio.se Cendio Systems AB | http://www.cendio.com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: real_root_dev 2000-11-27 21:27 ` Marcus Sundberg @ 2000-11-28 0:49 ` Andries Brouwer 0 siblings, 0 replies; 23+ messages in thread From: Andries Brouwer @ 2000-11-28 0:49 UTC (permalink / raw) To: Marcus Sundberg; +Cc: linux-kernel On Mon, Nov 27, 2000 at 10:27:00PM +0100, Marcus Sundberg wrote: > This reminded me of an old bug which apparently still hasn't been > fixed (not in 2.2 at least). In init/main.c we have: > > extern int rd_image_start; /* starting block # of image */ > #ifdef CONFIG_BLK_DEV_INITRD > kdev_t real_root_dev; > #endif > #endif > > int root_mountflags = MS_RDONLY; > > and then in kernel/sysctl.c: > > #ifdef CONFIG_BLK_DEV_INITRD > {KERN_REALROOTDEV, "real-root-dev", &real_root_dev, sizeof(int), > 0644, NULL, &proc_dointvec}, > #endif > > Because rd_image_start and root_mountflags are both int-aligned, > this happens to work on little endian platforms. On big endian > platforms however writing a value in the range 0-65535 to > /proc/sys/kernel/real-root-dev will place 0 in real_root_dev, > and the actual value in the two padding bytes... > > Unfortunately proc_dointvec() doesn't support shorts, so what is > the correct fix? Changing: > kdev_t real_root_dev; > into > int real_root-dev; > is a perfectly working solution, but is it acceptable? If you compile the kernel and use an integral type for kdev_t, perhaps. On the other hand, I usually use a pointer type for kdev_t, and then this entire sysctl construction is broken. Andries - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] removal of "static foo = 0" 2000-11-27 17:21 ` Andrea Arcangeli 2000-11-27 17:36 ` Michael Meissner @ 2000-11-27 18:11 ` Richard B. Johnson 1 sibling, 0 replies; 23+ messages in thread From: Richard B. Johnson @ 2000-11-27 18:11 UTC (permalink / raw) To: Andrea Arcangeli; +Cc: David S. Miller, Werner.Almesberger, adam, linux-kernel On Mon, 27 Nov 2000, Andrea Arcangeli wrote: > On Mon, Nov 27, 2000 at 12:39:55AM -0800, David S. Miller wrote: > > Also I believe linkers are allowed to arbitrarily reorder members in > > the data and bss sections. I could be wrong on this one though. > > I'm not sure either, but we certainly rely on that behaviour somewhere. > Just to make an example fs/dquot.c: > > int nr_dquots, nr_free_dquots; > > kernel/sysctl.c: > > {FS_NRDQUOT, "dquot-nr", &nr_dquots, 2*sizeof(int), > > The above is ok also on mips in practice though. > This code is simply wrong! You can't assume that the declaration of two variables, no matter how similar, makes them adjacent in memory! You also can't assume any order. We have good 'C' compilers that do order things as we assume, however it is only fortuitous, and not defined any any rule(s). Cheers, Dick Johnson Penguin : Linux version 2.4.0 on an i686 machine (799.54 BogoMips). "Memory is like gasoline. You use it up when you are running. Of course you get it all back when you reboot..."; Actual explanation obtained from the Micro$oft help desk. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] removal of "static foo = 0" 2000-11-27 8:41 ` Werner Almesberger 2000-11-27 8:39 ` David S. Miller @ 2000-11-27 18:01 ` Michael Meissner 1 sibling, 0 replies; 23+ messages in thread From: Michael Meissner @ 2000-11-27 18:01 UTC (permalink / raw) To: Werner Almesberger; +Cc: Adam J. Richter, linux-kernel On Mon, Nov 27, 2000 at 09:41:39AM +0100, Werner Almesberger wrote: > Adam J. Richter wrote: > > At the moment, I have started daydreaming about instead > > writing an "elf squeezer" to do this and other space optimizations > > by modifying objdump. > > Hmm, this would require that gcc never calculates the location of an > explicitly initialized static variable based on the address of another > one. E.g. in > > static int a = 0, b = 0, c = 0, d = 0; > > ... > ... a+b+c+d ... > ... > > egcs-2.91.66 indeed doesn't seem to make this optimization on i386. > (Maybe the pointer increment or pointer offset solution would > actually be slower - didn't check.) But I'm not sure if this is also > true for other versions/architectures, or other code constructs. Jeff Law played with a similar optimization about 1-2 years ago, and eventually deleted all of the code because there were problems with the code. It would help some of our platforms (but not the x86) to access all variables in the same section via a common pointer. This is because on those systems, it often times takes 2-3 instructions to access global/static variables. Global variables you have more problems visiblity and such. -- Michael Meissner, Red Hat, Inc. PMB 198, 174 Littleton Road #3, Westford, Massachusetts 01886, USA Work: meissner@redhat.com phone: +1 978-486-9304 Non-work: meissner@spectacle-pond.org fax: +1 978-692-4482 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2000-11-28 19:59 UTC | newest] Thread overview: 23+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2000-11-27 5:56 [PATCH] removal of "static foo = 0" Adam J. Richter 2000-11-27 8:41 ` Werner Almesberger 2000-11-27 8:39 ` David S. Miller 2000-11-27 9:08 ` Werner Almesberger 2000-11-27 17:21 ` Andrea Arcangeli 2000-11-27 17:36 ` Michael Meissner 2000-11-27 19:06 ` Andrea Arcangeli 2000-11-27 19:34 ` Richard B. Johnson 2000-11-28 0:28 ` Andrea Arcangeli 2000-11-28 11:25 ` Horst von Brand 2000-11-28 3:10 ` kumon 2000-11-28 3:28 ` Andrea Arcangeli 2000-11-28 3:35 ` Alexander Viro 2000-11-28 4:15 ` Michael Meissner 2000-11-28 9:55 ` Andreas Schwab 2000-11-28 15:16 ` Andrea Arcangeli 2000-11-28 16:09 ` Andreas Schwab 2000-11-28 19:29 ` Andrea Arcangeli 2000-11-28 16:44 ` Michael Meissner 2000-11-27 21:27 ` Marcus Sundberg 2000-11-28 0:49 ` real_root_dev Andries Brouwer 2000-11-27 18:11 ` [PATCH] removal of "static foo = 0" Richard B. Johnson 2000-11-27 18:01 ` Michael Meissner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox