* [PATCH v3 0/2] Add scope-based cleanup capability to eventfd_ctx
@ 2026-10-27 16:22 Bence Csokas
2026-10-27 16:22 ` [PATCH v3 1/2] eventfd: Add scope-based cleanup capability Bence Csokas
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Bence Csokas @ 2026-10-27 16:22 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Jan Kara, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: Bence Csokas, linux-fsdevel, linux-kernel, dri-devel
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>
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v3 1/2] eventfd: Add scope-based cleanup capability 2026-10-27 16:22 [PATCH v3 0/2] Add scope-based cleanup capability to eventfd_ctx Bence Csokas @ 2026-10-27 16:22 ` Bence Csokas 2026-10-27 16:22 ` [PATCH v3 2/2] drm/syncobj: Use scope-based cleanup for eventfd_ctx Bence Csokas 2026-11-06 9:05 ` [PATCH v3 0/2] Add scope-based cleanup capability to eventfd_ctx Christian Brauner 2 siblings, 0 replies; 8+ messages in thread From: Bence Csokas @ 2026-10-27 16:22 UTC (permalink / raw) To: Alexander Viro, Christian Brauner, Jan Kara, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter Cc: Bence Csokas, linux-fsdevel, linux-kernel, dri-devel 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 2/2] drm/syncobj: Use scope-based cleanup for eventfd_ctx 2026-10-27 16:22 [PATCH v3 0/2] Add scope-based cleanup capability to eventfd_ctx Bence Csokas 2026-10-27 16:22 ` [PATCH v3 1/2] eventfd: Add scope-based cleanup capability Bence Csokas @ 2026-10-27 16:22 ` Bence Csokas 2026-11-06 9:05 ` [PATCH v3 0/2] Add scope-based cleanup capability to eventfd_ctx Christian Brauner 2 siblings, 0 replies; 8+ messages in thread From: Bence Csokas @ 2026-10-27 16:22 UTC (permalink / raw) To: Alexander Viro, Christian Brauner, Jan Kara, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter Cc: Bence Csokas, linux-fsdevel, linux-kernel, dri-devel 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 0/2] Add scope-based cleanup capability to eventfd_ctx 2026-10-27 16:22 [PATCH v3 0/2] Add scope-based cleanup capability to eventfd_ctx Bence Csokas 2026-10-27 16:22 ` [PATCH v3 1/2] eventfd: Add scope-based cleanup capability Bence Csokas 2026-10-27 16:22 ` [PATCH v3 2/2] drm/syncobj: Use scope-based cleanup for eventfd_ctx Bence Csokas @ 2026-11-06 9:05 ` Christian Brauner 2026-11-06 9:31 ` Bence Csókás 2 siblings, 1 reply; 8+ messages in thread From: Christian Brauner @ 2026-11-06 9:05 UTC (permalink / raw) To: Bence Csokas Cc: Alexander Viro, Jan Kara, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, linux-fsdevel, linux-kernel, dri-devel On Tue, Oct 27, 2026 at 05:22:00PM +0100, Bence Csokas wrote: > 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> > --- I'm really having trouble seeing how this improves anything significantly. Probably others have the same impression which is why this doesn't get a lot of review. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 0/2] Add scope-based cleanup capability to eventfd_ctx 2026-11-06 9:05 ` [PATCH v3 0/2] Add scope-based cleanup capability to eventfd_ctx Christian Brauner @ 2026-11-06 9:31 ` Bence Csókás 2026-11-06 14:13 ` Jan Kara 0 siblings, 1 reply; 8+ messages in thread From: Bence Csókás @ 2026-11-06 9:31 UTC (permalink / raw) To: Christian Brauner Cc: Alexander Viro, Jan Kara, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, linux-fsdevel, linux-kernel, dri-devel Hi, On 2026. 11. 06. 10:05, Christian Brauner wrote: > On Tue, Oct 27, 2026 at 05:22:00PM +0100, Bence Csokas wrote: >> 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> >> --- > > I'm really having trouble seeing how this improves anything > significantly. Probably others have the same impression which is why > this doesn't get a lot of review. The goal of the __free() API is to reduce the number of goto-based error paths. Normally, you'd have to keep the state of every local in your head for every early return. (Has it been initialized? Has it been consumed? Or does it need to be freed?) Then you need to arrange your "goto tree" (error labels) in a way that frees everything, and then for every early exit, jump to the correct one. This can be quite complex for some functions. Look at KVM or HyperV eventfd code, for example. By contrast, you just mark the local var with _free() and an appropriate destructor snippet, and the consumption point (the point after which we don't need to free it on a return) with no_free_ptr(), and the compiler instruments every early return accordingly. Bence ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 0/2] Add scope-based cleanup capability to eventfd_ctx 2026-11-06 9:31 ` Bence Csókás @ 2026-11-06 14:13 ` Jan Kara 2026-11-06 14:30 ` Bence Csókás 0 siblings, 1 reply; 8+ messages in thread From: Jan Kara @ 2026-11-06 14:13 UTC (permalink / raw) To: Bence Csókás Cc: Christian Brauner, Alexander Viro, Jan Kara, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, linux-fsdevel, linux-kernel, dri-devel Hi! On Fri 06-11-26 10:31:58, Bence Csókás wrote: > On 2026. 11. 06. 10:05, Christian Brauner wrote: > > On Tue, Oct 27, 2026 at 05:22:00PM +0100, Bence Csokas wrote: > > > 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> > > > --- > > > > I'm really having trouble seeing how this improves anything > > significantly. Probably others have the same impression which is why > > this doesn't get a lot of review. > > The goal of the __free() API is to reduce the number of goto-based error > paths. Normally, you'd have to keep the state of every local in your head > for every early return. (Has it been initialized? Has it been consumed? Or > does it need to be freed?) Then you need to arrange your "goto tree" (error > labels) in a way that frees everything, and then for every early exit, jump > to the correct one. This can be quite complex for some functions. Look at > KVM or HyperV eventfd code, for example. > > By contrast, you just mark the local var with _free() and an appropriate > destructor snippet, and the consumption point (the point after which we > don't need to free it on a return) with no_free_ptr(), and the compiler > instruments every early return accordingly. Sure, but there's also the other side about which you don't speak: unless the destructor is trivial (like kfree()), there are implications about locking, when resources (such as references held by the object) get released etc. So it can also be more complex to wrap your head around __free usage than around standard cleanup code. So unless __free is providing a noticeable simplification of some goto hell, cleanup in many functions or similar, I generally don't find it worth it... Honza -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 0/2] Add scope-based cleanup capability to eventfd_ctx 2026-11-06 14:13 ` Jan Kara @ 2026-11-06 14:30 ` Bence Csókás 2026-11-06 15:02 ` Jan Kara 0 siblings, 1 reply; 8+ messages in thread From: Bence Csókás @ 2026-11-06 14:30 UTC (permalink / raw) To: Jan Kara Cc: Christian Brauner, Alexander Viro, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, linux-fsdevel, linux-kernel, dri-devel Hi, On 2026. 11. 06. 15:13, Jan Kara wrote: > Hi! > > On Fri 06-11-26 10:31:58, Bence Csókás wrote: >> On 2026. 11. 06. 10:05, Christian Brauner wrote: >>> On Tue, Oct 27, 2026 at 05:22:00PM +0100, Bence Csokas wrote: >>>> 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> >>>> --- >>> >>> I'm really having trouble seeing how this improves anything >>> significantly. Probably others have the same impression which is why >>> this doesn't get a lot of review. >> >> The goal of the __free() API is to reduce the number of goto-based error >> paths. Normally, you'd have to keep the state of every local in your head >> for every early return. (Has it been initialized? Has it been consumed? Or >> does it need to be freed?) Then you need to arrange your "goto tree" (error >> labels) in a way that frees everything, and then for every early exit, jump >> to the correct one. This can be quite complex for some functions. Look at >> KVM or HyperV eventfd code, for example. >> >> By contrast, you just mark the local var with _free() and an appropriate >> destructor snippet, and the consumption point (the point after which we >> don't need to free it on a return) with no_free_ptr(), and the compiler >> instruments every early return accordingly. > > Sure, but there's also the other side about which you don't speak: unless > the destructor is trivial (like kfree()), there are implications about > locking, when resources (such as references held by the object) get > released etc. So it can also be more complex to wrap your head around > __free usage than around standard cleanup code. Sure. Thankfully eventfd_ctx_put() is one of the pretty trivial ones, basically just calling kfree() on itself when the kref drops to 0. > So unless __free is providing a noticeable simplification of some goto > hell, cleanup in many functions or similar, I generally don't find it worth > it... > > Honza Look into for instance mshv_irqfd_assign() and weep... And then imagine how much simpler it'd be if it were using __free() instead. For the example in 2/2, I deliberately chose one callsite where it's trivial to deduce that behaviour is unchanged. Bence ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 0/2] Add scope-based cleanup capability to eventfd_ctx 2026-11-06 14:30 ` Bence Csókás @ 2026-11-06 15:02 ` Jan Kara 0 siblings, 0 replies; 8+ messages in thread From: Jan Kara @ 2026-11-06 15:02 UTC (permalink / raw) To: Bence Csókás Cc: Jan Kara, Christian Brauner, Alexander Viro, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, linux-fsdevel, linux-kernel, dri-devel Hi! On Fri 06-11-26 15:30:07, Bence Csókás wrote: > On 2026. 11. 06. 15:13, Jan Kara wrote: > > Hi! > > > > On Fri 06-11-26 10:31:58, Bence Csókás wrote: > > > On 2026. 11. 06. 10:05, Christian Brauner wrote: > > > > On Tue, Oct 27, 2026 at 05:22:00PM +0100, Bence Csokas wrote: > > > > > 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> > > > > > --- > > > > > > > > I'm really having trouble seeing how this improves anything > > > > significantly. Probably others have the same impression which is why > > > > this doesn't get a lot of review. > > > > > > The goal of the __free() API is to reduce the number of goto-based error > > > paths. Normally, you'd have to keep the state of every local in your head > > > for every early return. (Has it been initialized? Has it been consumed? Or > > > does it need to be freed?) Then you need to arrange your "goto tree" (error > > > labels) in a way that frees everything, and then for every early exit, jump > > > to the correct one. This can be quite complex for some functions. Look at > > > KVM or HyperV eventfd code, for example. > > > > > > By contrast, you just mark the local var with _free() and an appropriate > > > destructor snippet, and the consumption point (the point after which we > > > don't need to free it on a return) with no_free_ptr(), and the compiler > > > instruments every early return accordingly. > > > > Sure, but there's also the other side about which you don't speak: unless > > the destructor is trivial (like kfree()), there are implications about > > locking, when resources (such as references held by the object) get > > released etc. So it can also be more complex to wrap your head around > > __free usage than around standard cleanup code. > > Sure. Thankfully eventfd_ctx_put() is one of the pretty trivial ones, > basically just calling kfree() on itself when the kref drops to 0. Ok, fair. > > So unless __free is providing a noticeable simplification of some goto > > hell, cleanup in many functions or similar, I generally don't find it worth > > it... > > Look into for instance mshv_irqfd_assign() and weep... And then imagine how > much simpler it'd be if it were using __free() instead. Well, over the years I've seen much worse things :) But I agree for that function using __free would be a simplification. > For the example in 2/2, I deliberately chose one callsite where it's trivial > to deduce that behaviour is unchanged. Well, the change is kind of "meh, I don't care much" for me. So I guess if you include some conversion that actually makes code simpler, I guess Christian might be open to merging this. Honza -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-11-06 15:02 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-10-27 16:22 [PATCH v3 0/2] Add scope-based cleanup capability to eventfd_ctx Bence Csokas 2026-10-27 16:22 ` [PATCH v3 1/2] eventfd: Add scope-based cleanup capability Bence Csokas 2026-10-27 16:22 ` [PATCH v3 2/2] drm/syncobj: Use scope-based cleanup for eventfd_ctx Bence Csokas 2026-11-06 9:05 ` [PATCH v3 0/2] Add scope-based cleanup capability to eventfd_ctx Christian Brauner 2026-11-06 9:31 ` Bence Csókás 2026-11-06 14:13 ` Jan Kara 2026-11-06 14:30 ` Bence Csókás 2026-11-06 15:02 ` Jan Kara
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®