All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Dan Williams <dan.j.williams@intel.com>
Cc: dave.jiang@intel.com, linux-kernel@vger.kernel.org,
	linux-cxl@vger.kernel.org, Nathan Chancellor <nathan@kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	David Lechner <dlechner@baylibre.com>,
	Jonathan Cameron <jonathan.cameron@huawei.com>,
	Andy Shevchenko <andriy.shevchenko@intel.com>
Subject: Re: [PATCH] cleanup: Fix unused guard error function with DEFINE_CLASS_IS_COND_GUARD
Date: Wed, 13 Aug 2025 16:13:46 +0200	[thread overview]
Message-ID: <20250813141346.GM4067720@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <20250804220955.1453135-1-dan.j.williams@intel.com>

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 ?

  parent reply	other threads:[~2025-08-13 14:13 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-04 22:09 [PATCH] cleanup: Fix unused guard error function with DEFINE_CLASS_IS_COND_GUARD Dan Williams
2025-08-05 22:38 ` Andy Shevchenko
2025-08-11 22:56   ` dan.j.williams
2025-08-13 13:46     ` Andy Shevchenko
2025-08-09  9:46 ` Andy Shevchenko
2025-08-13 14:13 ` Peter Zijlstra [this message]
2025-08-13 15:14   ` Linus Torvalds
2025-08-13 15:21     ` Peter Zijlstra
2025-08-13 15:27       ` Linus Torvalds
2025-08-13 21:45     ` Andy Shevchenko
2025-08-13 18:57   ` dan.j.williams

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250813141346.GM4067720@noisy.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=andriy.shevchenko@intel.com \
    --cc=dan.j.williams@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=dlechner@baylibre.com \
    --cc=jonathan.cameron@huawei.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nathan@kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.