[PATCH v3 0/4] cleanup: Remove NULL check from unconditional guards

[PATCH v3 0/4] cleanup: Remove NULL check from unconditional guards

[Dmitry Ilvokhin]

Unconditional guard destructors have dead NULL checks. The lock operation in
the constructor would crash before the destructor ever runs with NULL.

- Patches 1-2 prepare guards that legitimately handle NULL.
- Patch 3 adds __nonnull() to guard constructors for compile-time enforcement.
- Patch 4 removes the dead checks.

As compiled by GCC-11 with defconfig on top of the locking/core:

    Total: Before=23889980, After=23834334, chg -0.23%

Changes in v3:

- Audited usages of DEFINE_GUARD(), __DEFINE_UNLOCK_GUARD() and
  DEFINE_LOCK_GUARD_1() to make sure NULL check removal will work correctly
  (Peter Zijlstra).
- Moved NULL check into irqdesc_lock unlock expression (Peter Zijlstra).
- Added compiler-enforced nonnull() check for guard constructors.
- Converted nvdimm_bus guard to class.

Changes in v2:

- Expand commit message with detailed reasoning, why the proposed
  change is correct.
- Rebase on top of locking/core.

v2: https://lore.kernel.org/all/20260512071510.92451-1-d@ilvokhin.com/
v1: https://lore.kernel.org/all/20260427165037.205337-1-d@ilvokhin.com/

See also [1] for relevant discussion.

[1]: https://lore.kernel.org/all/afCS4d4YccQFtvpi@shell.ilvokhin.com/

Dmitry Ilvokhin (4):
  nvdimm: Convert nvdimm_bus guard to class
  genirq: Move NULL check into irqdesc_lock guard unlock expression
  cleanup: Annotate guard constructors with __nonnull()
  cleanup: Remove NULL check from unconditional guards

 drivers/nvdimm/nd.h                 | 7 +++++--
 include/linux/cleanup.h             | 8 +++++---
 include/linux/compiler_attributes.h | 6 ++++++
 kernel/irq/internals.h              | 2 +-
 4 files changed, 17 insertions(+), 6 deletions(-)

-- 
2.53.0-Meta

[PATCH v3 1/4] nvdimm: Convert nvdimm_bus guard to class

[Dmitry Ilvokhin]

The nvdimm_bus guard accepts NULL and skips locking when NULL is passed.
Convert from DEFINE_GUARD() to DEFINE_CLASS() + DEFINE_CLASS_IS_GUARD().

This is a preparatory change for making DEFINE_GUARD() constructors
__nonnull(). nvdimm_bus legitimately passes NULL, so it must be adjusted
to avoid a compile error.

No functional change.

Signed-off-by: Dmitry Ilvokhin <d@ilvokhin.com>
---
 drivers/nvdimm/nd.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
