[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 1/1] pipe: only enable the extra wake_up(rd_wait) for edge-triggered consumers

[Mateusz Guzik]

On Thu, Jul 30, 2026 at 4:38 PM Oleg Nesterov <oleg@redhat.com> wrote:
> 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.
>

how about: There is userspace depending on the extra wake up, see
commit 3a34b13a88caeb28 ("pipe: make pipe writes always wake up
readers") for details.

or whatever else which refers to the commit, no need for anything
fancy. the current commentary is definitely lame.

> And the helper's name? pipe_enable_poll_et() ?
>

perhaps pipe_enable_epoll_semantics()?

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

[Oleg Nesterov]

Mateusz, Breno,

thanks, I'll try to think about it later, but...

On 07/30, Mateusz Guzik wrote:
>
> On Thu, Jul 30, 2026 at 4:38 PM Oleg Nesterov <oleg@redhat.com> wrote:
> > 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.
> >
>
> how about: There is userspace depending on the extra wake up, see
> commit 3a34b13a88caeb28 ("pipe: make pipe writes always wake up
> readers") for details.

To me this looks confusing.

IMO, the comment like this (with the reference to the commit) would make
sense to document the unconditional/undocumented kill_fasync(fasync_readers)
in anon_pipe_write(), this SIGIO is even worse in some sense and I would like
to discuss it another time ;)

But as for poll_usage/poll_et... We have the established API, and (afaics) it
works as documented. It doesn't matter if EPOLLET behaviour is good or bad.
We only need to document what ->poll_et means for pipes.

> > And the helper's name? pipe_enable_poll_et() ?
>
> perhaps pipe_enable_epoll_semantics()?

Again, contrary to the current comments this is not Epoll-only...

Oleg.

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

[Linus Torvalds]

On Thu, 30 Jul 2026 at 13:35, Oleg Nesterov <oleg@redhat.com> wrote:
>
> Again, contrary to the current comments this is not Epoll-only...

Sure it is. As far as we know, only epoll has ever *cared*.

Yes, you can show the semantics with io_uring, but do you have a user
application that actually cares?

The only reason that ugly hack exists is because we did have that
break user space. If we can get rid of th eugly hack for io_uring,
that would only be a good thing.

So this literally *should* be about only epoll unless you have a
report that io_uring users are equally broken and use that
shit-for-brains notion of edges that aren't edges that nobody sane
should ever use.

               Linus

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

[Oleg Nesterov]

On 07/30, Linus Torvalds wrote:
>
> On Thu, 30 Jul 2026 at 13:35, Oleg Nesterov <oleg@redhat.com> wrote:
> >
> > Again, contrary to the current comments this is not Epoll-only...
>
> Sure it is. As far as we know, only epoll has ever *cared*.
>
> Yes, you can show the semantics with io_uring, but do you have a user
> application that actually cares?

No. But I know nothing about io_uring, this is the question for Pavel
and Jens.

io_uring claims itself edge-triggered (whatever that means). It has
IORING_POLL_ADD_LEVEL, but this mode was disabled by d59bd748db0a9
("io_uring/poll: disable level triggered poll").

I don't understand io_uring/poll.c even remotely, but it seems that
without IORING_POLL_ADD_LEVEL io_uring expects that the io_poll_wake()
callback should be called on every write.

Same for epoll(EPOLLET)... I mean, I have no idea why anyone would
need this behavior. But from the commit 3a34b13a88caeb28 ("pipe: make
pipe writes always wake up readers") we know that such users exist.

> The only reason that ugly hack exists is because we did have that
> break user space. If we can get rid of th eugly hack for io_uring,
> that would only be a good thing.
>
> So this literally *should* be about only epoll unless you have a
> report that io_uring users are equally broken and use that
> shit-for-brains notion of edges that aren't edges that nobody sane
> should ever use.

In the 1st version I did:

	static void pipe_set_epoll_usage(struct file *filp, struct pipe_inode_info *pipe)
	{
	#ifdef CONFIG_EPOLL
		if ((filp->f_mode & FMODE_READ) && filp->f_ep &&
		   unlikely(!READ_ONCE(pipe->epoll_usage)))
		       WRITE_ONCE(pipe->epoll_usage, true);
	#endif
	}

but sashiko didn't like it, 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?

See https://lore.kernel.org/all/amIu5WWgTNkdvqIz@redhat.com/ for details.

So I wrote that "Test-case for io_uring" to a) check that sashiko was right,
and b) to ensure that V2 doesn't change the current behaviour of io_uring.

