All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH v 0/1] pipe: only enable the extra wake_up(rd_wait) for edge-triggered consumers
@ 2026-07-27 12:23 Oleg Nesterov
  2026-07-27 12:24 ` [RFC PATCH v2 " Oleg Nesterov
  2026-07-27 12:28 ` [RFC PATCH v2 1/1] " Oleg Nesterov
  0 siblings, 2 replies; 8+ messages in thread
From: Oleg Nesterov @ 2026-07-27 12:23 UTC (permalink / raw)
  To: Breno Leitao, Christian Brauner, Mateusz Guzik, Jens Axboe
  Cc: Alexander Viro, Jan Kara, Alexey Gladkov, linux-kernel,
	linux-fsdevel, io-uring

Again, I am not sure this makes a lot of sense. But I'd like to know
your opinion.

Plus I'd like to have the review from sashiko ;)

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.

tools/testing/selftests/filesystems/epoll/epoll_wakeup_test.c passes.

See also the test-cases in 1/1, they pass with this patch. And fail if
I remove WRITE_ONCE(pipe->poll_usage, true) in pipe_poll().

Oleg.
---

 fs/pipe.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [RFC PATCH v2 0/1] pipe: only enable the extra wake_up(rd_wait) for edge-triggered consumers
  2026-07-27 12:23 [RFC PATCH v 0/1] pipe: only enable the extra wake_up(rd_wait) for edge-triggered consumers Oleg Nesterov
