Re: [PATCH liburing 3/5] Add helpers to set and get eventfd notification status

[Jens Axboe <axboe@kernel.dk>]

On 5/20/20 7:12 AM, Stefano Garzarella wrote:
>>> The bigger question is probably how to handle kernels that don't
>>> have this feature. It'll succeed, but we'll still post events. Maybe
>>> the kernel side should have a feature flag that we can test?
>>
>> I thought about that, and initially I added a
>> IORING_FEAT_EVENTFD_DISABLE, but then I realized that we are adding
>> the CQ 'flags' field together with the eventfd disabling feature.
>>
>> So I supposed that if 'p->cq_off.flags' is not zero, than the kernel
>> supports CQ flags and also the IORING_CQ_EVENTFD_DISABLED bit.
>>
>> Do you think that's okay, or should we add IORING_FEAT_EVENTFD_DISABLE
>> (or something similar)?
> 
> Hi Jens,
> I'm changing io_uring_cq_eventfd_enable() to io_uring_cq_eventfd_toggle().

Sounds good.

> Any advice on the error and eventual feature flag?

I guess we can use cq_off.flags != 0 to tell if we have this feature or
not, even though it's a bit quirky. But at the same time, probably not
worth adding a specific feature flag for.

For the error, -EOPNOTSUPP seems fine if we don't have the feature. Just
don't flag errors for enabling when already enabled, or vice versa. It's
inherently racy in that completions can come in while the app is calling
the helper, so we should make the interface relaxed.

-- 
Jens Axboe