Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1651654 > unrolled thread
| Started by | Matthias Kaehlcke <mka@chromium.org> |
|---|---|
| First post | 2017-05-27 03:30 +0200 |
| Last post | 2017-05-30 18:30 +0200 |
| Articles | 6 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH 0/3] cfq-iosched: Fix warnings about unused functions Matthias Kaehlcke <mka@chromium.org> - 2017-05-27 03:30 +0200
[PATCH 3/3] cfq-iosched: Delete unused function min_vdisktime() Matthias Kaehlcke <mka@chromium.org> - 2017-05-27 03:30 +0200
Re: [PATCH 3/3] cfq-iosched: Delete unused function min_vdisktime() Jens Axboe <axboe@kernel.dk> - 2017-05-30 18:00 +0200
[PATCH 1/3] cfq-iosched: Mark cfq_clear_cfqq_*() as __maybe_unused Matthias Kaehlcke <mka@chromium.org> - 2017-05-27 03:30 +0200
Re: [PATCH 1/3] cfq-iosched: Mark cfq_clear_cfqq_*() as __maybe_unused Christoph Hellwig <hch@infradead.org> - 2017-05-28 10:00 +0200
Re: [PATCH 1/3] cfq-iosched: Mark cfq_clear_cfqq_*() as __maybe_unused Doug Anderson <dianders@chromium.org> - 2017-05-30 18:30 +0200
| From | Matthias Kaehlcke <mka@chromium.org> |
|---|---|
| Date | 2017-05-27 03:30 +0200 |
| Subject | [PATCH 0/3] cfq-iosched: Fix warnings about unused functions |
| Message-ID | <tLyIi-4Vw-23@gated-at.bofh.it> |
This patch series fixes a bunch of 'unused-function' warnings raised by clang. Matthias Kaehlcke (3): cfq-iosched: Mark cfq_clear_cfqq_*() as __maybe_unused cfq-iosched: Fix warning about unused dummy functions cfq-iosched: Delete unused function min_vdisktime() block/cfq-iosched.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) -- 2.13.0.219.gdb65acc882-goog
[toc] | [next] | [standalone]
| From | Matthias Kaehlcke <mka@chromium.org> |
|---|---|
| Date | 2017-05-27 03:30 +0200 |
| Subject | [PATCH 3/3] cfq-iosched: Delete unused function min_vdisktime() |
| Message-ID | <tLyIl-4Vw-123@gated-at.bofh.it> |
| In reply to | #1651654 |
This fixes the following warning when building with clang:
block/cfq-iosched.c:970:19: error: unused function 'min_vdisktime'
[-Werror,-Wunused-function]
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
block/cfq-iosched.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index b23c73db4cc0..61cb45553314 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -981,15 +981,6 @@ static inline u64 max_vdisktime(u64 min_vdisktime, u64 vdisktime)
return min_vdisktime;
}
-static inline u64 min_vdisktime(u64 min_vdisktime, u64 vdisktime)
-{
- s64 delta = (s64)(vdisktime - min_vdisktime);
- if (delta < 0)
- min_vdisktime = vdisktime;
-
- return min_vdisktime;
-}
-
static void update_min_vdisktime(struct cfq_rb_root *st)
{
struct cfq_group *cfqg;
--
2.13.0.219.gdb65acc882-goog
[toc] | [prev] | [next] | [standalone]
| From | Jens Axboe <axboe@kernel.dk> |
|---|---|
| Date | 2017-05-30 18:00 +0200 |
| Subject | Re: [PATCH 3/3] cfq-iosched: Delete unused function min_vdisktime() |
| Message-ID | <tMRIR-Op-5@gated-at.bofh.it> |
| In reply to | #1651663 |
On 05/26/2017 03:22 PM, Matthias Kaehlcke wrote: > This fixes the following warning when building with clang: > > block/cfq-iosched.c:970:19: error: unused function 'min_vdisktime' > [-Werror,-Wunused-function] I have applied this one for 4.13, as that is just dead code. I don't think the others are worth bothering with. -- Jens Axboe
[toc] | [prev] | [next] | [standalone]
| From | Matthias Kaehlcke <mka@chromium.org> |
|---|---|
| Date | 2017-05-27 03:30 +0200 |
| Subject | [PATCH 1/3] cfq-iosched: Mark cfq_clear_cfqq_*() as __maybe_unused |
| Message-ID | <tLyIn-4Vw-169@gated-at.bofh.it> |
| In reply to | #1651654 |
This fixes the following warning when building with clang:
block/cfq-iosched.c:449:1: error: unused function 'cfq_clear_cfqq_sync'
[-Werror,-Wunused-function]
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
block/cfq-iosched.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index da69b079725f..36ab3645effc 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -431,7 +431,8 @@ static inline void cfq_mark_cfqq_##name(struct cfq_queue *cfqq) \
{ \
(cfqq)->flags |= (1 << CFQ_CFQQ_FLAG_##name); \
} \
-static inline void cfq_clear_cfqq_##name(struct cfq_queue *cfqq) \
+static inline void __maybe_unused \
+cfq_clear_cfqq_##name(struct cfq_queue *cfqq) \
{ \
(cfqq)->flags &= ~(1 << CFQ_CFQQ_FLAG_##name); \
} \
--
2.13.0.219.gdb65acc882-goog
[toc] | [prev] | [next] | [standalone]
| From | Christoph Hellwig <hch@infradead.org> |
|---|---|
| Date | 2017-05-28 10:00 +0200 |
| Subject | Re: [PATCH 1/3] cfq-iosched: Mark cfq_clear_cfqq_*() as __maybe_unused |
| Message-ID | <tM1hf-7t8-1@gated-at.bofh.it> |
| In reply to | #1651684 |
On Fri, May 26, 2017 at 02:22:35PM -0700, Matthias Kaehlcke wrote: > This fixes the following warning when building with clang: > > block/cfq-iosched.c:449:1: error: unused function 'cfq_clear_cfqq_sync' > [-Werror,-Wunused-function] > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org> Matthias, can you please stop sending these patches? Gcc semantics correctly are that static inlines can be unused and it's perfectly fine. It's your job to make clang fit that instead of spreading garbage all over the kernel.
[toc] | [prev] | [next] | [standalone]
| From | Doug Anderson <dianders@chromium.org> |
|---|---|
| Date | 2017-05-30 18:30 +0200 |
| Subject | Re: [PATCH 1/3] cfq-iosched: Mark cfq_clear_cfqq_*() as __maybe_unused |
| Message-ID | <tMSbU-1dJ-27@gated-at.bofh.it> |
| In reply to | #1652052 |
Christoph, On Sun, May 28, 2017 at 12:49 AM, Christoph Hellwig <hch@infradead.org> wrote: > On Fri, May 26, 2017 at 02:22:35PM -0700, Matthias Kaehlcke wrote: >> This fixes the following warning when building with clang: >> >> block/cfq-iosched.c:449:1: error: unused function 'cfq_clear_cfqq_sync' >> [-Werror,-Wunused-function] >> >> Signed-off-by: Matthias Kaehlcke <mka@chromium.org> > > Matthias, can you please stop sending these patches? Gcc semantics > correctly are that static inlines can be unused and it's perfectly > fine. It's your job to make clang fit that instead of spreading garbage > all over the kernel. I think we've been having discussions about this in many different scattered threads. A quick summary here is: * clang only warns about unused static inline functions if those functions are in ".c" files. That basically means there aren't nearly as many false positives of the check as you would think. * Matthias has found several instances of dead code with his work. It's nice to get rid of those. In addition, in at least one example his work has actually identified code that was not dead (AKA actual instructions were generated) but the code was clearly not correct. That's because there was a "static inline" save and restore function. The save was called but not the restore. This points to either a bug (should have called the restore) or code that should be eliminated. https://patchwork.kernel.org/patch/9750813/ * Most compiler warnings generate a bit of "noise". It's always a judgement call about whether the signal to noise ratio makes the warning useful. This is a tough call, but IMHO the signal to noise ratio for the clang behavior makes it worth it. The number of "maybe unused" patches is not that great and IMHO adding the attribute is also documenting the function, which is useful too. * There exists a patch to make clang behave like gcc. https://patchwork.kernel.org/patch/9746913/. If folks truly believe that the noise is not worth it, we can apply that. I just talked to Matthias and he is going to try to start a thread to get hopefully get a general policy agreed upon before continuing to post patches. -Doug
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web