Re: [PATCH] io_uring/poll: use IOU_F_TWQ_LAZY_WAKE for wakeups

[Jens Axboe <axboe@kernel.dk>]

On 11/7/23 10:47 AM, Pavel Begunkov wrote:
> On 10/19/23 14:01, Jens Axboe wrote:
>> With poll triggered retries, each event trigger will cause a task_work
>> item to be added for processing. If the ring is setup with
>> IORING_SETUP_DEFER_TASKRUN and a task is waiting on multiple events to
>> complete, any task_work addition will wake the task for processing these
>> items. This can cause more context switches than we would like, if the
>> application is deliberately waiting on multiple items to increase
>> efficiency.
> 
> I'm a bit late here. The reason why I didn't enable it for polling is
> because it changes the behaviour. Let's think of a situation where we
> want to accept 2 sockets, so we send a multishot accept and do
> cq_wait(nr=2). It was perfectly fine before, but now it'll hung as
> there's only 1 request and so 1 tw queued. And same would happen with
> multishot recv even though it's more relevant to packet based protocols
> like UDP.
> 
> It might be not specific to multishots:
> listen(backlog=1), queue N oneshot accepts and cq_wait(N).

I don't think it's unreasonable to assume that you need a backlog of N
if you batch wait for N to come in, with defer taskrun. I'm more curious
about non-accept cases that would potentially break.

> Now we get the first connection in the queue to accept.
> 
>     [IORING_OP_ACCEPT] = {
>         .poll_exclusive        = 1,
>     }
> 
> Due to poll_exclusive (I assume) it wakes only one accept. That

Right, if poll_exclusive is set, then we use exclusive waits. IFF the
caller also groks that and asks to wake just 1, then we will indeed just
wake one. I can certainly see this one being a problem, but at least
that could be handled by not doing this for exclusive poll waits.

> will try to queue up a tw for it, but it'll not be executed
> because it's just one item. No other connection can be queued
> up because of the backlog limit => presumably no other request
> will be woken up => that first tw never runs. It's more subtle
> and timing specific than the previous example, but nevertheless
> it's concerning we might step on sth like that.

Backlog aside, as per above, what other cases would we need to worry
about here? It's really anything where something in poll would need
processing to trigger more events.
 
IOW, if we can allow some known cases at least that'd certainly
help, or conversely exclude ones (like poll_exclusive).

-- 
Jens Axboe