index b199eea3260e..18b64559664b 100644
--- a/drivers/nvdimm/nd.h
+++ b/drivers/nvdimm/nd.h
@@ -632,8 +632,11 @@ u64 nd_region_interleave_set_cookie(struct nd_region *nd_region,
 u64 nd_region_interleave_set_altcookie(struct nd_region *nd_region);
 void nvdimm_bus_lock(struct device *dev);
 void nvdimm_bus_unlock(struct device *dev);
-DEFINE_GUARD(nvdimm_bus, struct device *,
-	     if (_T) nvdimm_bus_lock(_T), if (_T) nvdimm_bus_unlock(_T));
+DEFINE_CLASS(nvdimm_bus, struct device *,
+	     if (_T) nvdimm_bus_unlock(_T),
+	     ({ if (_T) nvdimm_bus_lock(_T); _T; }),
+	     struct device *_T);
+DEFINE_CLASS_IS_GUARD(nvdimm_bus);
 
 bool is_nvdimm_bus_locked(struct device *dev);
 void nvdimm_check_and_set_ro(struct gendisk *disk);
-- 
2.53.0-Meta

Re: [PATCH v3 1/4] nvdimm: Convert nvdimm_bus guard to class

[Dave Jiang]

On 5/18/26 8:21 AM, Dmitry Ilvokhin wrote:
> The nvdimm_bus guard accepts NULL and skips locking when NULL is passed.
> Convert from DEFINE_GUARD() to DEFINE_CLASS() + DEFINE_CLASS_IS_GUARD().
> 
> This is a preparatory change for making DEFINE_GUARD() constructors
> __nonnull(). nvdimm_bus legitimately passes NULL, so it must be adjusted
> to avoid a compile error.
> 
> No functional change.
> 
> Signed-off-by: Dmitry Ilvokhin <d@ilvokhin.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/nvdimm/nd.h | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
> index b199eea3260e..18b64559664b 100644
> --- a/drivers/nvdimm/nd.h
> +++ b/drivers/nvdimm/nd.h
> @@ -632,8 +632,11 @@ u64 nd_region_interleave_set_cookie(struct nd_region *nd_region,
>  u64 nd_region_interleave_set_altcookie(struct nd_region *nd_region);
>  void nvdimm_bus_lock(struct device *dev);
>  void nvdimm_bus_unlock(struct device *dev);
> -DEFINE_GUARD(nvdimm_bus, struct device *,
> -	     if (_T) nvdimm_bus_lock(_T), if (_T) nvdimm_bus_unlock(_T));
> +DEFINE_CLASS(nvdimm_bus, struct device *,
> +	     if (_T) nvdimm_bus_unlock(_T),
> +	     ({ if (_T) nvdimm_bus_lock(_T); _T; }),
> +	     struct device *_T);
> +DEFINE_CLASS_IS_GUARD(nvdimm_bus);
>  
>  bool is_nvdimm_bus_locked(struct device *dev);
>  void nvdimm_check_and_set_ro(struct gendisk *disk);

[PATCH v3 2/4] genirq: Move NULL check into irqdesc_lock guard unlock expression

[Dmitry Ilvokhin]

irqdesc_lock uses __DEFINE_UNLOCK_GUARD() directly with a custom
constructor that can set .lock to NULL.

In preparation for removing the NULL check from __DEFINE_UNLOCK_GUARD(),
move the NULL check into the irqdesc_lock unlock expression, making the
NULL handling explicit at the call site.

No functional change.

Signed-off-by: Dmitry Ilvokhin <d@ilvokhin.com>
---
 kernel/irq/internals.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
index 9412e57056f5..347cb333b9fe 100644
--- a/kernel/irq/internals.h
+++ b/kernel/irq/internals.h
@@ -171,7 +171,7 @@ void __irq_put_desc_unlock(struct irq_desc *desc, unsigned long flags, bool bus)
 
 __DEFINE_CLASS_IS_CONDITIONAL(irqdesc_lock, true);
 __DEFINE_UNLOCK_GUARD(irqdesc_lock, struct irq_desc,
-		      __irq_put_desc_unlock(_T->lock, _T->flags, _T->bus),
+		      if (_T->lock) __irq_put_desc_unlock(_T->lock, _T->flags, _T->bus),
 		      unsigned long flags; bool bus);
 
 static inline class_irqdesc_lock_t class_irqdesc_lock_constructor(unsigned int irq, bool bus,
-- 
2.53.0-Meta

[PATCH v3 3/4] cleanup: Annotate guard constructors with __nonnull()

[Dmitry Ilvokhin]

Add __nonnull() to unconditional guard constructors so the compiler
verifies at each call site that NULL is never passed:

- DEFINE_GUARD(): re-declare the constructor with __nonnull().
- __DEFINE_LOCK_GUARD_1(): annotate the constructor directly.

DEFINE_LOCK_GUARD_0() needs no annotation: its constructor takes no
pointer arguments (.lock is hardcoded to (void *)1).

This provides automated, compiler-enforced verification that no
unconditional guard constructor receives NULL.

Define the __nonnull() macro in compiler_attributes.h, following the
existing convention for attribute wrappers.

Signed-off-by: Dmitry Ilvokhin <d@ilvokhin.com>
---
 include/linux/cleanup.h             | 4 +++-
 include/linux/compiler_attributes.h | 6 ++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h
index ea95ca4bc11c..8f8d588b5595 100644
--- a/include/linux/cleanup.h
+++ b/include/linux/cleanup.h
@@ -397,6 +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) \
+	static __always_inline __nonnull() _type class_##_name##_constructor(_type _T); \
 	DEFINE_CLASS(_name, _type, if (_T) { _unlock; }, ({ _lock; _T; }), _type _T); \
 	DEFINE_CLASS_IS_GUARD(_name)
 
@@ -497,7 +498,8 @@ static __always_inline void class_##_name##_destructor(class_##_name##_t *_T) \
 __DEFINE_GUARD_LOCK_PTR(_name, &_T->lock)
 
 #define __DEFINE_LOCK_GUARD_1(_name, _type, ...)			\
-static __always_inline class_##_name##_t class_##_name##_constructor(_type *l) \
+static __always_inline __nonnull()					\
+class_##_name##_t class_##_name##_constructor(_type *l)			\
 	__no_context_analysis						\
 {									\
 	class_##_name##_t _t = { .lock = l }, *_T = &_t;		\
diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
index c16d4199bf92..85f08d6137a2 100644
--- a/include/linux/compiler_attributes.h
+++ b/include/linux/compiler_attributes.h
@@ -176,6 +176,12 @@
  */
 #define __mode(x)                       __attribute__((__mode__(x)))
 
+/*
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-nonnull-function-attribute
+ * clang: https://clang.llvm.org/docs/AttributeReference.html#nonnull
+ */
+#define __nonnull(x...)			__attribute__((__nonnull__(x)))
+
 /*
  * Optional: only supported since gcc >= 7
  *
-- 
2.53.0-Meta

Re: [PATCH v3 3/4] cleanup: Annotate guard constructors with __nonnull()

[Miguel Ojeda]

On Mon, May 18, 2026 at 5:22 PM Dmitry Ilvokhin <d@ilvokhin.com> wrote:
>
> Add __nonnull() to unconditional guard constructors so the compiler
> verifies at each call site that NULL is never passed:

> This provides automated, compiler-enforced verification that no
> unconditional guard constructor receives NULL.

I wouldn't say "verify", since the compiler does a best-effort here
with the information it has statically.

In other words, the attribute does not prevent NULL pointers to be passed.

> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-nonnull-function-attribute

Hmm... It appears GCC has changed the docs in commit 6e3c137f5dbb
("doc: Merge function, variable, type, and statement attribute
sections [PR88472]"), dropping the per-kind attribute pages.

So the right link would need to be now:

  https://gcc.gnu.org/onlinedocs/gcc/Common-Attributes.html#index-nonnull

I will need to send a patch to fix the other links.

> + * clang: https://clang.llvm.org/docs/AttributeReference.html#nonnull

I think this link goes to `_Nonnull` -- the GNU one is instead:

  https://clang.llvm.org/docs/AttributeReference.html#id10

(I don't love the numeric IDs, though, since they break, so I think it
is fine either way -- the `_Nonnull` is fairly close to the one we
want and I hope that one doesn't break)

> + */
> +#define __nonnull(x...)                        __attribute__((__nonnull__(x)))

This is indeed available for a long time, and we already use it
elsewhere in the kernel tree (which would be nice to clean up
separately).

If you don't mind, please place it before `__nonstring__` (the file is
meant to be sorted by the actual attribute name -- there are a few
instances where this is not the case anymore, which I will eventually
clean up)

Thanks!

Cheers,
Miguel

Re: [PATCH v3 3/4] cleanup: Annotate guard constructors with __nonnull()

[Dmitry Ilvokhin]

On Mon, May 18, 2026 at 08:19:35PM +0200, Miguel Ojeda wrote:
> On Mon, May 18, 2026 at 5:22 PM Dmitry Ilvokhin <d@ilvokhin.com> wrote:
> >
> > Add __nonnull() to unconditional guard constructors so the compiler
> > verifies at each call site that NULL is never passed:
> 
> > This provides automated, compiler-enforced verification that no
> > unconditional guard constructor receives NULL.
> 
> I wouldn't say "verify", since the compiler does a best-effort here
> with the information it has statically.
> 
> In other words, the attribute does not prevent NULL pointers to be passed.

Fair enough.

I'll re-word this paragraph as "Add __nonnull() to unconditional guard
constructors so the compiler warns when NULL is statically known to be
passed" and drop the "compiler-enforced verification" paragraph.

> 
> > + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-nonnull-function-attribute
> 
> Hmm... It appears GCC has changed the docs in commit 6e3c137f5dbb
> ("doc: Merge function, variable, type, and statement attribute
> sections [PR88472]"), dropping the per-kind attribute pages.
> 
> So the right link would need to be now:
> 
>   https://gcc.gnu.org/onlinedocs/gcc/Common-Attributes.html#index-nonnull
> 
> I will need to send a patch to fix the other links.

Fixed locally. Thanks!

> 
> > + * clang: https://clang.llvm.org/docs/AttributeReference.html#nonnull
> 
> I think this link goes to `_Nonnull` -- the GNU one is instead:
> 
>   https://clang.llvm.org/docs/AttributeReference.html#id10
> 
> (I don't love the numeric IDs, though, since they break, so I think it
> is fine either way -- the `_Nonnull` is fairly close to the one we
> want and I hope that one doesn't break)

I don't quite like numeric IDs either. There is only one #id reference
in include/linux/compiler_attributes.h and link is already dead. I'll
keep current link since it gives at least some clue what to look for on
the page.

> 
> > + */
> > +#define __nonnull(x...)                        __attribute__((__nonnull__(x)))
> 
> This is indeed available for a long time, and we already use it
> elsewhere in the kernel tree (which would be nice to clean up
> separately).

> 
> If you don't mind, please place it before `__nonstring__` (the file is
> meant to be sorted by the actual attribute name -- there are a few
> instances where this is not the case anymore, which I will eventually
> clean up)

Thanks, fixed locally.

> 
> Thanks!
> 
> Cheers,
> Miguel

Re: [PATCH v3 3/4] cleanup: Annotate guard constructors with __nonnull()

[Miguel Ojeda]

On Tue, May 19, 2026 at 1:54 PM Dmitry Ilvokhin <d@ilvokhin.com> wrote:
>
> Thanks, fixed locally.

Sounds good, thanks!

Acked-by: Miguel Ojeda <ojeda@kernel.org>

Cheers,
Miguel

[PATCH v3 4/4] cleanup: Remove NULL check from unconditional guards

[Dmitry Ilvokhin]

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() runs the lock operation unconditionally in the
constructor. 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. All DEFINE_GUARD() users
dereference the pointer in their lock. Verified by auditing every
instance found by: git grep -n -A 1 'DEFINE_GUARD('. The only exception
is xe_pm_runtime_release_only, whose constructor is a noop, but it has
no callers.

__DEFINE_UNLOCK_GUARD() has only a few usages outside of
include/linux/cleanup.h: tty_port_tty (NULL-checks in its tty_kref_put()
call), irqdesc_lock (fixed earlier) and two guards in
kernel/sched/sched.h (dereference the pointer unconditionally in their
lock constructors).

DEFINE_LOCK_GUARD_1() sets .lock from its argument and runs the lock
operation in the constructor. Same reasoning applies. All
DEFINE_LOCK_GUARD_1() users dereference the pointer in their lock. Also,
verified by auditing every match of: git grep -n 'DEFINE_LOCK_GUARD_1('.

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 commit 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=23834334, chg -0.23%

Signed-off-by: Dmitry Ilvokhin <d@ilvokhin.com>
---
 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 8f8d588b5595..1f6d1a97617a 100644
--- a/include/linux/cleanup.h
+++ b/include/linux/cleanup.h
@@ -398,7 +398,7 @@ static __maybe_unused const bool class_##_name##_is_conditional = _is_cond
 
 #define DEFINE_GUARD(_name, _type, _lock, _unlock) \
 	static __always_inline __nonnull() _type class_##_name##_constructor(_type _T); \
-	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) \
@@ -492,7 +492,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)
-- 
2.53.0-Meta