Re: [PATCH v3 0/2] fs/pipe: reduce pipe->mutex contention by pre-allocating outside the lock

[Oleg Nesterov <oleg@redhat.com>]

On 06/17, Mateusz Guzik wrote:
>
> There are trivial touch ups which can be done by adding a bunch of
> predicts and inlining kill_fasync if someone can be bothered.

Speaking of trivial touch ups...

anon_pipe_write() does:

	 * Epoll nonsensically wants a wakeup whether the pipe
	 * was already empty or not.
	 */
	if (was_empty || pipe->poll_usage)
		wake_up_interruptible_sync_poll(&pipe->rd_wait, EPOLLIN | EPOLLRDNORM);

Again, I have no idea if the unnecessary wakeup affects the performance,
probably not. But somehow this "|| poll_usage" condition looks annoying
to me...

Perhaps it makes sense to change pipe_poll() to not set ->poll_usage
unconditionally?

Oleg.
---

diff --git a/fs/pipe.c b/fs/pipe.c
index 429b0714ec57..a60be1b71eb7 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -760,10 +760,12 @@ pipe_poll(struct file *filp, poll_table *wait)
 	struct pipe_inode_info *pipe = filp->private_data;
 	union pipe_index idx;
 
+#ifdef CONFIG_EPOLL
 	/* Epoll has some historical nasty semantics, this enables them */
-	if (unlikely(!READ_ONCE(pipe->poll_usage)))
+	if ((filp->f_mode & FMODE_READ) && filp->f_ep
+	    && unlikely(!READ_ONCE(pipe->poll_usage)))
 		WRITE_ONCE(pipe->poll_usage, true);
-
+#endif
 	/*
 	 * Reading pipe state only -- no need for acquiring the semaphore.
 	 *