All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] pipe: only enable the extra wake_up(rd_wait) for EPOLLET consumers
@ 2026-08-03 12:46 Oleg Nesterov
  2026-08-03 13:32 ` Breno Leitao
  2026-08-12  7:46 ` Christian Brauner
  0 siblings, 2 replies; 4+ messages in thread
From: Oleg Nesterov @ 2026-08-03 12:46 UTC (permalink / raw)
  To: Breno Leitao, Christian Brauner, Jens Axboe, Linus Torvalds,
	Mateusz Guzik, Pavel Begunkov
  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.

The reason is that some legacy epoll(EPOLLET) users depend on historical
per-write wakeups, see commit 3a34b13a88ca ("pipe: make pipe writes always
wake up readers").

Test-case:

	#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;
	}

it fails if WRITE_ONCE(poll_usage, true) is removed from pipe_poll().
However, without EPOLLET in .events, it does not need the extra wakeup
and succeeds even if write() is called only once before the main loop.

Currently io_uring without (unsupported) IORING_POLL_ADD_LEVEL always
sets EPOLLET, and in IORING_POLL_ADD_MULTI mode it depends on per-write
wakeups the same way:

	#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;
	}

the 2nd assert() in the main loop fails without ->poll_usage == true.

Rename ->poll_usage to ->pseudo_edgetrigger to make the purpose clearer,
update the comments, and change pipe_poll() to set ->pseudo_edgetrigger
only if wait->_key & EPOLLET is true. This check should catch both users,
and this way poll/select and epoll without EPOLLET users will not pay for
the extra wakeup.

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

diff --git a/fs/pipe.c b/fs/pipe.c
index 429b0714ec57..84a81db39f8a 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -686,10 +686,9 @@ 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.
+	 * ->pseudo_edgetrigger enables per-write wakeups, see pipe_poll()
 	 */
-	if (was_empty || pipe->poll_usage)
+	if (was_empty || READ_ONCE(pipe->pseudo_edgetrigger))
 		wake_up_interruptible_sync_poll(&pipe->rd_wait, EPOLLIN | EPOLLRDNORM);
 	kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
 	if (wake_next_writer)
@@ -752,7 +751,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 +758,17 @@ 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);
+	/*
+	 * Legacy epoll(EPOLLET) users depend on historical per-write wakeups,
+	 * see 3a34b13a88ca ("pipe: make pipe writes always wake up readers")
+	 * and the ->pseudo_edgetrigger check in anon_pipe_write().
+	 * Currently io_uring sets EPOLLET for multishot polls, so it gets the
+	 * same behaviour.
+	 */
+	if ((filp->f_mode & FMODE_READ) &&
+	    wait && (wait->_key & EPOLLET) &&
+	    unlikely(!READ_ONCE(pipe->pseudo_edgetrigger)))
+		WRITE_ONCE(pipe->pseudo_edgetrigger, 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..1acc76581af5 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?
+ *	@pseudo_edgetrigger: has an EPOLLET consumer, enable per-write wakeups
  *	@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 pseudo_edgetrigger;
 #ifdef CONFIG_WATCH_QUEUE
 	bool note_loss;
 #endif
-- 
2.52.0



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

* Re: [PATCH v4] pipe: only enable the extra wake_up(rd_wait) for EPOLLET consumers
  2026-08-03 12:46 [PATCH v4] pipe: only enable the extra wake_up(rd_wait) for EPOLLET consumers Oleg Nesterov
@ 2026-08-03 13:32 ` Breno Leitao
  2026-08-03 13:55   ` Oleg Nesterov
  2026-08-12  7:46 ` Christian Brauner
  1 sibling, 1 reply; 4+ messages in thread
From: Breno Leitao @ 2026-08-03 13:32 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: Christian Brauner, Jens Axboe, Linus Torvalds, Mateusz Guzik,
	Pavel Begunkov, Alexander Viro, Jan Kara, Alexey Gladkov,
	linux-kernel, linux-fsdevel, io-uring

On Mon, Aug 03, 2026 at 02:46:25PM +0200, Oleg Nesterov wrote:
> +	 * Currently io_uring sets EPOLLET for multishot polls, so it gets the
> +	 * same behaviour.

I got the impression that Linus wanted the comment epoll-only, and
wanted the io_uring case treated as something to remove, not to bless.


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

* Re: [PATCH v4] pipe: only enable the extra wake_up(rd_wait) for EPOLLET consumers
  2026-08-03 13:32 ` Breno Leitao
@ 2026-08-03 13:55   ` Oleg Nesterov
  0 siblings, 0 replies; 4+ messages in thread
From: Oleg Nesterov @ 2026-08-03 13:55 UTC (permalink / raw)
  To: Breno Leitao
  Cc: Christian Brauner, Jens Axboe, Linus Torvalds, Mateusz Guzik,
	Pavel Begunkov, Alexander Viro, Jan Kara, Alexey Gladkov,
	linux-kernel, linux-fsdevel, io-uring

On 08/03, Breno Leitao wrote:
>
> On Mon, Aug 03, 2026 at 02:46:25PM +0200, Oleg Nesterov wrote:
> > +	 * Currently io_uring sets EPOLLET for multishot polls, so it gets the
> > +	 * same behaviour.
>
> I got the impression that Linus wanted the comment epoll-only, and
> wanted the io_uring case treated as something to remove, not to bless.

Well, perhaps I misunderstood Linus... but it seems he was fine with this
comment which I showed in

	https://lore.kernel.org/all/am9an4HWPGaxmbcK@redhat.com/

And. This comment (and the test-case in the changelog) doesn't try to bless
the current behaviour, it only tries to document the "status quo".

Let me also quote my other reply to Linus:

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


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

* Re: [PATCH v4] pipe: only enable the extra wake_up(rd_wait) for EPOLLET consumers
  2026-08-03 12:46 [PATCH v4] pipe: only enable the extra wake_up(rd_wait) for EPOLLET consumers Oleg Nesterov
  2026-08-03 13:32 ` Breno Leitao
@ 2026-08-12  7:46 ` Christian Brauner
  1 sibling, 0 replies; 4+ messages in thread
From: Christian Brauner @ 2026-08-12  7:46 UTC (permalink / raw)
  To: Breno Leitao, Jens Axboe, Linus Torvalds, Mateusz Guzik,
	Pavel Begunkov, Oleg Nesterov
  Cc: Alexander Viro, Jan Kara, Alexey Gladkov, linux-kernel,
	linux-fsdevel, io-uring

On Mon, 03 Aug 2026 14:46:25 +0200, Oleg Nesterov wrote:
> pipe: only enable the extra wake_up(rd_wait) for EPOLLET consumers

Applied to the vfs-7.3.misc branch of the vfs/vfs.git tree.
Patches in the vfs-7.3.misc branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs-7.3.misc

[1/1] pipe: only enable the extra wake_up(rd_wait) for EPOLLET consumers
      https://git.kernel.org/vfs/vfs/c/07c3efe20e24


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

end of thread, other threads:[~2026-08-12  7:47 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-03 12:46 [PATCH v4] pipe: only enable the extra wake_up(rd_wait) for EPOLLET consumers Oleg Nesterov
2026-08-03 13:32 ` Breno Leitao
2026-08-03 13:55   ` Oleg Nesterov
2026-08-12  7:46 ` Christian Brauner

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.