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

Oleg Nesterov <[email protected]>
Newsgroups gmane.linux.kernel.io-uring,gmane.linux.kernel,gmane.linux.file-systems
Message-ID <[email protected]>
On 07/31, Linus Torvalds wrote:
>
> 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.

OK, I'll rename poll_usage/poll_et to ->pseudo_edgetrigger as Christian
suggests and update the comments as below:

struct pipe_inode_info:

	@pseudo_edgetrigger: has an EPOLLET consumer, enable per-write wakeups

pipe_poll:

	/*
	 * 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);

anon_pipe_write:

	 * ->pseudo_edgetrigger enables per-write wakeups, see pipe_poll()
	 */
	if (was_empty || READ_ONCE(pipe->pseudo_edgetrigger))
		wake_up_interruptible_sync_poll(&pipe->rd_wait, EPOLLIN | EPOLLRDNORM);

Will you agree? Or should I tweak the comments somehow to make it more clear
that the only reason is a user space legacy bug?

Oleg.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.