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

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

[Oleg Nesterov]

Let me repeat, I do not think this patch can improve performance. In fact
I only hope that none of (micro)benchmarks will suffer, they are often
very sensitive to any changes in pipe.c

And yes, even if this patch is correct (I hope) it can expose the latent
bugs that were hidden by the extra wakeup, like it happened in the past.

But at least the comments should be updated: io_uring depends on poll_usage
"nasty semantics" too and this is not obvious at all. And IMO, the EPOLLET
check added by this patch acts as a documentation too.

And if this patch does cause a regression... I think we need to learn who
else depends on the extra wakeup and how; this is something we should know
anyway.

Changes since v2: renamed ->poll_usage to ->poll_et, and updated comments.

See the tests in 1/1, both pass. And both fail if I remove
WRITE_ONCE(pipe->poll_usage) in pipe_poll().

Oleg.
---

 fs/pipe.c                 | 15 ++++++++-------
 include/linux/pipe_fs_i.h |  4 ++--
 2 files changed, 10 insertions(+), 9 deletions(-)

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

[Oleg Nesterov]

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.

Rename ->poll_usage to ->poll_et to make the purpose clearer, update the
comments to explain that io_uring depends on the "nasty semantics" too,
and change pipe_poll() to set ->poll_et only if wait->_key & EPOLLET is
true; this check should catch both users.

Also, 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                 | 15 ++++++++-------
 include/linux/pipe_fs_i.h |  4 ++--
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/fs/pipe.c b/fs/pipe.c
index 429b0714ec57..e009772d860b 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -686,10 +686,10 @@ anon_pipe_write(struct kiocb *iocb, struct iov_iter *from)
 	 * how (for example) the GNU make jobserver uses small writes to
 	 * wake up pending jobs
 	 *
-	 * Epoll nonsensically wants a wakeup whether the pipe
-	 * was already empty or not.
+	 * If poll_et is set, edge-triggered consumers need a wakeup
+	 * on every write regardless of was_empty.
 	 */
-	if (was_empty || pipe->poll_usage)
+	if (was_empty || READ_ONCE(pipe->poll_et))
 		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)
 {
@@ -760,9 +759,11 @@ pipe_poll(struct file *filp, poll_table *wait)
 	struct pipe_inode_info *pipe = filp->private_data;
 	union pipe_index idx;
 
-	/* Epoll has some historical nasty semantics, this enables them */
-	if (unlikely(!READ_ONCE(pipe->poll_usage)))
-		WRITE_ONCE(pipe->poll_usage, true);
+	/* Enable edge-triggered (epoll, io_uring) per-write wakeups */
+	if ((filp->f_mode & FMODE_READ) &&
+	    wait && (wait->_key & EPOLLET) &&
+	    unlikely(!READ_ONCE(pipe->poll_et)))
+		WRITE_ONCE(pipe->poll_et, true);
 
 	/*
 	 * Reading pipe state only -- no need for acquiring the semaphore.
diff --git a/include/linux/pipe_fs_i.h b/include/linux/pipe_fs_i.h
index 7f6a92ac9704..322661d2a0c8 100644
--- a/include/linux/pipe_fs_i.h
+++ b/include/linux/pipe_fs_i.h
@@ -74,7 +74,7 @@ union pipe_index {
  *	@files: number of struct file referring this pipe (protected by ->i_lock)
  *	@r_counter: reader counter
  *	@w_counter: writer counter
- *	@poll_usage: is this pipe used for epoll, which has crazy wakeups?
+ *	@poll_et: has an edge-triggered (epoll, io_uring) consumer
  *	@fasync_readers: reader side fasync
  *	@fasync_writers: writer side fasync
  *	@bufs: the circular array of pipe buffers
@@ -95,7 +95,7 @@ struct pipe_inode_info {
 	unsigned int files;
 	unsigned int r_counter;
 	unsigned int w_counter;
-	bool poll_usage;
+	bool poll_et;
 #ifdef CONFIG_WATCH_QUEUE
 	bool note_loss;
 #endif
-- 
2.52.0

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

[Breno Leitao]

On Thu, Jul 30, 2026 at 04:12:22PM +0200, 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.
> 
> Rename ->poll_usage to ->poll_et to make the purpose clearer, update the
> comments to explain that io_uring depends on the "nasty semantics" too,
> and change pipe_poll() to set ->poll_et only if wait->_key & EPOLLET is
> true; this check should catch both users.

This look great at first sight, thanks!

> @@ -760,9 +759,11 @@ pipe_poll(struct file *filp, poll_table *wait)
>  	struct pipe_inode_info *pipe = filp->private_data;
>  	union pipe_index idx;
>  
> -	/* Epoll has some historical nasty semantics, this enables them */
> -	if (unlikely(!READ_ONCE(pipe->poll_usage)))
> -		WRITE_ONCE(pipe->poll_usage, true);
> +	/* Enable edge-triggered (epoll, io_uring) per-write wakeups */
> +	if ((filp->f_mode & FMODE_READ) &&
> +	    wait && (wait->_key & EPOLLET) &&
> +	    unlikely(!READ_ONCE(pipe->poll_et)))
> +		WRITE_ONCE(pipe->poll_et, true);

Can I ask you to factor this out set code, and comment this nasty
semantics in the function and why we need to do it?

Tha would help to make this corner case more visible for readers, other
than hidden on commit messages.

Thanks,
--breno

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

[Oleg Nesterov]

Hi Breno,

thanks for taking a look!

On 07/30, Breno Leitao wrote:
>
> On Thu, Jul 30, 2026 at 04:12:22PM +0200, Oleg Nesterov wrote:
> > -	/* Epoll has some historical nasty semantics, this enables them */
> > -	if (unlikely(!READ_ONCE(pipe->poll_usage)))
> > -		WRITE_ONCE(pipe->poll_usage, true);
> > +	/* Enable edge-triggered (epoll, io_uring) per-write wakeups */
> > +	if ((filp->f_mode & FMODE_READ) &&
> > +	    wait && (wait->_key & EPOLLET) &&
> > +	    unlikely(!READ_ONCE(pipe->poll_et)))
> > +		WRITE_ONCE(pipe->poll_et, true);
>
> Can I ask you to factor this out set code, and comment this nasty
> semantics in the function and why we need to do it?

Dou you mean a new helper?

You can't imagine how much time I spent trying to make the comments more
clear but keep them concise ;) More than writing the test for io_uring.

What exactly do you think the comment should say? I agree with anything in
advance. I thought that "edge-triggered" provides enough info, but I would
be happy to improve the docs.

And the helper's name? pipe_enable_poll_et() ?

Thanks,

Oleg.

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

[Linus Torvalds]

On Thu, 30 Jul 2026 at 07:12, Oleg Nesterov <oleg@redhat.com> wrote:
>
> Let me repeat, I do not think this patch can improve performance. In fact
> I only hope that none of (micro)benchmarks will suffer, they are often
> very sensitive to any changes in pipe.c

I like this patch mostly for the renaming, not because I think it
matters. I think "poll usage" was a mistake in naming and doesn't
explain the issue. That said, I'd go even further, and make it clear
that it's not about "poll" itself - which is fine, it's about "epoll",
which has that broken crazy bug where it calls something "edge
triggered" but then actually wants effectively level-triggered
behavior - wakeups when nothing actually changed, which is the
*opposite* of an edge.

Pure garbage.

So the real name should be something like "epoll_pseudo_edgetrigger".
Because "et" isn't really helpful either.

             Linus