I won't argue with "nobody sane should ever use", but IMO the same is true
for epoll with EPOLLET.

--------------------------------------------------------------------------
So, let me ask. Apart from the comments and naming, do you agree with this
patch?

I like the new version more, even if we forget about io_uring. Note that
this way epoll_ctl() without EPOLLET in .events will not set ->poll_usage,
and hopefully "nobody sane" use this flag...

What do you think?

Oleg.

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

[Oleg Nesterov]

Sorry for noise, forgot to mention...

> The only reason that ugly hack exists is because we did have that
> break user space. If we can get rid of th eugly hack for io_uring,
> that would only be a good thing.

Perhaps, I can't really comment. But it seems to me that in this case
it would make more sense to change the logic in io_uring/ rather than
in pipe_poll(). And this needs a separate change.

Oleg.

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 10:01:56PM +0200, Mateusz Guzik wrote:
> On Thu, Jul 30, 2026 at 4:38 PM Oleg Nesterov <oleg@redhat.com> wrote:
> > 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.
> >
> 
> how about: There is userspace depending on the extra wake up, see
> commit 3a34b13a88caeb28 ("pipe: make pipe writes always wake up
> readers") for details.
> 
> or whatever else which refers to the commit, no need for anything
> fancy. the current commentary is definitely lame.

Agreed. The rationale was non-obvious without digging into commit
3a34b13a88caeb28 ("pipe: make pipe writes always wake up readers").

Having an explicit reference in the comment would clarify why the
unconditional wakeup exists, rather than leaving it to appear as a
potential weirdness without clear comment.

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

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

[Oleg Nesterov]

On 07/30, Linus Torvalds wrote:
>
> 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.

I am not sure this is a "crazy bug". To me, epoll with EPOLLET works
"as documented". Perhaps I am wrong, this predates the git history.

I'd say the very idea of EPOLLET was wrong, but this doesn't matter:
we have what we have.

And this patch (mostly) tries to document what we have.

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

I'd agree with "pseudo" simply because I can hardly say how could we
define "edgetrigger" in this particular case.

But then we should name it "epoll_or_io_uring_pseudo_edgetrigger".
See the test-cases, they demonstrate the same pattern.

Oleg.

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

[Christian Brauner]

> But then we should name it "epoll_or_io_uring_pseudo_edgetrigger".
> See the test-cases, they demonstrate the same pattern.

Wouldn't pseudo_edgetrigger be sufficient? I think the comment as you
wrote it is ok.

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

[Oleg Nesterov]

On 07/31, Christian Brauner wrote:
>
> > But then we should name it "epoll_or_io_uring_pseudo_edgetrigger".
> > See the test-cases, they demonstrate the same pattern.
>
> Wouldn't pseudo_edgetrigger be sufficient? I think the comment as you
> wrote it is ok.

I agree with any naming ;) even if I personally don't think that "pseudo"
make the purpose any clearer, but this is minor.

It seems that Linus doesn't really like this patch "in general", lets wait
for reply from him...

Oleg.

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

[Linus Torvalds]

On Fri, 31 Jul 2026 at 07:07, Oleg Nesterov <oleg@redhat.com> wrote:
>
> It seems that Linus doesn't really like this patch "in general", lets wait
> for reply from him...

No, I like the patch, but I want the naming to be about the *reason* for it.

The problem it tries to solve is literally that some people think
"edge" means something completely %^@% different from reality. We had
legacy epoll users that used edge-triggered events but wanted
level-triggered semantics.

So it got literally hacked up the minimal way possible.

If we change this to be something that isn't the minimal way possible,
we should do that *right*. we should make it clear that it's a hack
for user space behavior where user space was simply asking for the
wrong thing entirely, and it happened to work because we would send
wakeups willy-nilly for everything, so even level things that didn't
change at all ended up getting those "something changed".

But "user space is doing crazy things" isn't an excuse for the kernel
breaking user space, so thus that "we'll just continue to do our extra
notifications if you use poll".

You are now changing it. And what I disagree is that "change it to be
something else than the minimal thing, but make the naming be bad and
the explanations for the non-minimal thing be bad".

My argument is that IF we change this area, we should damn well do it
right, and document it, and make it very very clear that the *ONLY*
reason this exists is a user space legacy bug that took advantage of
legacy kernel behavior, and that we are papering this over.

               Linus