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

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

[Oleg Nesterov]

Breno, Christian, Mateusz,

To be honest, I am not sure this change makes a lot of sense.
But somehow the extra wakeup in anon_pipe_write() looks annoying to me.

Can you take a look?

Plus I'd like to know what sashiko.dev thinks.

I have no idea how to test it wrt performance...

Oleg.
---

 fs/pipe.c                 | 23 +++++++++++++++++++----
 include/linux/pipe_fs_i.h |  6 ++++--
 2 files changed, 23 insertions(+), 6 deletions(-)

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

[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 epoll's "nasty semantics"; poll()
and select() users pay for it for no reason.

Rename it to epoll_usage, and only set it when the caller is actually
using epoll on the read side of the pipe.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 fs/pipe.c                 | 23 +++++++++++++++++++----
 include/linux/pipe_fs_i.h |  6 ++++--
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/fs/pipe.c b/fs/pipe.c
index 32140cb00d7e..b45b4b8d0b0b 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -357,6 +357,23 @@ static inline unsigned int pipe_update_tail(struct pipe_inode_info *pipe,
 	return tail;
 }
 
+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
+}
+
+static bool pipe_get_epoll_usage(struct pipe_inode_info *pipe)
+{
+#ifdef CONFIG_EPOLL
+	return pipe->epoll_usage;
+#endif
+	return false;
+}
+
 static ssize_t
 anon_pipe_read(struct kiocb *iocb, struct iov_iter *to)
 {
@@ -689,7 +706,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 || pipe_get_epoll_usage(pipe))
 		wake_up_interruptible_sync_poll(&pipe->rd_wait, EPOLLIN | EPOLLRDNORM);
 	kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
 	if (wake_next_writer)
