Another problem with making things static

First of all thanks to Andrew for forwarding this email to me - it's otherwise a bit hard to defend against an email that doesn't reach me.

Quoted 18 lines

...

We're going to need this one reverting for parisc. If you remember we've been trying to push a compat_sys_rt_sigqueueinfo but Andi keeps objecting, so it's remained local to our tree (an hence so has the symbol usage).

However, can we *please* stop this indiscriminate rampage through all the sybsystems making things static just because we can. The true test of whether a symbol should be static or not is whether it forms part of a sensible API or not. For subsystems with a well defined API, this is quite easy (although it hasn't prevented attempts to make even those symbols static). For some of our core subsystems (like the kernel API in this case) it's much less well defined ... although we have a good solid white area of things that must be part of the API and a good solid black area of things that mustn't, we still have a large grey area which things like kill_proc_info() fall into.

Quoted 5 lines

For any person who wants to make a static symbol exported, we force an explanation out of them about what they're trying to do and why. Might I suggest we apply the same standard to anyone trying to make something static? i.e. they need to explain clearly why the symbol shouldn't be part of any rational API.

And by how many percent will this bloat the kernel more in the long term?

Quoted 26 lines

Thanks,

James
Date: Thu, 8 Feb 2007 22:33:21 -0500
From: Kyle McMartin <[email protected]>
To: Matthew Wilcox <[email protected]>
Cc: Kyle McMartin <[email protected]>,
parisc-linux <[email protected]>
Subject: Re: [parisc-linux] 64-bit kernel broken.

Quoted 11 lines · Matthew Wilcox

Quoted 4 lines · Kyle McMartin

Quoted 2 lines

Right. This got inadvertantly reverted with some other stuff. Fixed in a second.

Fixed in 1736b4bbf2bbfebaad06114d569f3617517524d1.

make.err:/home/willy/nicol-2.6/arch/parisc/kernel/signal32.c:519:
warning: implicit declaration of function 'kill_proc_info' make.err:(.text.compat_sys_rt_sigqueueinfo+0x80): undefined reference to `kill_proc_info'

Guess who came home for Christmas

commit d3228a887cae75ef2b8b1211c31c539bef5a5698

Quoted 8 lines

Author: Adrian Bunk <[email protected]>
Date:   Wed Dec 6 20:38:22 2006 -0800

[PATCH] make kernel/signal.c:kill_proc_info() static

Signed-off-by Adrian Bunk <[email protected]>

Signed-off-by Andrew Morton <[email protected]>

Signed-off-by Linus Torvalds <[email protected]>

There are hundreds of such patches of mine that got included in Linus' tree and that didn't cause problems. Besides making the kernel smaller, they regularly find "it should have been used" bugs.

In this one case, a patch that made something static that wasn't used from other files during the over 9 years of it's existence clashes with another patch that adds a user.

Can we agree that it's not a usual case that the first user gets added after more than 9 years?

And making it global again when a user gets included isn't really that hard.

The life of a non-x86 architecture maintainer is hard ... it involves having lots of volunteers tirelessly chasing down things that broke during the big two week merge frenzy.

Quoted 3 lines · On Sat, 10 Feb 2007 19:47:16 -0600 James Bottomley…

The life of a non-x86 architecture maintainer is hard ... it involves having lots of volunteers tirelessly chasing down things that broke during the big two week merge frenzy.

They should test -mm kernels and report any problems so those problems don't get into mainline.

My attempt to build a working parisc cross-compiler failed, which doesn't help.

_______________________________________________
parisc-linux mailing list
[email protected]
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

Quoted 1 line

My attempt to build a working parisc cross-compiler failed, which doesn't help.

I appreciate your attempt to build parisc-linux.
Could you post which source tree and which build command line you used?

I'm pretty sure this can be fixed.
We've used cross compilers in the past so I know it's worked before.

I say this a lot, but... It is in the interests of arch maintainers to help others to build cross-compilers. If someone were to prepare a web page (or even a script) which could be used to generate a kernel cross-compilation environment for parisc then the parisc maintainers would see a lot less breakage.

Quoted 5 lines · Andrew Morton

I say this a lot, but... It is in the interests of arch maintainers to help others to build cross-compilers. If someone were to prepare a web page (or even a script) which could be used to generate a kernel cross-compilation environment for parisc then the parisc maintainers would see a lot less breakage.

Agreed.
We have such a page:
http://www.parisc-linux.org/toolchain/PARISC-Linux-XC-HOWTO.html

(link "Build XC" from www.p-l.o homepage)

Quoted 6 lines · Andrew Morton

I say this a lot, but... It is in the interests of arch maintainers to help others to build cross-compilers. If someone were to prepare a web page (or even a script) which could be used to generate a kernel cross-compilation environment for parisc then the parisc maintainers would see a lot less breakage.

Heh, I put this up last time you asked. :)

http://kyle.mcmartin.ca/hppa64-hp-linux-gcc-3.4.tar

Quoted 2 lines · Adrian Bunk

First of all thanks to Andrew for forwarding this email to me - it's otherwise a bit hard to defend against an email that doesn't reach me.

I think if you read the email you'll find it didn't actually attack you at all ... I was careful only to ask for a change of policy before accepting a patch that does nothing but make a function static.

Quoted 24 lines

Quoted 23 lines

...

We're going to need this one reverting for parisc. If you remember we've been trying to push a compat_sys_rt_sigqueueinfo but Andi keeps objecting, so it's remained local to our tree (an hence so has the symbol usage).

However, can we *please* stop this indiscriminate rampage through all the sybsystems making things static just because we can. The true test of whether a symbol should be static or not is whether it forms part of a sensible API or not. For subsystems with a well defined API, this is quite easy (although it hasn't prevented attempts to make even those symbols static). For some of our core subsystems (like the kernel API in this case) it's much less well defined ... although we have a good solid white area of things that must be part of the API and a good solid black area of things that mustn't, we still have a large grey area which things like kill_proc_info() fall into.

For any person who wants to make a static symbol exported, we force an explanation out of them about what they're trying to do and why. Might I suggest we apply the same standard to anyone trying to make something static? i.e. they need to explain clearly why the symbol shouldn't be part of any rational API.

Quoted 2 lines

And by how many percent will this bloat the kernel more in the long term?

That's something I'm more than willing to live with if the tradeoff is we think before making things static.

Quoted 35 lines

Quoted 26 lines

Thanks,

James
Date: Thu, 8 Feb 2007 22:33:21 -0500
From: Kyle McMartin <[email protected]>
To: Matthew Wilcox <[email protected]>
Cc: Kyle McMartin <[email protected]>,
parisc-linux <[email protected]>
Subject: Re: [parisc-linux] 64-bit kernel broken.

Quoted 11 lines · Matthew Wilcox

Quoted 4 lines · Kyle McMartin

Quoted 2 lines

Right. This got inadvertantly reverted with some other stuff. Fixed in a second.

Fixed in 1736b4bbf2bbfebaad06114d569f3617517524d1.

make.err:/home/willy/nicol-2.6/arch/parisc/kernel/signal32.c:519:
warning: implicit declaration of function 'kill_proc_info' make.err:(.text.compat_sys_rt_sigqueueinfo+0x80): undefined reference to `kill_proc_info'

Guess who came home for Christmas

commit d3228a887cae75ef2b8b1211c31c539bef5a5698

Quoted 8 lines

Author: Adrian Bunk <[email protected]>
Date:   Wed Dec 6 20:38:22 2006 -0800

[PATCH] make kernel/signal.c:kill_proc_info() static

Signed-off-by Adrian Bunk <[email protected]>

Signed-off-by Andrew Morton <[email protected]>

Signed-off-by Linus Torvalds <[email protected]>

Quoted 7 lines

There are hundreds of such patches of mine that got included in Linus' tree and that didn't cause problems. Besides making the kernel smaller, they regularly find "it should have been used" bugs.

In this one case, a patch that made something static that wasn't used from other files during the over 9 years of it's existence clashes with another patch that adds a user.

I'm really not that interested in the number of users ... that's only one part of the measure of the relevance of an API. However, if we're actually going to revoke an API I'd like it to be done on the API's merits (or lack of them) rather than simply whether there are any current in-kernel users.

Quoted 2 lines

Can we agree that it's not a usual case that the first user gets added after more than 9 years?

Actually, the compat_rtsiginfo patch has been argued over and honed for at least 3 years, so I'd say your statement isn't necessarily correct. For some of its life, it was even in -mm.

Quoted 2 lines

And making it global again when a user gets included isn't really that hard.

The life of a non-x86 architecture maintainer is hard ... it involves having lots of volunteers tirelessly chasing down things that broke during the big two week merge frenzy. It would be really helpful if things like this didn't add to that burden.

James

Quoted 4 lines · James Bottomley

Quoted 2 lines · Adrian Bunk

First of all thanks to Andrew for forwarding this email to me - it's otherwise a bit hard to defend against an email that doesn't reach me.

Quoted 3 lines

I think if you read the email you'll find it didn't actually attack you at all ... I was careful only to ask for a change of policy before accepting a patch that does nothing but make a function static.

Then let's call it "trying to establish rule forbidding something established without even informing the people whose actions you want to become forbidden".