Re: [PATCH 0/1] pipe: only enable the extra wake_up(rd_wait) when epoll is actually used

On 7/25/26 16:19, Oleg Nesterov wrote:
> On 07/24, Oleg Nesterov wrote:
>>
>> On 07/23, Oleg Nesterov wrote:
>>>
>>> OK, sashiko has some concerns
>>>
>>> 	https://sashiko.dev/#/patchset/amIqmbbZx3NlzUsX%40redhat.com
>>
>> Let me quote:
>>
>> 	Does skipping this wakeup for non-epoll consumers break io_uring?
>>
>> 	Applications polling pipes via io_uring do not attach an eventpoll context,
>> 	so pipe->epoll_usage will be false. If a writer writes to an empty pipe,
>> 	io_uring receives the wakeup.
>>
>> 	If the writer then writes a second chunk before the first is drained,
>> 	anon_pipe_write() observes was_empty == false and
>> 	pipe_get_epoll_usage() == false, skipping the waitqueue wakeup.
>>
>> 	Could this cause io_uring to miss events and hang permanently, waiting
>> 	for a CQE that will never be emitted for the new data?
>>
>> and I am starting to think sashiko is right (damn as always ;) and this
>> patch does affect/break io_uring.
>>
>> Jens, could you confirm? If yes, we need to update the comments in pipe.c
>> (I've attached 1/1 at the end, so that you can see what this patch does)
> ...
>> It seems that IORING_OP_POLL_ADD / IORING_POLL_ADD_MULTI is edge-triggered
>> by default! Like EPOLL_CTL_ADD / EPOLLET.
> 
> Yes, sashiko is right. With some help from AI I wrote the simple test-case

https://github.com/axboe/liburing/blob/master/man/io_uring_enter.2#L400

Seems like it's even documented, which is a shame because I deem
IORING_OP_POLL_ADD + IORING_POLL_ADD_MULTI to be a half broken API that
should be deprecated after investigating lxc problems with it some years
back. Makes me wonder, likely it wouldn't even break anything if shifts
from strict edge definition for pipes. It's a stream, and the user doesn't
generally know the number of bytes sitting there.

-- 
Pavel Begunkov