[PATCH] cleanup: Fix unused guard error function with DEFINE_CLASS_IS_COND_GUARD

[PATCH] cleanup: Fix unused guard error function with DEFINE_CLASS_IS_COND_GUARD

[Dan Williams]

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. Per Nathan, clang catches unused "static inline"
functions in C files since commit 6863f5643dd7 ("kbuild: allow Clang to
find unused static inline functions for W=1 build").

There are 2 ways to solve this, either mark the class_##_lock_err()
function as __maybe_unused, or make sure class_##_lock_err() *is* used /
gets called to disposition the lock status.

At present __lock_timer() only indicates failure with a NULL __guard_ptr().
However, one could imagine that __lock_timer(), or some other custom
conditional locking primitive, wants to pass an ERR_PTR() to indicate the
reason for the lock acquisition failure.

Update __scoped_cond_guard() to check for ERR_PTR() in addition to NULL
@scope values. This allows __lock_timer(), or another open coded
DEFINE_CLASS_IS_COND_GUARD() user, to switch to passing an ERR_PTR() in the
future. In the meantime, this just silences the warning.

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
Fixes: 857d18f23ab1 ("cleanup: Introduce ACQUIRE() and ACQUIRE_ERR() for conditional locks")
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
Dave, I am sending this to you to take upstream since the warning came
in through the CXL tree. If anyone else wants to take it just holler.

 include/linux/cleanup.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h
index 4eb83dd71cfe..d8e7d1e5561b 100644
--- a/include/linux/cleanup.h
+++ b/include/linux/cleanup.h
@@ -423,7 +423,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:									\

base-commit: 857d18f23ab17284d1b6de6f61f4e74958596376
-- 
2.50.1

Re: [PATCH] cleanup: Fix unused guard error function with DEFINE_CLASS_IS_COND_GUARD

[Andy Shevchenko]

On Mon, Aug 04, 2025 at 03:09:54PM -0700, Dan Williams wrote:
> 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. Per Nathan, clang catches unused "static inline"
> functions in C files since commit 6863f5643dd7 ("kbuild: allow Clang to
> find unused static inline functions for W=1 build").
> 
> There are 2 ways to solve this, either mark the class_##_lock_err()
> function as __maybe_unused, or make sure class_##_lock_err() *is* used /
> gets called to disposition the lock status.
> 
> At present __lock_timer() only indicates failure with a NULL __guard_ptr().
> However, one could imagine that __lock_timer(), or some other custom
> conditional locking primitive, wants to pass an ERR_PTR() to indicate the
> reason for the lock acquisition failure.
> 
> Update __scoped_cond_guard() to check for ERR_PTR() in addition to NULL
> @scope values. This allows __lock_timer(), or another open coded
> DEFINE_CLASS_IS_COND_GUARD() user, to switch to passing an ERR_PTR() in the
> future. In the meantime, this just silences the warning.

Hmm... It seems fixes the timer case, but left others still fail:

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))
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/cleanup.h:380:2: note: expanded from macro 'DEFINE_GUARD'
  380 |         DEFINE_CLASS_IS_GUARD(_name)
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/cleanup.h:372:2: note: expanded from macro 'DEFINE_CLASS_IS_GUARD'
  372 |         __DEFINE_GUARD_LOCK_PTR(_name, _T)
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/cleanup.h:358:20: note: expanded from macro '__DEFINE_GUARD_LOCK_PTR'
  358 |         static inline int class_##_name##_lock_err(class_##_name##_t *_T)   \
      |                           ^~~~~~~~~~~~~~~~~~~~~~~~
<scratch space>:81:1: note: expanded from here
   81 | class_pwmchip_lock_err
      | ^~~~~~~~~~~~~~~~~~~~~~
1 error generated.

