[PATCH v2] cleanup: Fix "unused function" warnings with conditional guards

[PATCH v2] cleanup: Fix "unused function" warnings with conditional guards

[Dan Williams]

While the warning could simply be moved to W=2 [1], there is some small
value, and not much cost to fixing it.

The issue, Andy reports that the "lock_timer" scheme in
kernel/time/posix-timers.c, with its custom usage of
DEFINE_CLASS_IS_COND_GUARD(), results in:

kernel/time/posix-timers.c:89:1: error: unused function 'class_lock_timer_lock_err' [-Werror,-Wunused-function]
   89 | DEFINE_CLASS_IS_COND_GUARD(lock_timer);

...with a clang W=1 build. This warning has some value because it can catch
when a conditional guard is defined, but not evaluated by a conditional
acquisition helper like scoped_cond_guard() or ACQUIRE().

Andy also reports that plain DEFINE_GUARD() also encounters this warning:

drivers/pwm/core.c:54:1: error: unused function 'class_pwmchip_lock_err' [-Werror,-Wunused-function]
   54 | DEFINE_GUARD(pwmchip, struct pwm_chip *, pwmchip_lock(_T), pwmchip_unlock(_T))

...which *is* a false positive.

Fix those 2 issues by teaching scoped_cond_guard() to check for error
values, and otherwise teach the DEFINE_GUARD() path to mark the conditional
helpers as __maybe_unused.

The compiler does change the polarity of some tests, but the size of the
binary is identical:

Before:
        scoped_timer_get_or_fail(timer_id)
    1246:       44 89 ff                mov    %r15d,%edi
    1249:       e8 d2 5a 00 00          call   6d20 <class_lock_timer_constructor>
    124e:       49 89 c7                mov    %rax,%r15
    1251:       48 3d 01 f0 ff ff       cmp    $0xfffffffffffff001,%rax
    1257:       0f 92 c0                setb   %al
    125a:       4d 85 ff                test   %r15,%r15
    125d:       0f 95 c1                setne  %cl
    1260:       84 c8                   test   %cl,%al
    1262:       0f 84 85 00 00 00       je     12ed <__ia32_sys_timer_gettime+0x15d>
    ...
    12ed:       48 c7 c3 ea ff ff ff    mov    $0xffffffffffffffea,%rbx
        if (likely((timr)))
    12f4:       4d 85 ff                test   %r15,%r15
    12f7:       74 0c                   je     1305 <__ia32_sys_timer_gettime+0x175>

After:
        scoped_timer_get_or_fail(timer_id)
    1246:       44 89 ff                mov    %r15d,%edi
    1249:       e8 d2 5a 00 00          call   6d20 <class_lock_timer_constructor>
    124e:       49 89 c7                mov    %rax,%r15
    1251:       48 3d 01 f0 ff ff       cmp    $0xfffffffffffff001,%rax
    1257:       0f 92 c0                setb   %al
    125a:       4d 85 ff                test   %r15,%r15
    125d:       0f 95 c1                setne  %cl
    1260:       84 c8                   test   %cl,%al
    1262:       75 21                   jne    1285 <__ia32_sys_timer_gettime+0xf5>
    1264:       48 c7 c3 ea ff ff ff    mov    $0xffffffffffffffea,%rbx
    126b:       4d 85 ff                test   %r15,%r15
        if (likely((timr)))
    126e:       0f 84 94 00 00 00       je     1308 <__ia32_sys_timer_gettime+0x178>

Alternatively just merge the suggestion in [1], and call it a day.

Link: http://lore.kernel.org/20250813152142.GP4067720@noisy.programming.kicks-ass.net [1]

Cc: Nathan Chancellor <nathan@kernel.org>
Cc: "Peter Zijlstra (Intel)" <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Dave Jiang <dave.jiang@intel.com>
Cc: David Lechner <dlechner@baylibre.com>
Cc: Jonathan Cameron <jonathan.cameron@huawei.com>
Reported-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Closes: http://lore.kernel.org/aIo18KZpmKuR4hVZ@black.igk.intel.com
Tested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Fixes: 857d18f23ab1 ("cleanup: Introduce ACQUIRE() and ACQUIRE_ERR() for conditional locks")
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
v1: http://lore.kernel.org/20250804220955.1453135-1-dan.j.williams@intel.com

 include/linux/cleanup.h | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h
index 2573585b7f06..b3a7f6b2142d 100644
--- a/include/linux/cleanup.h
+++ b/include/linux/cleanup.h
@@ -354,26 +354,30 @@ static __maybe_unused const bool class_##_name##_is_conditional = _is_cond
 			_ptr = NULL;                                        \
 		}                                                           \
 		return _ptr;                                                \
