Re: [PATCH 1/3] mm/slub: Only define kmalloc_large_node_hook() for NUMA systems

[Doug Anderson]

Hi,

On Tue, May 23, 2017 at 9:56 AM, Matthias Kaehlcke <mka@xxxxxxxxxxxx> wrote:
> Hi David,
>
> El Mon, May 22, 2017 at 06:35:23PM -0700 David Rientjes ha dit:
>
>> On Mon, 22 May 2017, Andrew Morton wrote:
>>
>> > > > Is clang not inlining kmalloc_large_node_hook() for some reason?  I don't
>> > > > think this should ever warn on gcc.
>> > >
>> > > clang warns about unused static inline functions outside of header
>> > > files, in difference to gcc.
>> >
>> > I wish it wouldn't.  These patches just add clutter.
>> >
>>
>> Matthias, what breaks if you do this?
>>
>> diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
>> index de179993e039..e1895ce6fa1b 100644
>> --- a/include/linux/compiler-clang.h
>> +++ b/include/linux/compiler-clang.h
>> @@ -15,3 +15,8 @@
>>   * with any version that can compile the kernel
>>   */
>>  #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
>> +
>> +#ifdef inline
>> +#undef inline
>> +#define inline __attribute__((unused))
>> +#endif
>
> Thanks for the suggestion!

Wow, I totally didn't think of this either when talking with Matthias
about this.  I guess it makes the assumption that nobody else is
hacking "inline" like we are, but "what are the chances of someone
doing that (TM)"  ;-)


> Nothing breaks and the warnings are silenced. It seems we could use
> this if there is a stong opposition against having warnings on unused
> static inline functions in .c files.
>
> Still I am not convinced that gcc's behavior is preferable in this
> case. True, it saves us from adding a bunch of __maybe_unused or
> #ifdefs, on the other hand the warning is a useful tool to spot truly
> unused code. So far about 50% of the warnings I looked into fall into
> this category.

Personally I actually prefer clang's behavior to gcc's here and I
think Matthias's patches are actually doing a service to the
community.  As he points out, many of the patches he's posted have
removed totally dead code from the kernel.  This code has often
existed for many years but never been noticed.  While the code wasn't
hurting anyone (it was dead and the compiler wasn't generating any
code for it), it would certainly add confusion to anyone reading the
source code.

Obviously my opinion isn't as important as the opinion of upstream
maintainers, but hopefully you'll consider it anyway.  :)  If you
still want to just make clang behave like gcc then I think David's
suggestion is a great one.


-Doug