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

["Bence Csókás" <bence.csokas@arm.com>]

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