@@ -761,9 +778,7 @@ 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)))
-		WRITE_ONCE(pipe->poll_usage, true);
-
+	pipe_set_epoll_usage(filp, pipe);
 	/*
 	 * 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..d06da9bad3a8 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?
+ *	@epoll_usage: is this pipe used for epoll, which has crazy wakeups?
  *	@fasync_readers: reader side fasync
  *	@fasync_writers: writer side fasync
  *	@bufs: the circular array of pipe buffers
@@ -95,7 +95,9 @@ struct pipe_inode_info {
 	unsigned int files;
 	unsigned int r_counter;
 	unsigned int w_counter;
-	bool poll_usage;
+#ifdef CONFIG_EPOLL
+	bool epoll_usage;
+#endif
 #ifdef CONFIG_WATCH_QUEUE
 	bool note_loss;
 #endif
-- 
2.52.0

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

[Oleg Nesterov]

OK, sashiko has some concerns

	https://sashiko.dev/#/patchset/amIqmbbZx3NlzUsX%40redhat.com

As for READ_ONCE(), I agree, I just forgot to add them.

But so far I don't understand the note about io_uring...
I'll try to check later.

Anyway, I'd like to know your opinion.

On 07/23, Oleg Nesterov wrote:
>
> Breno, Christian, Mateusz,
>
> To be honest, I am not sure this change makes a lot of sense.
> But somehow the extra wakeup in anon_pipe_write() looks annoying to me.
>
> Can you take a look?
>
> Plus I'd like to know what sashiko.dev thinks.
>
> I have no idea how to test it wrt performance...
>
> Oleg.
> ---
>
>  fs/pipe.c                 | 23 +++++++++++++++++++----
>  include/linux/pipe_fs_i.h |  6 ++++--
>  2 files changed, 23 insertions(+), 6 deletions(-)

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

[Mateusz Guzik]

On Thu, Jul 23, 2026 at 4:52 PM Oleg Nesterov <oleg@redhat.com> 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 epoll's "nasty semantics"; poll()
> and select() users pay for it for no reason.
>
> Rename it to epoll_usage, and only set it when the caller is actually
> using epoll on the read side of the pipe.
>

I am worried in that the poll_usage thing showed up in 2021 in commit
3b844826b6c6affa ("pipe: avoid unnecessary EPOLLET wakeups under
normal loads"), fixing up "missing" wakeups after some rework. Even
then it went ahead and as you noted it also did it for poll/select
users. So by Hyrum's law there is plenty of potential for someone to
depend on it.

However, I think this makes sense to try out and there is history of
committing possibly breaking changes, so I think the idea is fine to
go in.

I does rub me the wrong way that epoll lands in the same handler
though, but that's not something I'm going to do anything about.

The routine pretends to operate in a lockless manner, possibly
returning only a partial result. But it guarantees not blocking when
it should not by queueing up the caller unconditionally, which later
has to be undone. Have you considered patching that up? (as in, do the
work. if there are no events, queue up and do the work again -- should
reduce work if there was stuff already there)

> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> ---
>  fs/pipe.c                 | 23 +++++++++++++++++++----
>  include/linux/pipe_fs_i.h |  6 ++++--
>  2 files changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/fs/pipe.c b/fs/pipe.c
> index 32140cb00d7e..b45b4b8d0b0b 100644
> --- a/fs/pipe.c
> +++ b/fs/pipe.c
> @@ -357,6 +357,23 @@ static inline unsigned int pipe_update_tail(struct pipe_inode_info *pipe,
>         return tail;
>  }
>
> +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
> +}
> +
> +static bool pipe_get_epoll_usage(struct pipe_inode_info *pipe)
> +{
> +#ifdef CONFIG_EPOLL
> +       return pipe->epoll_usage;
> +#endif

If patching this up I think a comment explaining what's going on would be nice.

I think you could steal parts of
3a34b13a88caeb2800ab44a4918f230041b37dd9 for that purpose.

> +       return false;
> +}
> +
>  static ssize_t
>  anon_pipe_read(struct kiocb *iocb, struct iov_iter *to)
>  {
> @@ -689,7 +706,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 || pipe_get_epoll_usage(pipe))
>                 wake_up_interruptible_sync_poll(&pipe->rd_wait, EPOLLIN | EPOLLRDNORM);
>         kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
>         if (wake_next_writer)
> @@ -761,9 +778,7 @@ 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)))
> -               WRITE_ONCE(pipe->poll_usage, true);
> -
> +       pipe_set_epoll_usage(filp, pipe);
>         /*
>          * Reading pipe state only -- no need for acquiring the semaphore.
>          *

nit: I would add a new line between this and the following comment

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

[Oleg Nesterov]

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)

I know nothing about io_uring and io_uring/poll.c is not trivial to say at
least ;) please check my understanding.

It seems that IORING_OP_POLL_ADD / IORING_POLL_ADD_MULTI is edge-triggered
by default! Like EPOLL_CTL_ADD / EPOLLET.

This means that io_uring depends on the "nasty semantics" too, io_poll_wake()
path should add the task work which calls io_req_post_cqe() every time the new
data arrives, even if the pipe was not empty.

Strange...

And. io_poll_parse_events() doesn't set EPOLLET if IORING_POLL_ADD_LEVEL, but
how is it possible to use IORING_POLL_ADD_LEVEL? io_poll_add_prep() only allows
IORING_POLL_ADD_MULTI in flags? OK, I don't understand this code anyway...

Oleg.
---

Subject: [PATCH] pipe: only enable the extra wake_up(rd_wait) when epoll is actually used

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 epoll's "nasty semantics"; poll()
and select() users pay for it for no reason.

Rename it to epoll_usage, and only set it when the caller is actually
using epoll on the read side of the pipe.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 fs/pipe.c                 | 23 +++++++++++++++++++----
 include/linux/pipe_fs_i.h |  6 ++++--
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/fs/pipe.c b/fs/pipe.c
index 32140cb00d7e..b45b4b8d0b0b 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -357,6 +357,23 @@ static inline unsigned int pipe_update_tail(struct pipe_inode_info *pipe,
 	return tail;
 }
 
+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
+}
+
+static bool pipe_get_epoll_usage(struct pipe_inode_info *pipe)
+{
+#ifdef CONFIG_EPOLL
+	return pipe->epoll_usage;
+#endif
+	return false;
+}
+
 static ssize_t
 anon_pipe_read(struct kiocb *iocb, struct iov_iter *to)
 {
@@ -689,7 +706,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 || pipe_get_epoll_usage(pipe))
 		wake_up_interruptible_sync_poll(&pipe->rd_wait, EPOLLIN | EPOLLRDNORM);
 	kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
 	if (wake_next_writer)
@@ -761,9 +778,7 @@ 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)))
-		WRITE_ONCE(pipe->poll_usage, true);
-
+	pipe_set_epoll_usage(filp, pipe);
 	/*
 	 * 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..d06da9bad3a8 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?
+ *	@epoll_usage: is this pipe used for epoll, which has crazy wakeups?
  *	@fasync_readers: reader side fasync
  *	@fasync_writers: writer side fasync
  *	@bufs: the circular array of pipe buffers
@@ -95,7 +95,9 @@ struct pipe_inode_info {
 	unsigned int files;
 	unsigned int r_counter;
 	unsigned int w_counter;
-	bool poll_usage;
+#ifdef CONFIG_EPOLL
+	bool epoll_usage;
+#endif
 #ifdef CONFIG_WATCH_QUEUE
 	bool note_loss;
 #endif
-- 
2.52.0

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

[Mateusz Guzik]

On Fri, Jul 24, 2026 at 3:58 PM Oleg Nesterov <oleg@redhat.com> 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)
>
> I know nothing about io_uring and io_uring/poll.c is not trivial to say at
> least ;) please check my understanding.
>
> It seems that IORING_OP_POLL_ADD / IORING_POLL_ADD_MULTI is edge-triggered
> by default! Like EPOLL_CTL_ADD / EPOLLET.
>
> This means that io_uring depends on the "nasty semantics" too, io_poll_wake()
> path should add the task work which calls io_req_post_cqe() every time the new
> data arrives, even if the pipe was not empty.
>

well io_uring could set the flag as well in that case? it does not
inherently mean random select and poll consumers definitely need this.

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

[Oleg Nesterov]

On 07/24, Mateusz Guzik wrote:
>
> On Fri, Jul 24, 2026 at 3:58 PM Oleg Nesterov <oleg@redhat.com> wrote:
> >
> > This means that io_uring depends on the "nasty semantics" too, io_poll_wake()
> > path should add the task work which calls io_req_post_cqe() every time the new
> > data arrives, even if the pipe was not empty.
> >
>
> well io_uring could set the flag as well in that case? it does not
> inherently mean random select and poll consumers definitely need this.

How? I don't think io_uring can do this. It doesn't even know if the file
is pipe or not. Only file_operations->poll() does know...

__Perhaps__ pipe_poll() could check something like wait->_key & EPOLLET to
detect both epoll/io_uring... Not sure, probably not, I'll try to check later.

Oleg.