-	}                                                                   \
-	static inline int class_##_name##_lock_err(class_##_name##_t *_T)   \
-	{                                                                   \
-		long _rc = (__force unsigned long)*(_exp);                  \
-		if (!_rc) {                                                 \
-			_rc = -EBUSY;                                       \
-		}                                                           \
-		if (!IS_ERR_VALUE(_rc)) {                                   \
-			_rc = 0;                                            \
-		}                                                           \
-		return _rc;                                                 \
+	}
+
+#define __DEFINE_GUARD_LOCK_ERR(_name, _exp, _attr)                      \
+	static _attr int class_##_name##_lock_err(class_##_name##_t *_T) \
+	{                                                                \
+		long _rc = (__force unsigned long)*(_exp);               \
+		if (!_rc) {                                              \
+			_rc = -EBUSY;                                    \
+		}                                                        \
+		if (!IS_ERR_VALUE(_rc)) {                                \
+			_rc = 0;                                         \
+		}                                                        \
+		return _rc;                                              \
 	}
 
 #define DEFINE_CLASS_IS_GUARD(_name) \
 	__DEFINE_CLASS_IS_CONDITIONAL(_name, false); \
-	__DEFINE_GUARD_LOCK_PTR(_name, _T)
+	__DEFINE_GUARD_LOCK_PTR(_name, _T) \
+	__DEFINE_GUARD_LOCK_ERR(_name, _T, __maybe_unused)
 
 #define DEFINE_CLASS_IS_COND_GUARD(_name) \
 	__DEFINE_CLASS_IS_CONDITIONAL(_name, true); \
-	__DEFINE_GUARD_LOCK_PTR(_name, _T)
+	__DEFINE_GUARD_LOCK_PTR(_name, _T) \
+	__DEFINE_GUARD_LOCK_ERR(_name, _T, inline)
 
 #define DEFINE_GUARD(_name, _type, _lock, _unlock) \
 	DEFINE_CLASS(_name, _type, if (!__GUARD_IS_ERR(_T)) { _unlock; }, ({ _lock; _T; }), _type _T); \
@@ -430,7 +434,8 @@ _label:									\
 
 #define __scoped_cond_guard(_name, _fail, _label, args...)		\
 	for (CLASS(_name, scope)(args); true; ({ goto _label; }))	\
