Skip to contentSkip to conversation

pipe: do not return POLLERR for fifo_poll

PATCH New Andrew Morton 3 participants 4 messages Aug 26, 2005 2 messages not in the archive VFS

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

Andrew Morton

author
#2 lore raw
Quoted 38 lines · Pekka Enberg

This patch changes fifo_poll not to return POLLERR to take care of a FIXME in fs/pipe.c stating that "Most unices do not set POLLERR for fifos." The comment has been there since 2.3.99-pre3 so either apply this patch or alternatively, I can send a new one removing the unnecessary abstraction.

...

fs/pipe.c +7 -5
@@ -399,8 +399,8 @@ pipe_ioctl(struct inode *pino, struct fi
 }
 
 /* No kernel lock held - fine */
-static unsigned int
-pipe_poll(struct file *filp, poll_table *wait)
+static inline unsigned int
+__pipe_poll(struct file *filp, poll_table *wait, int can_err)
 {
 	unsigned int mask;
 	struct inode *inode = filp->f_dentry->d_inode;

@@ -420,15 +420,24 @@ pipe_poll(struct file *filp, poll_table 
 
 	if (filp->f_mode & FMODE_WRITE) {
 		mask |= (nrbufs < PIPE_BUFFERS) ? POLLOUT | POLLWRNORM : 0;
-		if (!info->readers)
+		if (can_err && !info->readers)
 			mask |= POLLERR;
 	}
 
 	return mask;
 }
 
-/* FIXME: most Unices do not set POLLERR for fifos */
-#define fifo_poll pipe_poll
+static unsigned int
+pipe_poll(struct file *filp, poll_table *wait)
+{
+	return __pipe_poll(filp, wait, 1);

fs/pipe.c pipe_poll(struct file *filp, poll_table · line 443

Quoted 10 lines
fs/pipe.c +7 -0
+}
+
+static unsigned int
+fifo_poll(struct file *filp, poll_table *wait)
+{
+	return __pipe_poll(filp, wait, 0);
+}
 
 static int
 pipe_release(struct inode *inode, int decr, int decw)

A userspace-visible change, no?

So there's a risk in changing it. What do we get in return? Worried.

#3 lore raw
Quoted 4 lines · Pekka Enberg
Quoted 4 lines

This patch changes fifo_poll not to return POLLERR to take care of a FIXME in fs/pipe.c stating that "Most unices do not set POLLERR for fifos." The comment has been there since 2.3.99-pre3 so either apply this patch or alternatively, I can send a new one removing the unnecessary abstraction.

Quoted 3 lines · Andrew Morton

A userspace-visible change, no?

So there's a risk in changing it. What do we get in return? Worried.

FWIW I have been running on this for few days now without any noticeable regressions. We get a solved FIXME in return but like I said I am a happy to remove the redundant abstraction if this is too risky.

Pekka

#4 lore raw
Quoted 4 lines · Pekka J Enberg

FWIW I have been running on this for few days now without any noticeable regressions. We get a solved FIXME in return but like I said I am a happy to remove the redundant abstraction if this is too risky.

I would prefer just to remove the abstraction, together with a comment that Linux fifos behave exactly like pipes, unlike the behavior of most unices.

--
Manfred

#5 lore raw

PATCH 1/1 pipe: remove redundant fifo_poll abstraction

Quoted 3 lines · Manfred Spraul

I would prefer just to remove the abstraction, together with a comment that Linux fifos behave exactly like pipes, unlike the behavior of most unices.

[PATCH] pipe: remove redundant fifo_poll abstraction

This patch removes a redundant fifo_poll() abstraction from fs/pipe.c and adds a big fat comment stating we set POLLERR for FIFOs too on Linux unlike most Unices.

Signed-off-by Pekka Enberg <[email protected]>

---

 pipe.c |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
fs/pipe.c +7 -6
Index: 2.6-mm/fs/pipe.c
===================================================================
@@ -419,6 +419,10 @@ pipe_poll(struct file *filp, poll_table 
 
 	if (filp->f_mode & FMODE_WRITE) {
 		mask |= (nrbufs < PIPE_BUFFERS) ? POLLOUT | POLLWRNORM : 0;
+		/*
+		 * Most Unices do not set POLLERR for FIFOs but on Linux they
+		 * behave exactly like pipes for poll().
+		 */
 		if (!PIPE_READERS(*inode))
 			mask |= POLLERR;
 	}

@@ -426,9 +430,6 @@ pipe_poll(struct file *filp, poll_table 
 	return mask;
 }
 
-/* FIXME: most Unices do not set POLLERR for fifos */
-#define fifo_poll pipe_poll
-
 static int
 pipe_release(struct inode *inode, int decr, int decw)
 {

@@ -572,7 +573,7 @@ struct file_operations read_fifo_fops = 
 	.read		= pipe_read,
 	.readv		= pipe_readv,
 	.write		= bad_pipe_w,
-	.poll		= fifo_poll,
+	.poll		= pipe_poll,
 	.ioctl		= pipe_ioctl,
 	.open		= pipe_read_open,
 	.release	= pipe_read_release,

@@ -584,7 +585,7 @@ struct file_operations write_fifo_fops =
 	.read		= bad_pipe_r,
 	.write		= pipe_write,
 	.writev		= pipe_writev,
-	.poll		= fifo_poll,
+	.poll		= pipe_poll,
 	.ioctl		= pipe_ioctl,
 	.open		= pipe_write_open,
 	.release	= pipe_write_release,

@@ -597,7 +598,7 @@ struct file_operations rdwr_fifo_fops = 
 	.readv		= pipe_readv,
 	.write		= pipe_write,
 	.writev		= pipe_writev,
-	.poll		= fifo_poll,
+	.poll		= pipe_poll,
 	.ioctl		= pipe_ioctl,
 	.open		= pipe_rdwr_open,
 	.release	= pipe_rdwr_release,