Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 0/2] io_uring: add a CQ ring flag to enable/disable
 eventfd notification
To:     Stefano Garzarella <sgarzare@redhat.com>
Cc:     io-uring@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-fsdevel@vger.kernel.org,
        Alexander Viro <viro@zeniv.linux.org.uk>
References: <20200515105414.68683-1-sgarzare@redhat.com>
From:   Jens Axboe <axboe@kernel.dk>
Message-ID: <eaab5cc7-0297-a8f8-f7a9-e00bcf12b678@kernel.dk>
Date:   Fri, 15 May 2020 08:24:58 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.7.0
MIME-Version: 1.0
In-Reply-To: <20200515105414.68683-1-sgarzare@redhat.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 5/15/20 4:54 AM, Stefano Garzarella wrote:
> The first patch adds the new 'cq_flags' field for the CQ ring. It
> should be written by the application and read by the kernel.
> 
> The second patch adds a new IORING_CQ_NEED_WAKEUP flag that can be
> used by the application to enable/disable eventfd notifications.
> 
> I'm not sure the name is the best one, an alternative could be
> IORING_CQ_NEED_EVENT.
> 
> This feature can be useful if the application are using eventfd to be
> notified when requests are completed, but they don't want a notification
> for every request.
> Of course the application can already remove the eventfd from the event
> loop, but as soon as it adds the eventfd again, it will be notified,
> even if it has already handled all the completed requests.
> 
> The most important use case is when the registered eventfd is used to
> notify a KVM guest through irqfd and we want a mechanism to
> enable/disable interrupts.
> 
> I also extended liburing API and added a test case here:
> https://github.com/stefano-garzarella/liburing/tree/eventfd-disable

Don't mind the feature, and I think the patches look fine. But the name
is really horrible, I'd have no idea what that flag does without looking
at the code or a man page. Why not call it IORING_CQ_EVENTFD_ENABLED or
something like that? Or maybe IORING_CQ_EVENTFD_DISABLED, and then you
don't have to muck with the default value either. The app would set the
flag to disable eventfd, temporarily, and clear it again when it wants
notifications again.

-- 
Jens Axboe