fs/namespace.c:1782:1: error: unused function 'class_namespace_lock_lock_err' [-Werror,-Wunused-function]
  1782 | DEFINE_GUARD(namespace_lock, struct rw_semaphore *, namespace_lock(), namespace_unlock())
       | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/cleanup.h:380:2: note: expanded from macro 'DEFINE_GUARD'
  380 |         DEFINE_CLASS_IS_GUARD(_name)
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/cleanup.h:372:2: note: expanded from macro 'DEFINE_CLASS_IS_GUARD'
  372 |         __DEFINE_GUARD_LOCK_PTR(_name, _T)
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/cleanup.h:358:20: note: expanded from macro '__DEFINE_GUARD_LOCK_PTR'
  358 |         static inline int class_##_name##_lock_err(class_##_name##_t *_T)   \
      |                           ^~~~~~~~~~~~~~~~~~~~~~~~
<scratch space>:15:1: note: expanded from here
   15 | class_namespace_lock_lock_err
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

These I have and I dunno how many more can be detected with allyesconfig and/or
allmodconfig on all possible architectures...

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] cleanup: Fix unused guard error function with DEFINE_CLASS_IS_COND_GUARD

[Andy Shevchenko]

On Mon, Aug 04, 2025 at 03:09:54PM -0700, Dan Williams wrote:
> 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. Per Nathan, clang catches unused "static inline"
> functions in C files since commit 6863f5643dd7 ("kbuild: allow Clang to
> find unused static inline functions for W=1 build").
> 
> There are 2 ways to solve this, either mark the class_##_lock_err()
> function as __maybe_unused, or make sure class_##_lock_err() *is* used /
> gets called to disposition the lock status.
> 
> At present __lock_timer() only indicates failure with a NULL __guard_ptr().
> However, one could imagine that __lock_timer(), or some other custom
> conditional locking primitive, wants to pass an ERR_PTR() to indicate the
> reason for the lock acquisition failure.
> 
> Update __scoped_cond_guard() to check for ERR_PTR() in addition to NULL
> @scope values. This allows __lock_timer(), or another open coded
> DEFINE_CLASS_IS_COND_GUARD() user, to switch to passing an ERR_PTR() in the
> future. In the meantime, this just silences the warning.

FWIW,

Tested-by: Andy Shevchenko <andriy.shevchenko@intel.com>

but as I mentioned in previous mail, it doesn't fix all issues I have, and
maybe more.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] cleanup: Fix unused guard error function with DEFINE_CLASS_IS_COND_GUARD

[dan.j.williams]

Andy Shevchenko wrote:
> On Mon, Aug 04, 2025 at 03:09:54PM -0700, Dan Williams wrote:
> > 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. Per Nathan, clang catches unused "static inline"
> > functions in C files since commit 6863f5643dd7 ("kbuild: allow Clang to
> > find unused static inline functions for W=1 build").
> > 
> > There are 2 ways to solve this, either mark the class_##_lock_err()
> > function as __maybe_unused, or make sure class_##_lock_err() *is* used /
> > gets called to disposition the lock status.
> > 
> > At present __lock_timer() only indicates failure with a NULL __guard_ptr().
> > However, one could imagine that __lock_timer(), or some other custom
> > conditional locking primitive, wants to pass an ERR_PTR() to indicate the
> > reason for the lock acquisition failure.
> > 
> > Update __scoped_cond_guard() to check for ERR_PTR() in addition to NULL
> > @scope values. This allows __lock_timer(), or another open coded
> > DEFINE_CLASS_IS_COND_GUARD() user, to switch to passing an ERR_PTR() in the
> > future. In the meantime, this just silences the warning.
> 
> Hmm... It seems fixes the timer case, but left others still fail:
> 
> 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))
>       | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> include/linux/cleanup.h:380:2: note: expanded from macro 'DEFINE_GUARD'
>   380 |         DEFINE_CLASS_IS_GUARD(_name)
>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> include/linux/cleanup.h:372:2: note: expanded from macro 'DEFINE_CLASS_IS_GUARD'
>   372 |         __DEFINE_GUARD_LOCK_PTR(_name, _T)
>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> include/linux/cleanup.h:358:20: note: expanded from macro '__DEFINE_GUARD_LOCK_PTR'
>   358 |         static inline int class_##_name##_lock_err(class_##_name##_t *_T)   \
>       |                           ^~~~~~~~~~~~~~~~~~~~~~~~
> <scratch space>:81:1: note: expanded from here
>    81 | class_pwmchip_lock_err
>       | ^~~~~~~~~~~~~~~~~~~~~~
> 1 error generated.

