Re: [RFC PATCH v2 1/1] pipe: only enable the extra wake_up(rd_wait) for edge-triggered consumers

Hey Oleg,

On 7/29/26 16:47, Oleg Nesterov wrote:
> Pavel, thanks for taking the look!
> 
> But let me ask a couple of questions to ensure I really understand you.
> 
> On 07/29, Pavel Begunkov wrote:
>>
>> On 7/27/26 13:28, Oleg Nesterov wrote:
>>> pipe_poll() unconditionally sets poll_usage on the first call, forcing
>>> anon_pipe_write() to wake up readers on every write even if the pipe was
>>> not empty. But this is only needed for edge-triggered consumers: epoll
>>> with EPOLLET and io_uring without (unsupported) IORING_POLL_ADD_LEVEL.
>>> poll() and select() users pay for it for no reason.
>>
>> Sounds good, especially with prep patches you mentioned.
> 
> By prep patches you mean the
> 
> 	With or without this patch we need to update the comments to document
> 	that io_uring depends on ->pipe_usage too. And probably rename it to
> 	(say) ->et_poll.
> 
> note in "v2 0/1" ?

Yep

> If yes, I'll send this change as "v3 1/2", rediff this patch on top of it,
> and make it "v3 2/2".

I should've been clearer, no preference whether it's split into
2 patches or not.

>> The only note your problems are caused by IORING_OP_POLL_ADD, which
>> is not that important comparing to other polled io_uring requests,
>> and they also set EPOLLET while should be fine with level. Not
>> asking to change anything, io_uring should just stop setting EPOLLET
>> for them. And IIUC poll callback implementations don't care about
>> EPOLLET, at least before this patch.
> 
> Sorry, I am a bit confused, could you add more details?

TLDR, io_uring has something to improve internally after this patch
lands.

> In particular, I don't understand the "IUC poll callback implementations
> don't care about EPOLLET, at least before this patch" part.

I was saying that from a quick look I don't see any struct
file_operations::poll implementation checking EPOLLET. And if so,
it makes changing io_uring easier, only need to consider this
patch.

> Although it seems you agree that this patch should not break (change the
> current behaviour of) io_uring, 

Yes

and right now this is my only concern.
> Can you ack/nack my understanding?

You got it all right

-- 
Pavel Begunkov