@ 2026-07-27 12:24 ` Oleg Nesterov
  2026-07-27 12:28   ` Oleg Nesterov
  2026-07-27 12:28 ` [RFC PATCH v2 1/1] " Oleg Nesterov
  1 sibling, 1 reply; 8+ messages in thread
From: Oleg Nesterov @ 2026-07-27 12:24 UTC (permalink / raw)
  To: Breno Leitao, Christian Brauner, Mateusz Guzik, Jens Axboe
  Cc: Alexander Viro, Jan Kara, Alexey Gladkov, linux-kernel,
	linux-fsdevel, io-uring

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.

Change pipe_poll() to set ->pipe_usage only if wait->_key & EPOLLET is
true, this check should catch both users.

While at it, add READ_ONCE() in anon_pipe_write() to pair with WRITE_ONCE()
in pipe_poll().

Test-case for epoll:

	#include <unistd.h>
	#include <sys/epoll.h>
	#include <assert.h>

	int main(void)
	{
		int pfd[2], efd;
		struct epoll_event evt = { .events = EPOLLIN | EPOLLET };

		pipe(pfd);
		efd = epoll_create1(0);
		epoll_ctl(efd, EPOLL_CTL_ADD, pfd[0], &evt);

		for (int i = 0; i < 2; ++i) {
			write(pfd[1], "", 1);
			assert(epoll_wait(efd, &evt, 1, 0) == 1);
		}

		return 0;
	}

Test-case for io_uring:

	#include <unistd.h>
	#include <sys/mman.h>
	#include <sys/epoll.h>
	#include <sys/syscall.h>
	#include <linux/io_uring.h>
	#include <assert.h>

	int main(void)
	{
		struct io_uring_params p = {};
		int fd, pfd[2];

		pipe(pfd);

		fd = syscall(SYS_io_uring_setup, 2, &p);
		assert(fd >= 0);

		void *ring = mmap(0, p.cq_off.cqes + p.cq_entries * sizeof(struct io_uring_cqe),
				  PROT_READ | PROT_WRITE, MAP_SHARED, fd, IORING_OFF_SQ_RING);
		assert(ring != MAP_FAILED);
		*(unsigned *)(ring + p.sq_off.tail) = 1;

		struct io_uring_sqe *sqes = mmap(0, p.sq_entries * sizeof(*sqes),
				  PROT_READ | PROT_WRITE, MAP_SHARED, fd, IORING_OFF_SQES);
		assert(sqes != MAP_FAILED);
		sqes[0].opcode = IORING_OP_POLL_ADD;
		sqes[0].fd = pfd[0];
		sqes[0].len = IORING_POLL_ADD_MULTI;
		sqes[0].poll32_events = EPOLLIN;

		syscall(SYS_io_uring_enter, fd, 1, 0, 0, 0, 0);

		unsigned *cq_head = ring + p.cq_off.head;
		unsigned *cq_tail = ring + p.cq_off.tail;
		for (int i = 0; i < 2; ++i) {
			write(pfd[1], "", 1);
			syscall(SYS_io_uring_enter, fd, 0, 0, IORING_ENTER_GETEVENTS, 0, 0);
			assert(*cq_tail == ++*cq_head);
		}

		return 0;
	}

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 fs/pipe.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/pipe.c b/fs/pipe.c
index 429b0714ec57..98b1e2385103 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -689,7 +689,7 @@ anon_pipe_write(struct kiocb *iocb, struct iov_iter *from)
 	 * Epoll nonsensically wants a wakeup whether the pipe
 	 * was already empty or not.
 	 */
-	if (was_empty || pipe->poll_usage)
+	if (was_empty || READ_ONCE(pipe->poll_usage))
 		wake_up_interruptible_sync_poll(&pipe->rd_wait, EPOLLIN | EPOLLRDNORM);
 	kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
 	if (wake_next_writer)
@@ -752,7 +752,6 @@ static long pipe_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	}
 }
 
-/* No kernel lock held - fine */
 static __poll_t
 pipe_poll(struct file *filp, poll_table *wait)
 {
@@ -761,7 +760,9 @@ pipe_poll(struct file *filp, poll_table *wait)
 	union pipe_index idx;
 
 	/* Epoll has some historical nasty semantics, this enables them */
-	if (unlikely(!READ_ONCE(pipe->poll_usage)))
+	if ((filp->f_mode & FMODE_READ) &&
+	    wait && (wait->_key & EPOLLET) &&
+	    unlikely(!READ_ONCE(pipe->poll_usage)))
 		WRITE_ONCE(pipe->poll_usage, true);
 
 	/*
-- 
2.52.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH v2 0/1] pipe: only enable the extra wake_up(rd_wait) for edge-triggered consumers
  2026-07-27 12:24 ` [RFC PATCH v2 " Oleg Nesterov
@ 2026-07-27 12:28   ` Oleg Nesterov
  2026-07-28  7:41     ` Christian Brauner
  0 siblings, 1 reply; 8+ messages in thread
From: Oleg Nesterov @ 2026-07-27 12:28 UTC (permalink / raw)
  To: Breno Leitao, Christian Brauner, Mateusz Guzik, Jens Axboe
  Cc: Alexander Viro, Jan Kara, Alexey Gladkov, linux-kernel,
	linux-fsdevel, io-uring

Damn, sorry, the subject is wrong... ignore, will resend

On 07/27, 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.
> 
> Change pipe_poll() to set ->pipe_usage only if wait->_key & EPOLLET is
> true, this check should catch both users.
> 
> While at it, add READ_ONCE() in anon_pipe_write() to pair with WRITE_ONCE()
> in pipe_poll().
> 
> Test-case for epoll:
> 
> 	#include <unistd.h>
> 	#include <sys/epoll.h>
> 	#include <assert.h>
> 
> 	int main(void)
> 	{
> 		int pfd[2], efd;
> 		struct epoll_event evt = { .events = EPOLLIN | EPOLLET };
> 
> 		pipe(pfd);
> 		efd = epoll_create1(0);
> 		epoll_ctl(efd, EPOLL_CTL_ADD, pfd[0], &evt);
> 
> 		for (int i = 0; i < 2; ++i) {
> 			write(pfd[1], "", 1);
> 			assert(epoll_wait(efd, &evt, 1, 0) == 1);
> 		}
> 
> 		return 0;
> 	}
> 
> Test-case for io_uring:
> 
> 	#include <unistd.h>
> 	#include <sys/mman.h>
> 	#include <sys/epoll.h>
> 	#include <sys/syscall.h>
> 	#include <linux/io_uring.h>
> 	#include <assert.h>
> 
> 	int main(void)
> 	{
> 		struct io_uring_params p = {};
> 		int fd, pfd[2];
> 
> 		pipe(pfd);
> 
> 		fd = syscall(SYS_io_uring_setup, 2, &p);
> 		assert(fd >= 0);
> 
> 		void *ring = mmap(0, p.cq_off.cqes + p.cq_entries * sizeof(struct io_uring_cqe),
> 				  PROT_READ | PROT_WRITE, MAP_SHARED, fd, IORING_OFF_SQ_RING);
> 		assert(ring != MAP_FAILED);
> 		*(unsigned *)(ring + p.sq_off.tail) = 1;
> 
> 		struct io_uring_sqe *sqes = mmap(0, p.sq_entries * sizeof(*sqes),
> 				  PROT_READ | PROT_WRITE, MAP_SHARED, fd, IORING_OFF_SQES);
> 		assert(sqes != MAP_FAILED);
> 		sqes[0].opcode = IORING_OP_POLL_ADD;
> 		sqes[0].fd = pfd[0];
> 		sqes[0].len = IORING_POLL_ADD_MULTI;
> 		sqes[0].poll32_events = EPOLLIN;
> 
> 		syscall(SYS_io_uring_enter, fd, 1, 0, 0, 0, 0);
> 
> 		unsigned *cq_head = ring + p.cq_off.head;
> 		unsigned *cq_tail = ring + p.cq_off.tail;
> 		for (int i = 0; i < 2; ++i) {
> 			write(pfd[1], "", 1);
> 			syscall(SYS_io_uring_enter, fd, 0, 0, IORING_ENTER_GETEVENTS, 0, 0);
> 			assert(*cq_tail == ++*cq_head);
> 		}
> 
> 		return 0;
> 	}
> 
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> ---
>  fs/pipe.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/pipe.c b/fs/pipe.c
> index 429b0714ec57..98b1e2385103 100644
> --- a/fs/pipe.c
> +++ b/fs/pipe.c
> @@ -689,7 +689,7 @@ anon_pipe_write(struct kiocb *iocb, struct iov_iter *from)
>  	 * Epoll nonsensically wants a wakeup whether the pipe
>  	 * was already empty or not.
>  	 */
> -	if (was_empty || pipe->poll_usage)
> +	if (was_empty || READ_ONCE(pipe->poll_usage))
>  		wake_up_interruptible_sync_poll(&pipe->rd_wait, EPOLLIN | EPOLLRDNORM);
>  	kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
>  	if (wake_next_writer)
> @@ -752,7 +752,6 @@ static long pipe_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  	}
>  }
>  
> -/* No kernel lock held - fine */
>  static __poll_t
>  pipe_poll(struct file *filp, poll_table *wait)
>  {
> @@ -761,7 +760,9 @@ pipe_poll(struct file *filp, poll_table *wait)
>  	union pipe_index idx;
>  
>  	/* Epoll has some historical nasty semantics, this enables them */
> -	if (unlikely(!READ_ONCE(pipe->poll_usage)))
> +	if ((filp->f_mode & FMODE_READ) &&
> +	    wait && (wait->_key & EPOLLET) &&
> +	    unlikely(!READ_ONCE(pipe->poll_usage)))
>  		WRITE_ONCE(pipe->poll_usage, true);
>  
>  	/*
> -- 
> 2.52.0
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [RFC PATCH v2 1/1] pipe: only enable the extra wake_up(rd_wait) for edge-triggered consumers
  2026-07-27 12:23 [RFC PATCH v 0/1] pipe: only enable the extra wake_up(rd_wait) for edge-triggered consumers Oleg Nesterov
  2026-07-27 12:24 ` [RFC PATCH v2 " Oleg Nesterov
@ 2026-07-27 12:28 ` Oleg Nesterov
  2026-07-29 14:58   ` Pavel Begunkov
  1 sibling, 1 reply; 8+ messages in thread
From: Oleg Nesterov @ 2026-07-27 12:28 UTC (permalink / raw)
  To: Breno Leitao, Christian Brauner, Mateusz Guzik, Jens Axboe
  Cc: Alexander Viro, Jan Kara, Alexey Gladkov, linux-kernel,
	linux-fsdevel, io-uring

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.

Change pipe_poll() to set ->pipe_usage only if wait->_key & EPOLLET is
true, this check should catch both users.

While at it, add READ_ONCE() in anon_pipe_write() to pair with WRITE_ONCE()
in pipe_poll().

Test-case for epoll:

	#include <unistd.h>
	#include <sys/epoll.h>
	#include <assert.h>

	int main(void)
	{
		int pfd[2], efd;
		struct epoll_event evt = { .events = EPOLLIN | EPOLLET };

		pipe(pfd);
		efd = epoll_create1(0);
		epoll_ctl(efd, EPOLL_CTL_ADD, pfd[0], &evt);

		for (int i = 0; i < 2; ++i) {
			write(pfd[1], "", 1);
			assert(epoll_wait(efd, &evt, 1, 0) == 1);
		}

		return 0;
	}

Test-case for io_uring:

	#include <unistd.h>
	#include <sys/mman.h>
	#include <sys/epoll.h>
	#include <sys/syscall.h>
	#include <linux/io_uring.h>
	#include <assert.h>

	int main(void)
	{
		struct io_uring_params p = {};
		int fd, pfd[2];

		pipe(pfd);

		fd = syscall(SYS_io_uring_setup, 2, &p);
		assert(fd >= 0);

		void *ring = mmap(0, p.cq_off.cqes + p.cq_entries * sizeof(struct io_uring_cqe),
				  PROT_READ | PROT_WRITE, MAP_SHARED, fd, IORING_OFF_SQ_RING);
		assert(ring != MAP_FAILED);
		*(unsigned *)(ring + p.sq_off.tail) = 1;

		struct io_uring_sqe *sqes = mmap(0, p.sq_entries * sizeof(*sqes),
				  PROT_READ | PROT_WRITE, MAP_SHARED, fd, IORING_OFF_SQES);
		assert(sqes != MAP_FAILED);
		sqes[0].opcode = IORING_OP_POLL_ADD;
		sqes[0].fd = pfd[0];
		sqes[0].len = IORING_POLL_ADD_MULTI;
		sqes[0].poll32_events = EPOLLIN;

		syscall(SYS_io_uring_enter, fd, 1, 0, 0, 0, 0);

		unsigned *cq_head = ring + p.cq_off.head;
		unsigned *cq_tail = ring + p.cq_off.tail;
		for (int i = 0; i < 2; ++i) {
			write(pfd[1], "", 1);
			syscall(SYS_io_uring_enter, fd, 0, 0, IORING_ENTER_GETEVENTS, 0, 0);
			assert(*cq_tail == ++*cq_head);
		}

		return 0;
	}

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 fs/pipe.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/pipe.c b/fs/pipe.c
index 429b0714ec57..98b1e2385103 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -689,7 +689,7 @@ anon_pipe_write(struct kiocb *iocb, struct iov_iter *from)
 	 * Epoll nonsensically wants a wakeup whether the pipe
 	 * was already empty or not.
 	 */
-	if (was_empty || pipe->poll_usage)
+	if (was_empty || READ_ONCE(pipe->poll_usage))
 		wake_up_interruptible_sync_poll(&pipe->rd_wait, EPOLLIN | EPOLLRDNORM);
 	kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
 	if (wake_next_writer)
@@ -752,7 +752,6 @@ static long pipe_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	}
 }
 