-		if (!__guard_ptr(_name)(&scope)) {			\
+		if (!__guard_ptr(_name)(&scope) ||			\
+		     __guard_err(_name)(&scope)) {			\
 			BUILD_BUG_ON(!__is_cond_ptr(_name));		\
 			_fail;						\
 _label:									\
@@ -471,7 +476,8 @@ static inline void class_##_name##_destructor(class_##_name##_t *_T)	\
 	if (!__GUARD_IS_ERR(_T->lock)) { _unlock; }			\
 }									\
 									\
-__DEFINE_GUARD_LOCK_PTR(_name, &_T->lock)
+__DEFINE_GUARD_LOCK_PTR(_name, &_T->lock) \
+__DEFINE_GUARD_LOCK_ERR(_name, &_T->lock, __maybe_unused)
 
 #define __DEFINE_LOCK_GUARD_1(_name, _type, _lock)			\
 static inline class_##_name##_t class_##_name##_constructor(_type *l)	\

base-commit: b320789d6883cc00ac78ce83bccbfe7ed58afcf0
-- 
2.51.0

Re: [PATCH v2] cleanup: Fix "unused function" warnings with conditional guards

[Andrew Morton]

On Thu, 4 Sep 2025 15:50:10 -0700 Dan Williams <dan.j.williams@intel.com> wrote:

> While the warning could simply be moved to W=2 [1], there is some small
> value, and not much cost to fixing it.
> 
> The issue, Andy reports that the "lock_timer" scheme in
> kernel/time/posix-timers.c, with its custom usage of
> DEFINE_CLASS_IS_COND_GUARD(), results in:
> 
> kernel/time/posix-timers.c:89:1: error: unused function 'class_lock_timer_lock_err' [-Werror,-Wunused-function]
>    89 | DEFINE_CLASS_IS_COND_GUARD(lock_timer);
> 
> ...with a clang W=1 build. This warning has some value because it can catch
> when a conditional guard is defined, but not evaluated by a conditional
> acquisition helper like scoped_cond_guard() or ACQUIRE().
> 
> Andy also reports that plain DEFINE_GUARD() also encounters this warning:
> 
> drivers/pwm/core.c:54:1: error: unused function 'class_pwmchip_lock_err' [-Werror,-Wunused-function]
>    54 | DEFINE_GUARD(pwmchip, struct pwm_chip *, pwmchip_lock(_T), pwmchip_unlock(_T))
> 
> ...which *is* a false positive.
> 
> Fix those 2 issues by teaching scoped_cond_guard() to check for error
> values, and otherwise teach the DEFINE_GUARD() path to mark the conditional
> helpers as __maybe_unused.

Warning about unused static inlines in .c files is just annoying.  If
the function is unused in all possible configs (man GREP(1)) then OK. 
Otherwise, let it be.

> Alternatively just merge the suggestion in [1], and call it a day.
> 
> Link: http://lore.kernel.org/20250813152142.GP4067720@noisy.programming.kicks-ass.net [1]

lgtm, unless we think this (your) patch improves the code for other reasons?

Re: [PATCH v2] cleanup: Fix "unused function" warnings with conditional guards

[dan.j.williams]

Andrew Morton wrote:
[..]
> > Alternatively just merge the suggestion in [1], and call it a day.
> > 
> > Link: http://lore.kernel.org/20250813152142.GP4067720@noisy.programming.kicks-ass.net [1]
> 
> lgtm, unless we think this (your) patch improves the code for other reasons?

The tl;dr above is that the warning could have small value, but probably
not greater than the overall benefit to Linux to stop bothering folks
with this low-value warning by default at W=1.

So I am over the sunk costs, and moving this warning to W=2 is the way
to go.

Re: [PATCH v2] cleanup: Fix "unused function" warnings with conditional guards

[Andy Shevchenko]

On Wed, Sep 17, 2025 at 04:21:07PM -0700, dan.j.williams@intel.com wrote:
> Andrew Morton wrote:

[..]

> > > Alternatively just merge the suggestion in [1], and call it a day.
> > > 
> > > Link: http://lore.kernel.org/20250813152142.GP4067720@noisy.programming.kicks-ass.net [1]
> > 
> > lgtm, unless we think this (your) patch improves the code for other reasons?
> 
> The tl;dr above is that the warning could have small value, but probably
> not greater than the overall benefit to Linux to stop bothering folks
> with this low-value warning by default at W=1.
> 
> So I am over the sunk costs, and moving this warning to W=2 is the way
> to go.

Can somebody add a fix so, we have v6.17 able to be built with `make W=1`, please?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] cleanup: Fix "unused function" warnings with conditional guards

[dan.j.williams]

Andy Shevchenko wrote:
> On Wed, Sep 17, 2025 at 04:21:07PM -0700, dan.j.williams@intel.com wrote:
> > Andrew Morton wrote:
> 
> [..]
> 
> > > > Alternatively just merge the suggestion in [1], and call it a day.
> > > > 
> > > > Link: http://lore.kernel.org/20250813152142.GP4067720@noisy.programming.kicks-ass.net [1]
> > > 
> > > lgtm, unless we think this (your) patch improves the code for other reasons?
> > 
> > The tl;dr above is that the warning could have small value, but probably
> > not greater than the overall benefit to Linux to stop bothering folks
> > with this low-value warning by default at W=1.
> > 
> > So I am over the sunk costs, and moving this warning to W=2 is the way
> > to go.
> 
> Can somebody add a fix so, we have v6.17 able to be built with `make W=1`, please?

Andy, might you have time to take Peter's proposed diff and wrap it with
a changelog for Andrew to pull?

Re: [PATCH v2] cleanup: Fix "unused function" warnings with conditional guards

[Andy Shevchenko]

On Thu, Sep 18, 2025 at 12:26:28PM -0700, dan.j.williams@intel.com wrote:
> Andy Shevchenko wrote:
> > On Wed, Sep 17, 2025 at 04:21:07PM -0700, dan.j.williams@intel.com wrote:
> > > Andrew Morton wrote:

[..]

> > > > > Alternatively just merge the suggestion in [1], and call it a day.
> > > > > 
> > > > > Link: http://lore.kernel.org/20250813152142.GP4067720@noisy.programming.kicks-ass.net [1]
> > > > 
> > > > lgtm, unless we think this (your) patch improves the code for other reasons?
> > > 
> > > The tl;dr above is that the warning could have small value, but probably
> > > not greater than the overall benefit to Linux to stop bothering folks
> > > with this low-value warning by default at W=1.
> > > 
> > > So I am over the sunk costs, and moving this warning to W=2 is the way
> > > to go.
> > 
> > Can somebody add a fix so, we have v6.17 able to be built with `make W=1`, please?
> 
> Andy, might you have time to take Peter's proposed diff and wrap it with
> a changelog for Andrew to pull?

I will try my best, thanks!


-- 
With Best Regards,
Andy Shevchenko