[PATCH v3 0/2] Add scope-based cleanup capability to eventfd_ctx

[PATCH v3 0/2] Add scope-based cleanup capability to eventfd_ctx

[Bence Csokas]

Add __free() support to eventfd and update drm_syncobj to use it as an
example consumer.

A minor discrepancy I encountered is whether to put a semicolon after
DEFINE_FREE(). It seems some in-tree code does, some doesn't. In the end,
I followed the example set in core-api/cleanup.rst, and omitted it.

Signed-off-by: Bence Csokas <bence.csokas@arm.com>
---
Changes in v3:
- IS_ERR_OR_NULL() moved into DEFINE_FREE() body
- Link to v2: https://patch.msgid.link/20260915-efd-autofree-v2-0-06d2c9d1547f@arm.com

Changes in v2:
- IS_ERR_OR_NULL() check, instead of NULLing out pointer on IS_ERR
- Link to v1: https://patch.msgid.link/20260909-efd-autofree-v1-0-c6c3c19cc702@arm.com

To: Alexander Viro <viro@zeniv.linux.org.uk>
To: Christian Brauner <brauner@kernel.org>
To: Jan Kara <jack@suse.cz>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: Maxime Ripard <mripard@kernel.org>
To: Thomas Zimmermann <tzimmermann@suse.de>
To: David Airlie <airlied@gmail.com>
To: Simona Vetter <simona@ffwll.ch>
Cc: linux-fsdevel@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org

---
Bence Csokas (2):
      eventfd: Add scope-based cleanup capability
      drm/syncobj: Use scope-based cleanup for eventfd_ctx

 drivers/gpu/drm/drm_syncobj.c | 8 +++-----
 include/linux/eventfd.h       | 3 +++
 2 files changed, 6 insertions(+), 5 deletions(-)
---
base-commit: d135ab453a7400a34930bdda1ef4bdc8acfac80e
change-id: 20260826-efd-autofree-766608e8d8aa

Best regards,
--  
Bence Csokas <bence.csokas@arm.com>

[PATCH v3 1/2] eventfd: Add scope-based cleanup capability

[Bence Csokas]

Allow eventfd_ctx_put() to be used with the RAII-inspired __free() API.

Signed-off-by: Bence Csokas <bence.csokas@arm.com>
---
 include/linux/eventfd.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/eventfd.h b/include/linux/eventfd.h
index e32bee4345fb..48c40f8c4dfd 100644
--- a/include/linux/eventfd.h
+++ b/include/linux/eventfd.h
@@ -10,6 +10,7 @@
 #define _LINUX_EVENTFD_H
 
 #include <linux/wait.h>
+#include <linux/cleanup.h>
 #include <linux/err.h>
 #include <linux/percpu-defs.h>
 #include <linux/percpu.h>
@@ -89,5 +90,7 @@ static inline void eventfd_signal(struct eventfd_ctx *ctx)
 	eventfd_signal_mask(ctx, 0);
 }
 
+DEFINE_FREE(eventfd, struct eventfd_ctx *, if (!IS_ERR_OR_NULL(_T)) eventfd_ctx_put(_T))
+
 #endif /* _LINUX_EVENTFD_H */
 

-- 
2.56.0

[PATCH v3 2/2] drm/syncobj: Use scope-based cleanup for eventfd_ctx

[Bence Csokas]

Now that eventfd supports __free(), use it in drm_syncobj_eventfd_ioctl().