Ok, so in these cases where the guard is unconditional, the _lock_err()
method can be safely ignored. Here is a quick patch (below), only gcc
compile tested, that tries to quiet the warning for unconditional
guards, but keep the warning for conditional guards.

If you have some time to try it out, great, otherwise I will circle back
with a clang W=1 compile test before submitting the formal patch:

-- 8< --
diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h
index d8e7d1e5561b..4f5b719d0117 100644
--- a/include/linux/cleanup.h
+++ b/include/linux/cleanup.h
@@ -347,26 +347,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); \
@@ -465,7 +469,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)	\

Re: [PATCH] cleanup: Fix unused guard error function with DEFINE_CLASS_IS_COND_GUARD

[Andy Shevchenko]

On Mon, Aug 11, 2025 at 03:56:10PM -0700, dan.j.williams@intel.com wrote:
> Andy Shevchenko wrote:
> > On Mon, Aug 04, 2025 at 03:09:54PM -0700, Dan Williams wrote:
> > > 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. Per Nathan, clang catches unused "static inline"
> > > functions in C files since commit 6863f5643dd7 ("kbuild: allow Clang to
> > > find unused static inline functions for W=1 build").
> > > 
> > > There are 2 ways to solve this, either mark the class_##_lock_err()
> > > function as __maybe_unused, or make sure class_##_lock_err() *is* used /
> > > gets called to disposition the lock status.
> > > 
> > > At present __lock_timer() only indicates failure with a NULL __guard_ptr().
> > > However, one could imagine that __lock_timer(), or some other custom
> > > conditional locking primitive, wants to pass an ERR_PTR() to indicate the
> > > reason for the lock acquisition failure.
> > > 
> > > Update __scoped_cond_guard() to check for ERR_PTR() in addition to NULL
> > > @scope values. This allows __lock_timer(), or another open coded
> > > DEFINE_CLASS_IS_COND_GUARD() user, to switch to passing an ERR_PTR() in the
> > > future. In the meantime, this just silences the warning.
> > 
> > Hmm... It seems fixes the timer case, but left others still fail:
> > 
> > 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))
> >       | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > include/linux/cleanup.h:380:2: note: expanded from macro 'DEFINE_GUARD'
> >   380 |         DEFINE_CLASS_IS_GUARD(_name)
> >       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > include/linux/cleanup.h:372:2: note: expanded from macro 'DEFINE_CLASS_IS_GUARD'
> >   372 |         __DEFINE_GUARD_LOCK_PTR(_name, _T)
> >       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > include/linux/cleanup.h:358:20: note: expanded from macro '__DEFINE_GUARD_LOCK_PTR'
> >   358 |         static inline int class_##_name##_lock_err(class_##_name##_t *_T)   \
> >       |                           ^~~~~~~~~~~~~~~~~~~~~~~~
> > <scratch space>:81:1: note: expanded from here
> >    81 | class_pwmchip_lock_err
> >       | ^~~~~~~~~~~~~~~~~~~~~~
> > 1 error generated.
> 
> Ok, so in these cases where the guard is unconditional, the _lock_err()
> method can be safely ignored. Here is a quick patch (below), only gcc
> compile tested, that tries to quiet the warning for unconditional
> guards, but keep the warning for conditional guards.
> 
> If you have some time to try it out, great, otherwise I will circle back
> with a clang W=1 compile test before submitting the formal patch:

I have applied it on top of the previous (incomplete) solution and tried on two
branches, one is based on v6.17-rc1 and the other on Linux Next. I do not see
problem anymore.

Tested-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] cleanup: Fix unused guard error function with DEFINE_CLASS_IS_COND_GUARD

[Peter Zijlstra]

