Re: [PATCH 18/46] kernel: Mark functions as static in sched/fair.c

[josh@joshtriplett.org]

On Thu, Feb 27, 2014 at 08:24:35PM +0100, Peter Zijlstra wrote:
> On Thu, Feb 27, 2014 at 08:03:22AM -0800, Josh Triplett wrote:
> > Did you perhaps check, and notice that there are *zero* uses of this
> > function in the kernel?  Nothing overrides this weak symbol; it is no
> > longer needed.  You removed the one and only user in your commit:
> 
> I know that; but you don't get to remove interfaces under the guise of a
> static checker and without mention of such in the changelog.

So the changelog message needed improvement.  Got it; that would have
been helpful to hear.

The kernel hardly goes to great lengths to preserve old interfaces with
no users (per stable_api_nonsense), and the commit message already stated
that the function was being marked as static because it wasn't used
elsewhere.  But sure, perhaps something like the following, added to the
commit message, would address your concern?

"""
arch_scale_smt_power, in particular, is a __weak function provided for
architectures to override.  However, the only overriding definition was
removed before v3.6, in commit ee08d1284ea9235b29bd2d9b7493b4b4cf3da09c
("sched/x86: Remove broken power estimation").  Thus, drop the __weak
and make the function static.
"""

Or would you prefer to see it completely eliminated (inlining it into
its caller) as part of the same patch?

- Josh Triplett