-/* No kernel lock held - fine */
 static __poll_t
 pipe_poll(struct file *filp, poll_table *wait)
 {
@@ -761,7 +760,9 @@ pipe_poll(struct file *filp, poll_table *wait)
 	union pipe_index idx;
 
 	/* Epoll has some historical nasty semantics, this enables them */
-	if (unlikely(!READ_ONCE(pipe->poll_usage)))
+	if ((filp->f_mode & FMODE_READ) &&
+	    wait && (wait->_key & EPOLLET) &&
+	    unlikely(!READ_ONCE(pipe->poll_usage)))
 		WRITE_ONCE(pipe->poll_usage, true);
 
 	/*
-- 
2.52.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH v2 0/1] pipe: only enable the extra wake_up(rd_wait) for edge-triggered consumers
  2026-07-27 12:28   ` Oleg Nesterov
@ 2026-07-28  7:41     ` Christian Brauner
  0 siblings, 0 replies; 8+ messages in thread
From: Christian Brauner @ 2026-07-28  7:41 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: Breno Leitao, Christian Brauner, Mateusz Guzik, Jens Axboe,
	Alexander Viro, Jan Kara, Alexey Gladkov, linux-kernel,
	linux-fsdevel, io-uring

On 2026-07-27 14:28 +0200, Oleg Nesterov wrote:
> Damn, sorry, the subject is wrong... ignore, will resend

Fwiw, in case of minor issues such as this feel free to just indicate
that you would like me to fix them up.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH v2 1/1] pipe: only enable the extra wake_up(rd_wait) for edge-triggered consumers
  2026-07-27 12:28 ` [RFC PATCH v2 1/1] " Oleg Nesterov
@ 2026-07-29 14:58   ` Pavel Begunkov
  2026-07-29 15:47     ` Oleg Nesterov
  0 siblings, 1 reply; 8+ messages in thread
From: Pavel Begunkov @ 2026-07-29 14:58 UTC (permalink / raw)
  To: Oleg Nesterov, Breno Leitao, Christian Brauner, Mateusz Guzik,
	Jens Axboe
  Cc: Alexander Viro, Jan Kara, Alexey Gladkov, linux-kernel,
	linux-fsdevel, io-uring

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.

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.


> Change pipe_poll() to set ->pipe_usage only if wait->_key & EPOLLET is
> true, this check should catch both users.
> 
> While at it, add READ_ONCE() in anon_pipe_write() to pair with WRITE_ONCE()
> in pipe_poll().
> 
> Test-case for epoll:
> 
> 	#include <unistd.h>
> 	#include <sys/epoll.h>
> 	#include <assert.h>
> 
> 	int main(void)
> 	{
> 		int pfd[2], efd;
> 		struct epoll_event evt = { .events = EPOLLIN | EPOLLET };
> 
> 		pipe(pfd);
> 		efd = epoll_create1(0);
> 		epoll_ctl(efd, EPOLL_CTL_ADD, pfd[0], &evt);
> 
> 		for (int i = 0; i < 2; ++i) {
> 			write(pfd[1], "", 1);
> 			assert(epoll_wait(efd, &evt, 1, 0) == 1);
> 		}
> 
> 		return 0;
> 	}
> 
> Test-case for io_uring:
> 
> 	#include <unistd.h>
> 	#include <sys/mman.h>
> 	#include <sys/epoll.h>
> 	#include <sys/syscall.h>
> 	#include <linux/io_uring.h>
> 	#include <assert.h>
> 
> 	int main(void)
> 	{
> 		struct io_uring_params p = {};
> 		int fd, pfd[2];
> 
> 		pipe(pfd);
> 
> 		fd = syscall(SYS_io_uring_setup, 2, &p);
> 		assert(fd >= 0);
> 
> 		void *ring = mmap(0, p.cq_off.cqes + p.cq_entries * sizeof(struct io_uring_cqe),
> 				  PROT_READ | PROT_WRITE, MAP_SHARED, fd, IORING_OFF_SQ_RING);
> 		assert(ring != MAP_FAILED);
> 		*(unsigned *)(ring + p.sq_off.tail) = 1;
> 
> 		struct io_uring_sqe *sqes = mmap(0, p.sq_entries * sizeof(*sqes),
> 				  PROT_READ | PROT_WRITE, MAP_SHARED, fd, IORING_OFF_SQES);
> 		assert(sqes != MAP_FAILED);
> 		sqes[0].opcode = IORING_OP_POLL_ADD;
> 		sqes[0].fd = pfd[0];
> 		sqes[0].len = IORING_POLL_ADD_MULTI;
> 		sqes[0].poll32_events = EPOLLIN;
> 
> 		syscall(SYS_io_uring_enter, fd, 1, 0, 0, 0, 0);
> 
> 		unsigned *cq_head = ring + p.cq_off.head;
> 		unsigned *cq_tail = ring + p.cq_off.tail;
> 		for (int i = 0; i < 2; ++i) {
> 			write(pfd[1], "", 1);
> 			syscall(SYS_io_uring_enter, fd, 0, 0, IORING_ENTER_GETEVENTS, 0, 0);
> 			assert(*cq_tail == ++*cq_head);
> 		}
> 
> 		return 0;
> 	}
> 
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> ---
>   fs/pipe.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/pipe.c b/fs/pipe.c
> index 429b0714ec57..98b1e2385103 100644
> --- a/fs/pipe.c
> +++ b/fs/pipe.c
> @@ -689,7 +689,7 @@ anon_pipe_write(struct kiocb *iocb, struct iov_iter *from)
>   	 * Epoll nonsensically wants a wakeup whether the pipe
>   	 * was already empty or not.
>   	 */
> -	if (was_empty || pipe->poll_usage)
> +	if (was_empty || READ_ONCE(pipe->poll_usage))
>   		wake_up_interruptible_sync_poll(&pipe->rd_wait, EPOLLIN | EPOLLRDNORM);
>   	kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
>   	if (wake_next_writer)
> @@ -752,7 +752,6 @@ static long pipe_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>   	}
>   }
>   
> -/* No kernel lock held - fine */
>   static __poll_t
>   pipe_poll(struct file *filp, poll_table *wait)
>   {
> @@ -761,7 +760,9 @@ pipe_poll(struct file *filp, poll_table *wait)
>   	union pipe_index idx;
>   
>   	/* Epoll has some historical nasty semantics, this enables them */
> -	if (unlikely(!READ_ONCE(pipe->poll_usage)))
> +	if ((filp->f_mode & FMODE_READ) &&
> +	    wait && (wait->_key & EPOLLET) &&
> +	    unlikely(!READ_ONCE(pipe->poll_usage)))
>   		WRITE_ONCE(pipe->poll_usage, true);
>   
>   	/*

-- 
Pavel Begunkov


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH v2 1/1] pipe: only enable the extra wake_up(rd_wait) for edge-triggered consumers
  2026-07-29 14:58   ` Pavel Begunkov
@ 2026-07-29 15:47     ` Oleg Nesterov
  2026-07-29 17:07       ` Pavel Begunkov
  0 siblings, 1 reply; 8+ messages in thread
From: Oleg Nesterov @ 2026-07-29 15:47 UTC (permalink / raw)
  To: Pavel Begunkov
  Cc: Breno Leitao, Christian Brauner, Mateusz Guzik, Jens Axboe,
	Alexander Viro, Jan Kara, Alexey Gladkov, linux-kernel,
	linux-fsdevel, io-uring

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" ?

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

> 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?

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

Although it seems you agree that this patch should not break (change the
current behaviour of) io_uring, and right now this is my only concern.

Can you ack/nack my understanding?

Thanks!

Oleg.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH v2 1/1] pipe: only enable the extra wake_up(rd_wait) for edge-triggered consumers
  2026-07-29 15:47     ` Oleg Nesterov
@ 2026-07-29 17:07       ` Pavel Begunkov
  0 siblings, 0 replies; 8+ messages in thread
From: Pavel Begunkov @ 2026-07-29 17:07 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: Breno Leitao, Christian Brauner, Mateusz Guzik, Jens Axboe,
	Alexander Viro, Jan Kara, Alexey Gladkov, linux-kernel,
	linux-fsdevel, io-uring

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-07-29 17:07 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-27 12:23 [RFC PATCH v 0/1] pipe: only enable the extra wake_up(rd_wait) for edge-triggered consumers Oleg Nesterov
2026-07-27 12:24 ` [RFC PATCH v2 " Oleg Nesterov
2026-07-27 12:28   ` Oleg Nesterov
2026-07-28  7:41     ` Christian Brauner
2026-07-27 12:28 ` [RFC PATCH v2 1/1] " Oleg Nesterov
2026-07-29 14:58   ` Pavel Begunkov
2026-07-29 15:47     ` Oleg Nesterov
2026-07-29 17:07       ` Pavel Begunkov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.