Signed-off-by: Bence Csokas <bence.csokas@arm.com>
---
 drivers/gpu/drm/drm_syncobj.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index f1d4da168545..40bedd30b2fc 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1429,9 +1429,9 @@ int
 drm_syncobj_eventfd_ioctl(struct drm_device *dev, void *data,
 			  struct drm_file *file_private)
 {
+	struct eventfd_ctx *ev_fd_ctx __free(eventfd) = NULL;
 	struct drm_syncobj_eventfd *args = data;
 	struct drm_syncobj *syncobj;
-	struct eventfd_ctx *ev_fd_ctx;
 	struct syncobj_eventfd_entry *entry;
 	int ret;
 
@@ -1457,10 +1457,10 @@ drm_syncobj_eventfd_ioctl(struct drm_device *dev, void *data,
 	entry = kzalloc_obj(*entry);
 	if (!entry) {
 		ret = -ENOMEM;
-		goto err_kzalloc;
+		goto err_fdget;
 	}
 	entry->syncobj = syncobj;
-	entry->ev_fd_ctx = ev_fd_ctx;
+	entry->ev_fd_ctx = no_free_ptr(ev_fd_ctx);
 	entry->point = args->point;
 	entry->flags = args->flags;
 
@@ -1469,8 +1469,6 @@ drm_syncobj_eventfd_ioctl(struct drm_device *dev, void *data,
 
 	return 0;
 
-err_kzalloc:
-	eventfd_ctx_put(ev_fd_ctx);
 err_fdget:
 	drm_syncobj_put(syncobj);
 	return ret;

-- 
2.56.0

Re: [PATCH v3 2/2] drm/syncobj: Use scope-based cleanup for eventfd_ctx

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] drm/syncobj: mixing __free() and goto cleanup in drm_syncobj_eventfd_ioctl()
--

commit 235e12bb4f2858e1c0f1b573b50d4958dd229203
Author: Bence Csokas <bence.csokas@arm.com>

drm/syncobj: Use scope-based cleanup for eventfd_ctx

This commit converts ev_fd_ctx in drm_syncobj_eventfd_ioctl() to use
__free(eventfd) and no_free_ptr(), removing the err_kzalloc error label.

> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index f1d4da1685457..40bedd30b2fc0 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -1429,9 +1429,9 @@ int
>  drm_syncobj_eventfd_ioctl(struct drm_device *dev, void *data,
>  			  struct drm_file *file_private)
>  {
> +	struct eventfd_ctx *ev_fd_ctx __free(eventfd) = NULL;
>  	struct drm_syncobj_eventfd *args = data;
>  	struct drm_syncobj *syncobj;
> -	struct eventfd_ctx *ev_fd_ctx;
>  	struct syncobj_eventfd_entry *entry;
>  	int ret;
>  
> @@ -1457,10 +1457,10 @@ drm_syncobj_eventfd_ioctl(struct drm_device *dev, void *data,
>  	entry = kzalloc_obj(*entry);
>  	if (!entry) {
>  		ret = -ENOMEM;
> -		goto err_kzalloc;
> +		goto err_fdget;
>  	}
>  	entry->syncobj = syncobj;
> -	entry->ev_fd_ctx = ev_fd_ctx;
> +	entry->ev_fd_ctx = no_free_ptr(ev_fd_ctx);
>  	entry->point = args->point;
>  	entry->flags = args->flags;
>  
> @@ -1469,8 +1469,6 @@ drm_syncobj_eventfd_ioctl(struct drm_device *dev, void *data,
>  
>  	return 0;
>  
> -err_kzalloc:
> -	eventfd_ctx_put(ev_fd_ctx);
>  err_fdget:
>  	drm_syncobj_put(syncobj);
>  	return ret;

[Severity: Low]
In drivers/gpu/drm/drm_syncobj.c, drm_syncobj_eventfd_ioctl() declares
ev_fd_ctx with __free(eventfd) = NULL at the top of the function while
retaining goto err_fdget to clean up syncobj.

When kzalloc_obj(*entry) fails, jumping to err_fdget runs
drm_syncobj_put(syncobj) before scope exit calls eventfd_ctx_put(ev_fd_ctx),
which reverses the teardown order relative to acquisition:

	syncobj = drm_syncobj_find(file_private, args->handle);
	if (!syncobj)
		return -ENOENT;

	ev_fd_ctx = eventfd_ctx_fdget(args->fd);
	if (IS_ERR(ev_fd_ctx)) {
		ret = PTR_ERR(ev_fd_ctx);
		goto err_fdget;
	}

	entry = kzalloc_obj(*entry);
	if (!entry) {
		ret = -ENOMEM;
		goto err_fdget;
	}

The documentation in include/linux/cleanup.h notes that __free() variables
should be defined and initialized in one statement rather than grouped at the
top with = NULL, and states that goto cleanup and scope-based cleanup helpers
should not be mixed in the same function:

 * Given that the "__free(...) = NULL" pattern for variables defined at
 * the top of the function poses this potential interdependency problem
 * the recommendation is to always define and assign variables in one
 * statement and not group variable definitions at the top of the
 * function when __free() is used.
 *
 * Lastly, given that the benefit of cleanup helpers is removal of
 * "goto", and that the "goto" statement can jump between scopes, the
 * expectation is that usage of "goto" and cleanup helpers is never
 * mixed in the same function. I.e. for a given routine, convert all
 * resources that need a "goto" cleanup to scope-based cleanup, or
 * convert none of them.

Should drm_syncobj_eventfd_ioctl() either convert syncobj to scope-based
cleanup as well (or restructure the acquisition order so goto err_fdget is no
longer needed), and define ev_fd_ctx at the point where eventfd_ctx_fdget()
is called?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027-efd-autofree-v3-0-caa2442e1beb@arm.com?part=2