Message-ID: <20170530181306.GV141096@google.com>
Date:   Tue, 30 May 2017 11:13:06 -0700
From:   Matthias Kaehlcke <mka@...omium.org>
To:     linux-kernel@...r.kernel.org
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Ingo Molnar <mingo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Christoph Hellwig <hch@....de>, Jens Axboe <axboe@...nel.dk>,
        Steven Rostedt <rostedt@...dmis.org>,
        David Rientjes <rientjes@...gle.com>,
        Douglas Anderson <dianders@...omium.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: [RFC] clang: 'unused-function' warning on static inline functions

Hi,

There has been discussion spread over different threads on how to deal
with 'unused-function' warnings raised by clang on static inline
functions. gcc in general does not emit warnings for unused static
inline functions, clang does if the function is in a .c file.

When building the kernel with clang several 'unused-function' warnings
are emitted, about half of them point out actual dead code, the others
are false positives stemming from functions that are only used with
certain combinations of configuration options.

I consider the warning a useful tool to detect unused code and started
to submit patches that remove dead code and suppress the false
positives. The dead code removal was generally well received,
maintainer response to suppressing the false positives was mixed. Some
patches were accepted, in other cases maintainers raised concerns
that adding '__maybe_unused' or putting a function inside an #ifdef
block just adds clutter to the code without providing any benefits,
and clang should behave like gcc in this aspect.

Most discussion took place in these two threads:

zlib: Put get_unaligned16() inside #ifdef block
https://patchwork.kernel.org/patch/9741413/

compiler, clang: suppress warning for unused static inline functions
https://patchwork.kernel.org/patch/9746913/

I understand the reluctance to adding clutter to the code (though one
could argue that __maybe_unused serves as documentation; using #ifdef
is an option, but is not needed in most cases), but I don't think that
it would be preferable to have clang behave like gcc. Silencing the
warning on false positives removes clutter from the build log and
makes it easier to spot the actual dead code.

These are examples of unused code that has been removed thanks to the
warning on static inline functions:

x86/ioapic: Remove unused function IO_APIC_irq_trigger()
https://patchwork.kernel.org/patch/9741539/

cfq-iosched: Delete unused function min_vdisktime()
https://patchwork.kernel.org/patch/9751327/

r8152: Remove unused function usb_ocp_read()
https://patchwork.kernel.org/patch/9735027/

net1080: Remove unused function nc_dump_ttl()
https://patchwork.kernel.org/patch/9735053/

crypto: rng: Remove unused function __crypto_rng_cast()
https://patchwork.kernel.org/patch/9741515/

perf/core: Remove unused function perf_cgroup_event_cgrp_time()
https://patchwork.kernel.org/patch/9744129/

net: jme: Remove unused functions
https://patchwork.kernel.org/patch/9744673/

ASoC: Intel: sst: Remove unused function sst_restore_shim64()
https://patchwork.kernel.org/patch/9741551/

A further code removal that was spotted in the context of the previous
patch:

ASoC: Intel: sst: Delete sst_shim_regs64; saved regs are never used
https://patchwork.kernel.org/patch/9754923/

The goal of this thread is to arrive to a conclusion on how to deal
with the warning. If we want clang to help to detect unused static
inline functions I think we should suppress the warning on false
positives (the number is limited, at least for x86 and arm64
defconfig, patches for most instances have already been sent out). And
if maintainers consider that having the extra ability to spot dead
code doesn't justify the clutter of marking some functions as
__maybe_used (or using #ifdef if preferred) we should probably apply
Davids patch (https://patchwork.kernel.org/patch/9746913/) to make
clang behave like gcc for unused static inline functions.

Thanks

Matthias

Powered by blists - more mailing lists