Re: [rfc] the kernel workflow & trivial "global -> static" patches

[Andi Kleen]

Adrian Bunk wrote:
> On Tue, May 06, 2008 at 02:21:31AM +0200, Andi Kleen wrote:
>> I don't think the code changes actually with current gcc for integer
>> code if you change something from global to static (unless it causes
>> gcc to inline the function, but then it might be well larger if you're
>> unlucky)
> 
> It's a common case that a function has only one caller. It should always 
> be an (at least tiny) space win to get them inlined.

At least for the scheduler patch that started this thread this was not
the case. One was a global variable and the other was a callback from
proc. Both cannot be inlined.

> There are many small aspects, e.g. both gcc with -Wmissing-prototypes 
> and sparse give warnings, and the problem might either be needlessly 
> global code or the fact that a function prototype is either not in a 
> header or the header not #include'd by the file. Although I've only
> 2 or 3 times catched such bugs in the kernel that is a nasty to debug 
> class of bugs and gcc can find such problems at compile time.

Yes it's good to catch those. However I suspect there are better tools
for that that do it less work intensive. Traditionally in the Unix
world "lint" has been used to track this kind of bugs (dating back from
before prototypes were added to C). Now running any lint over the kernel
source would result in a incredibly number of warnings I'm sure, but
perhaps one can be configured to only output warnings related to
inconsistent prototypes over files. There are a couple of free lints
like the one in NetBSD or splint.	

>> I could see some advantage from static in future compiler versions 
>> though from better optimization, but it's quite remote.
>> ...
> 
> The best case I've actually seen in practice was a variable I made 
> static, and with CONFIG_DEBUG_FOOBAR=n gcc was now able to prove that 
> the value never changed resulting in the variable plus quite a chunk
> of code no longer emitted.

Sounds like the variable should just have been removed then in the source?

-Andi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/