On Mon, Aug 04, 2025 at 03:09:54PM -0700, Dan Williams wrote:
> 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. Per Nathan, clang catches unused "static inline"
> functions in C files since commit 6863f5643dd7 ("kbuild: allow Clang to
> find unused static inline functions for W=1 build").

I so loathe that warning :/

> There are 2 ways to solve this, either mark the class_##_lock_err()
> function as __maybe_unused, or make sure class_##_lock_err() *is* used /
> gets called to disposition the lock status.
> 
> At present __lock_timer() only indicates failure with a NULL __guard_ptr().
> However, one could imagine that __lock_timer(), or some other custom
> conditional locking primitive, wants to pass an ERR_PTR() to indicate the
> reason for the lock acquisition failure.
> 
> Update __scoped_cond_guard() to check for ERR_PTR() in addition to NULL
> @scope values. This allows __lock_timer(), or another open coded
> DEFINE_CLASS_IS_COND_GUARD() user, to switch to passing an ERR_PTR() in the
> future. In the meantime, this just silences the warning.
> 
> 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
> Fixes: 857d18f23ab1 ("cleanup: Introduce ACQUIRE() and ACQUIRE_ERR() for conditional locks")
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> Dave, I am sending this to you to take upstream since the warning came
> in through the CXL tree. If anyone else wants to take it just holler.
> 
>  include/linux/cleanup.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h
> index 4eb83dd71cfe..d8e7d1e5561b 100644
> --- a/include/linux/cleanup.h
> +++ b/include/linux/cleanup.h
> @@ -423,7 +423,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:									\

What does this do for code generation ?

Re: [PATCH] cleanup: Fix unused guard error function with DEFINE_CLASS_IS_COND_GUARD

[Linus Torvalds]

On Wed, 13 Aug 2025 at 07:13, Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Mon, Aug 04, 2025 at 03:09:54PM -0700, Dan Williams wrote:
> >
> > ...with a clang W=1 build. Per Nathan, clang catches unused "static inline"
> > functions in C files since commit 6863f5643dd7 ("kbuild: allow Clang to
> > find unused static inline functions for W=1 build").
>
> I so loathe that warning :/

So I entirely ignore W=1 issues, because I think so many of the extra
warnings are bogus.

But if this one in particular is causing more problems than most -
some teams do seem to use W=1 as part of their test builds - it's fine
to send me a patch that just moves bad warnings to W=2.

And if anybody uses W=2 for their test builds, that's THEIR problem..

          Linus

Re: [PATCH] cleanup: Fix unused guard error function with DEFINE_CLASS_IS_COND_GUARD

[Peter Zijlstra]

On Wed, Aug 13, 2025 at 08:14:59AM -0700, Linus Torvalds wrote:
> On Wed, 13 Aug 2025 at 07:13, Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Mon, Aug 04, 2025 at 03:09:54PM -0700, Dan Williams wrote:
> > >
> > > ...with a clang W=1 build. Per Nathan, clang catches unused "static inline"
> > > functions in C files since commit 6863f5643dd7 ("kbuild: allow Clang to
> > > find unused static inline functions for W=1 build").
> >
> > I so loathe that warning :/
> 
> So I entirely ignore W=1 issues, because I think so many of the extra
> warnings are bogus.
> 
> But if this one in particular is causing more problems than most -
> some teams do seem to use W=1 as part of their test builds - it's fine
> to send me a patch that just moves bad warnings to W=2.
> 
> And if anybody uses W=2 for their test builds, that's THEIR problem..

Right, so I've had patches that added extra #ifdef gunk because of this,
and we've sprinked __maybe_unused in a few places just to shut this up.

I think something like the below will do... totally untested and all
that.

diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 16755431fc11..12b8b5d81459 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -253,7 +253,7 @@ struct ftrace_likely_data {
  * for W=1 build. This will allow clang to find unused functions. Remove the
  * __inline_maybe_unused entirely after fixing most of -Wunused-function warnings.
  */
-#ifdef KBUILD_EXTRA_WARN1
+#ifdef KBUILD_EXTRA_WARN2
 #define __inline_maybe_unused
 #else
 #define __inline_maybe_unused __maybe_unused

Re: [PATCH] cleanup: Fix unused guard error function with DEFINE_CLASS_IS_COND_GUARD

[Linus Torvalds]

On Wed, 13 Aug 2025 at 08:21, Peter Zijlstra <peterz@infradead.org> wrote:
>
> Right, so I've had patches that added extra #ifdef gunk because of this,
> and we've sprinked __maybe_unused in a few places just to shut this up.
>
> I think something like the below will do... totally untested and all
> that.

Yeah. I'm not convinced we want it even at W=2, but as mentioned, at
that point hopefully nobody really cares.

             Linus

Re: [PATCH] cleanup: Fix unused guard error function with DEFINE_CLASS_IS_COND_GUARD

[dan.j.williams]

Peter Zijlstra wrote:
> On Mon, Aug 04, 2025 at 03:09:54PM -0700, Dan Williams wrote:
> > 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. Per Nathan, clang catches unused "static inline"
> > functions in C files since commit 6863f5643dd7 ("kbuild: allow Clang to
> > find unused static inline functions for W=1 build").
> 
> I so loathe that warning :/

The only case I can think of where it might have some value, if you
squint, is in catching cases where someone defines a conditional guard,
but then somehow fails to use either scoped_cond_guard() or ACQUIRE().

I.e. guard(lock_timer)(timer_id), does not generate a compile time
warning.

However, if someone accidentally mixes guard() and ACQUIRE() in the same
C file, then the warning will not fire.

> 
> > There are 2 ways to solve this, either mark the class_##_lock_err()
> > function as __maybe_unused, or make sure class_##_lock_err() *is* used /
> > gets called to disposition the lock status.
> > 
> > At present __lock_timer() only indicates failure with a NULL __guard_ptr().
> > However, one could imagine that __lock_timer(), or some other custom
> > conditional locking primitive, wants to pass an ERR_PTR() to indicate the
> > reason for the lock acquisition failure.
> > 
> > Update __scoped_cond_guard() to check for ERR_PTR() in addition to NULL
> > @scope values. This allows __lock_timer(), or another open coded
> > DEFINE_CLASS_IS_COND_GUARD() user, to switch to passing an ERR_PTR() in the
> > future. In the meantime, this just silences the warning.
> > 
> > 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
> > Fixes: 857d18f23ab1 ("cleanup: Introduce ACQUIRE() and ACQUIRE_ERR() for conditional locks")
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > ---
> > Dave, I am sending this to you to take upstream since the warning came
> > in through the CXL tree. If anyone else wants to take it just holler.
> > 
> >  include/linux/cleanup.h | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h
> > index 4eb83dd71cfe..d8e7d1e5561b 100644
> > --- a/include/linux/cleanup.h
> > +++ b/include/linux/cleanup.h
> > @@ -423,7 +423,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:									\
> 
> What does this do for code generation ?

size(1) identical:

   text	   data	    bss	    dec	    hex	filename
  54648	  20439	     64	  75151	  1258f	kernel/time/posix-timers.o

Within that same size it flips the test to a negative branch and reorders the code.

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>

Re: [PATCH] cleanup: Fix unused guard error function with DEFINE_CLASS_IS_COND_GUARD

[Andy Shevchenko]

On Wed, Aug 13, 2025 at 08:14:59AM -0700, Linus Torvalds wrote:
> On Wed, 13 Aug 2025 at 07:13, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Mon, Aug 04, 2025 at 03:09:54PM -0700, Dan Williams wrote:
> > >
> > > ...with a clang W=1 build. Per Nathan, clang catches unused "static inline"
> > > functions in C files since commit 6863f5643dd7 ("kbuild: allow Clang to
> > > find unused static inline functions for W=1 build").
> >
> > I so loathe that warning :/
> 
> So I entirely ignore W=1 issues, because I think so many of the extra
> warnings are bogus.
> 
> But if this one in particular is causing more problems than most -
> some teams do seem to use W=1 as part of their test builds - it's fine
> to send me a patch that just moves bad warnings to W=2.
> 
> And if anybody uses W=2 for their test builds, that's THEIR problem..

Exactly. I am only aware of W=0/W=1, I think I run W=2 once or twice in my life.

-- 
With Best Regards,
Andy Shevchenko