Message-ID: <20170531235519.GX141096@google.com>
Date:   Wed, 31 May 2017 16:55:19 -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>,
        Guenter Roeck <linux@...ck-us.net>,
        Mark Brown <broonie@...nel.org>,
        David Miller <davem@...emloft.net>
Subject: Re: [RFC] clang: 'unused-function' warning on static inline functions

El Tue, May 30, 2017 at 11:13:06AM -0700 Matthias Kaehlcke ha dit:

> 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.

As suggested in one of the other threads, a list of the false
positives (from x86 and arm64 defconfig) that would need attention in
case of leaving the warning enabled:

arch/x86/kernel/cpu/common.c:267:19: warning: unused function 'flag_is_changeable_p' [-Wunused-function]
arch/x86/platform/efi/efi_64.c:240:1: warning: unused function 'virt_to_phys_or_null_size' [-Wunused-function]
block/cfq-iosched.c:451:1: warning: unused function 'cfq_clear_cfqq_sync' [-Wunused-function]
block/cfq-iosched.c:590:20: warning: unused function 'cfqg_stats_set_start_group_wait_time' [-Wunused-function]
block/cfq-iosched.c:591:20: warning: unused function 'cfqg_stats_end_empty_time' [-Wunused-function]
drivers/gpu/drm/drm_mm.c:165:1: warning: unused function 'drm_mm_interval_tree_insert' [-Wunused-function]
drivers/gpu/drm/drm_mm.c:165:1: warning: unused function 'drm_mm_interval_tree_iter_next' [-Wunused-function]
drivers/gpu/drm/i915/i915_sw_fence.c:97:20: warning: unused function 'debug_fence_free' [-Wunused-function]
drivers/hid/hid-sony.c:2105:20: warning: unused function 'sony_send_output_report' [-Wunused-function]
drivers/media/media-entity.c:41:27: warning: unused function 'intf_type' [-Wunused-function]
drivers/watchdog/s3c2410_wdt.c:212:35: warning: unused function 'freq_to_wdt' [-Wunused-function]
drivers/watchdog/s3c2410_wdt.c:308:19: warning: unused function 's3c2410wdt_is_running' [-Wunused-function]
kernel/events/core.c:928:19: warning: unused function 'perf_cgroup_event_cgrp_time' [-Wunused-function]
kernel/locking/osq_lock.c:24:19: warning: unused function 'node_cpu' [-Wunused-function]
kernel/power/snapshot.c:1256:21: warning: unused function 'saveable_highmem_page' [-Wunused-function]
kernel/sched/cputime.c:258:19: warning: unused function 'account_other_time' [-Wunused-function]
lib/zlib_inflate/inffast.c:31:1: warning: unused function 'get_unaligned16' [-Wunused-function]
mm/page_alloc.c:1312:30: warning: unused function 'meminit_pfn_in_nid' [-Wunused-function]
mm/slub.c:1328:21: warning: unused function 'slab_free_hook' [-Wunused-function]
mm/slub.c:1945:28: warning: unused function 'tid_to_cpu' [-Wunused-function]
mm/slub.c:1950:29: warning: unused function 'tid_to_event' [-Wunused-function]
net/netfilter/nf_conntrack_netlink.c:534:22: warning: unused function 'ctnetlink_proto_size' [-Wunused-function]
net/netfilter/nf_conntrack_netlink.c:551:22: warning: unused function 'ctnetlink_acct_size' [-Wunused-function]
net/netfilter/nf_conntrack_netlink.c:561:19: warning: unused function 'ctnetlink_secctx_size' [-Wunused-function]
net/netfilter/nf_conntrack_netlink.c:577:22: warning: unused function 'ctnetlink_timestamp_size' [-Wunused-function]

The list does not include instances that have already been addressed
in maintainer trees or warnings about actual dead code. Obviously
there will be more instances for other architectures and
configurations.

Powered by blists - more mailing lists