Re: [PATCH v2] cleanup: Remove NULL check from unconditional guards

[Peter Zijlstra]

On Tue, May 12, 2026 at 07:15:10AM +0000, Dmitry Ilvokhin wrote:
> The unconditional guard destructors check whether the lock pointer is
> NULL before unlocking. This check is dead code because unconditional
> guards guarantee a non-NULL lock pointer at destructor time.
> 
> DEFINE_GUARD() and DEFINE_LOCK_GUARD_1() both run the lock operation
> in the constructor before returning. If the pointer were NULL, the
> lock operation (e.g. mutex_lock(NULL)) would crash before the
> constructor returns. The destructor never runs with a NULL pointer.
> 
> DEFINE_LOCK_GUARD_0() hardcodes .lock = (void *)1 in the constructor,
> so it is never NULL by construction.
> 
> Conditional (_try) variants: DEFINE_GUARD_COND() and
> DEFINE_LOCK_GUARD_1_COND() use EXTEND_CLASS_COND(), whose wrapper
> destructor returns early when the lock was not acquired, before reaching
> the base destructor since 2deccd5c862a ("cleanup: Optimize guards"):
> 
>     if (_cond) return; class_##_name##_destructor(_T);
> 
> As compiled by GCC-11 with defconfig on top of the locking/core:
> 
>     Total: Before=23889980, After=23833993, chg -0.23%
> 
> Signed-off-by: Dmitry Ilvokhin <d@xxxxxxxxxxxx>
> ---
> Changes in v2:
> 
> - Expand commit message with detailed reasoning, why the proposed
>   change is correct.
> - Rebase on top of locking/core.
> 
> v1: https://lore.kernel.org/all/20260427165037.205337-1-d@xxxxxxxxxxxx/
> 
> See also [1] for relevant discussion.
> 
> [1]: https://lore.kernel.org/all/afCS4d4YccQFtvpi@xxxxxxxxxxxxxxxxxx/
> 
>  include/linux/cleanup.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h
> index ea95ca4bc11c..1410effa8780 100644
> --- a/include/linux/cleanup.h
> +++ b/include/linux/cleanup.h
> @@ -397,7 +397,7 @@ static __maybe_unused const bool class_##_name##_is_conditional = _is_cond
>  	__DEFINE_GUARD_LOCK_PTR(_name, _T)
>  
>  #define DEFINE_GUARD(_name, _type, _lock, _unlock) \
> -	DEFINE_CLASS(_name, _type, if (_T) { _unlock; }, ({ _lock; _T; }), _type _T); \
> +	DEFINE_CLASS(_name, _type, _unlock, ({ _lock; _T; }), _type _T); \
>  	DEFINE_CLASS_IS_GUARD(_name)
>  
>  #define DEFINE_GUARD_COND_4(_name, _ext, _lock, _cond) \
> @@ -491,7 +491,7 @@ typedef struct {							\
>  static __always_inline void class_##_name##_destructor(class_##_name##_t *_T) \
>  	__no_context_analysis						\
>  {									\
> -	if (_T->lock) { _unlock; }					\
> +	_unlock;							\
>  }									\
>  									\
>  __DEFINE_GUARD_LOCK_PTR(_name, &_T->lock)

What about class_irqdesc_lock_constructor() ? AFAICT
__irq_get_desc_lock() can return NULL.