Re: [PATCH] (resend) reuse xxx_fifo_fops for xxx_pipe_fops

Re: [PATCH] (resend) reuse xxx_fifo_fops for xxx_pipe_fops

[Christoph Hellwig]

On Tue, Jul 01, 2008 at 11:03:02AM +0200, Denys Vlasenko wrote:
> Hi Andrew, Al,
> 
> I posted this patch a few months ago, but it apparently
> fell thru cracks. Here we go again.
> 
> I noticed that read/write/rdwr_pipe_fops are (1) const and
> (2) exactly identical to xxx_fifo_fops, which are also const.
> 
> Attached patch #defines xxx_pipe_fops as aliases to xxx_fifo_fops.
> Size difference:

Why do you need the #defines?  Just use the _fifo_ name directly.

Re: [PATCH] (resend) reuse xxx_fifo_fops for xxx_pipe_fops

[Andrew Morton]

On Tue, 1 Jul 2008 11:03:02 +0200 Denys Vlasenko <vda.linux@googlemail.com> wrote:

> Hi Andrew, Al,
> 
> I posted this patch a few months ago, but it apparently
> fell thru cracks. Here we go again.
> 
> I noticed that read/write/rdwr_pipe_fops are (1) const and
> (2) exactly identical to xxx_fifo_fops, which are also const.
> 
> Attached patch #defines xxx_pipe_fops as aliases to xxx_fifo_fops.
> Size difference:
> 
> # size linux-2.6.25-rc6*/*/pipe.o
>    text    data     bss     dec     hex filename
>    6534     144       0    6678    1a16 linux-2.6.25-rc6/fs/pipe.o
>    5862     144       0    6006    1776 linux-2.6.25-rc6-pt/fs/pipe.o
> 
> Run-tested on 2.6.26-rc8. Please apply.
> 
> Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

<argh, an attachment.  save-as, read, edit..>

--- linux-2.6.25-rc6.src/fs/pipe.c	Sat Mar 22 23:00:34 2008
> +++ linux-2.6.25-rc6.pipe/fs/pipe.c	Fri Mar 28 15:52:00 2008
> @@ -814,42 +814,9 @@
>  	.fasync		= pipe_rdwr_fasync,
>  };
>  
> -static const struct file_operations read_pipe_fops = {
> -	.llseek		= no_llseek,
> -	.read		= do_sync_read,
> -	.aio_read	= pipe_read,
> -	.write		= bad_pipe_w,
> -	.poll		= pipe_poll,
> -	.unlocked_ioctl	= pipe_ioctl,
> -	.open		= pipe_read_open,
> -	.release	= pipe_read_release,
> -	.fasync		= pipe_read_fasync,
> -};
> -
> -static const struct file_operations write_pipe_fops = {
> -	.llseek		= no_llseek,
> -	.read		= bad_pipe_r,
> -	.write		= do_sync_write,
> -	.aio_write	= pipe_write,
> -	.poll		= pipe_poll,
> -	.unlocked_ioctl	= pipe_ioctl,
> -	.open		= pipe_write_open,
> -	.release	= pipe_write_release,
> -	.fasync		= pipe_write_fasync,
> -};
> -
> -static const struct file_operations rdwr_pipe_fops = {
> -	.llseek		= no_llseek,
> -	.read		= do_sync_read,
> -	.aio_read	= pipe_read,
> -	.write		= do_sync_write,
> -	.aio_write	= pipe_write,
> -	.poll		= pipe_poll,
> -	.unlocked_ioctl	= pipe_ioctl,
> -	.open		= pipe_rdwr_open,
> -	.release	= pipe_rdwr_release,
> -	.fasync		= pipe_rdwr_fasync,
> -};
> +#define read_pipe_fops  read_fifo_fops
> +#define write_pipe_fops write_fifo_fops
> +#define rdwr_pipe_fops  rdwr_fifo_fops
>  
>  struct pipe_inode_info * alloc_pipe_info(struct inode *inode)
>  {

Well OK.  But there's a risk that someone will go and modify
read_fifo_fops without realising that they're also modifying
read_pipe_fops.

So it'd be better to rename read_fifo_fops to (say) shared_read_fops
then do

#define read_pipe_fops shared_read_fops
#define read_fifo_fops shared_read_fops

no?

Re: [PATCH] (resend) reuse xxx_fifo_fops for xxx_pipe_fops

[Andrew Morton]

On Tue, 1 Jul 2008 12:03:50 +0200 Denys Vlasenko <vda.linux@googlemail.com> wrote:

> I think since XXX_pipe_fops are only used in this file,
> just explaining this in the comment would be enough.

no, a comment is only needed when the code is unobvious.  Make
the code obvious and we don't need a comment.

As Christoph pointed out, open-coding shared_read_fops everywhere
might make sense too.  It'd make it harder to unshare them later
on, but that's pretty improbable.

Re: [PATCH] (resend) reuse xxx_fifo_fops for xxx_pipe_fops

[Christoph Hellwig]

On Tue, Jul 01, 2008 at 01:10:45AM -0700, Andrew Morton wrote:
> On Tue, 1 Jul 2008 12:03:50 +0200 Denys Vlasenko <vda.linux@googlemail.com> wrote:
> 
> > I think since XXX_pipe_fops are only used in this file,
> > just explaining this in the comment would be enough.
> 
> no, a comment is only needed when the code is unobvious.  Make
> the code obvious and we don't need a comment.
> 
> As Christoph pointed out, open-coding shared_read_fops everywhere
> might make sense too.  It'd make it harder to unshare them later
> on, but that's pretty improbable.

Deny's latest patch doesn't have the #defines anymore and looks quite
reasonable to me.  If you really insist on magic naming I think shared
is a very bad choice because it doesn't have any useful meaning in this
context.  If you insist on magic naming do _pipefifo_, but I think
the current version with the comment is much better.

[PATCH] (resend) reuse xxx_fifo_fops for xxx_pipe_fops

[Denys Vlasenko]

[-- Attachment #1: Type: text/plain, Size: 642 bytes --]

Hi Andrew, Al,

I posted this patch a few months ago, but it apparently
fell thru cracks. Here we go again.

I noticed that read/write/rdwr_pipe_fops are (1) const and
(2) exactly identical to xxx_fifo_fops, which are also const.

Attached patch #defines xxx_pipe_fops as aliases to xxx_fifo_fops.
Size difference:

# size linux-2.6.25-rc6*/*/pipe.o
   text    data     bss     dec     hex filename
   6534     144       0    6678    1a16 linux-2.6.25-rc6/fs/pipe.o
   5862     144       0    6006    1776 linux-2.6.25-rc6-pt/fs/pipe.o

Run-tested on 2.6.26-rc8. Please apply.

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
--
vda

[-- Attachment #2: reuse_fifo_ops_for_pipe_ops.diff --]
[-- Type: text/x-diff, Size: 1315 bytes --]

--- linux-2.6.25-rc6.src/fs/pipe.c	Sat Mar 22 23:00:34 2008
+++ linux-2.6.25-rc6.pipe/fs/pipe.c	Fri Mar 28 15:52:00 2008
@@ -814,42 +814,9 @@
 	.fasync		= pipe_rdwr_fasync,
 };
 
-static const struct file_operations read_pipe_fops = {
-	.llseek		= no_llseek,
-	.read		= do_sync_read,
-	.aio_read	= pipe_read,
-	.write		= bad_pipe_w,
-	.poll		= pipe_poll,
-	.unlocked_ioctl	= pipe_ioctl,
-	.open		= pipe_read_open,
-	.release	= pipe_read_release,
-	.fasync		= pipe_read_fasync,
-};
-
-static const struct file_operations write_pipe_fops = {
-	.llseek		= no_llseek,
-	.read		= bad_pipe_r,
-	.write		= do_sync_write,
-	.aio_write	= pipe_write,
-	.poll		= pipe_poll,
-	.unlocked_ioctl	= pipe_ioctl,
-	.open		= pipe_write_open,
-	.release	= pipe_write_release,
-	.fasync		= pipe_write_fasync,
-};
-
-static const struct file_operations rdwr_pipe_fops = {
-	.llseek		= no_llseek,
-	.read		= do_sync_read,
-	.aio_read	= pipe_read,
-	.write		= do_sync_write,
-	.aio_write	= pipe_write,
-	.poll		= pipe_poll,
-	.unlocked_ioctl	= pipe_ioctl,
-	.open		= pipe_rdwr_open,
-	.release	= pipe_rdwr_release,
-	.fasync		= pipe_rdwr_fasync,
-};
+#define read_pipe_fops  read_fifo_fops
+#define write_pipe_fops write_fifo_fops
+#define rdwr_pipe_fops  rdwr_fifo_fops
 
 struct pipe_inode_info * alloc_pipe_info(struct inode *inode)
 {

Re: [PATCH] (resend) reuse xxx_fifo_fops for xxx_pipe_fops

[Denys Vlasenko]

On Tuesday 01 July 2008 09:32, Andrew Morton wrote:
> > I noticed that read/write/rdwr_pipe_fops are (1) const and
> > (2) exactly identical to xxx_fifo_fops, which are also const.
> > 
> > Attached patch #defines xxx_pipe_fops as aliases to xxx_fifo_fops.
> > Size difference:
> > 
> > # size linux-2.6.25-rc6*/*/pipe.o
> >    text    data     bss     dec     hex filename
> >    6534     144       0    6678    1a16 linux-2.6.25-rc6/fs/pipe.o
> >    5862     144       0    6006    1776 linux-2.6.25-rc6-pt/fs/pipe.o
> > 
> > Run-tested on 2.6.26-rc8. Please apply.
> > 
> > Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
> 
> <argh, an attachment.  save-as, read, edit..>

Let's see how KMail will cope with inline cut-n-paste...

> > -static const struct file_operations rdwr_pipe_fops = {
> > -	.llseek		= no_llseek,
> > -	.read		= do_sync_read,
> > -	.aio_read	= pipe_read,
> > -	.write		= do_sync_write,
> > -	.aio_write	= pipe_write,
> > -	.poll		= pipe_poll,
> > -	.unlocked_ioctl	= pipe_ioctl,
> > -	.open		= pipe_rdwr_open,
> > -	.release	= pipe_rdwr_release,
> > -	.fasync		= pipe_rdwr_fasync,
> > -};
> > +#define read_pipe_fops  read_fifo_fops
> > +#define write_pipe_fops write_fifo_fops
> > +#define rdwr_pipe_fops  rdwr_fifo_fops
> 
> Well OK.  But there's a risk that someone will go and modify
> read_fifo_fops without realising that they're also modifying
> read_pipe_fops.

Yes, it is possible.

> So it'd be better to rename read_fifo_fops to (say) shared_read_fops
> then do
> 
> #define read_pipe_fops shared_read_fops
> #define read_fifo_fops shared_read_fops

I think since XXX_pipe_fops are only used in this file,
just explaining this in the comment would be enough.

Please take a look at the version below.

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
--
vda


--- linux-2.6.26-rc8/fs/pipe.c.org	Tue Jul  1 11:52:28 2008
+++ linux-2.6.26-rc8/fs/pipe.c	Tue Jul  1 11:58:23 2008
@@ -777,6 +777,8 @@
 /*
  * The file_operations structs are not static because they
  * are also used in linux/fs/fifo.c to do operations on FIFOs.
+ *
+ * Pipes reuse fifos' file_operations structs.
  */
 const struct file_operations read_fifo_fops = {
 	.llseek		= no_llseek,
@@ -815,43 +817,6 @@
 	.fasync		= pipe_rdwr_fasync,
 };
 
-static const struct file_operations read_pipe_fops = {
-	.llseek		= no_llseek,
-	.read		= do_sync_read,
-	.aio_read	= pipe_read,
-	.write		= bad_pipe_w,
-	.poll		= pipe_poll,
-	.unlocked_ioctl	= pipe_ioctl,
-	.open		= pipe_read_open,
-	.release	= pipe_read_release,
-	.fasync		= pipe_read_fasync,
-};
-
-static const struct file_operations write_pipe_fops = {
-	.llseek		= no_llseek,
-	.read		= bad_pipe_r,
-	.write		= do_sync_write,
-	.aio_write	= pipe_write,
-	.poll		= pipe_poll,
-	.unlocked_ioctl	= pipe_ioctl,
-	.open		= pipe_write_open,
-	.release	= pipe_write_release,
-	.fasync		= pipe_write_fasync,
-};
-
-static const struct file_operations rdwr_pipe_fops = {
-	.llseek		= no_llseek,
-	.read		= do_sync_read,
-	.aio_read	= pipe_read,
-	.write		= do_sync_write,
-	.aio_write	= pipe_write,
-	.poll		= pipe_poll,
-	.unlocked_ioctl	= pipe_ioctl,
-	.open		= pipe_rdwr_open,
-	.release	= pipe_rdwr_release,
-	.fasync		= pipe_rdwr_fasync,
-};
-
 struct pipe_inode_info * alloc_pipe_info(struct inode *inode)
 {
 	struct pipe_inode_info *pipe;
@@ -927,7 +892,7 @@
 	inode->i_pipe = pipe;
 
 	pipe->readers = pipe->writers = 1;
-	inode->i_fop = &rdwr_pipe_fops;
+	inode->i_fop = &rdwr_fifo_fops;
 
 	/*
 	 * Mark the inode dirty from the very beginning,
@@ -978,7 +943,7 @@
 	d_instantiate(dentry, inode);
 
 	err = -ENFILE;
-	f = alloc_file(pipe_mnt, dentry, FMODE_WRITE, &write_pipe_fops);
+	f = alloc_file(pipe_mnt, dentry, FMODE_WRITE, &write_fifo_fops);
 	if (!f)
 		goto err_dentry;
 	f->f_mapping = inode->i_mapping;
@@ -1021,7 +986,7 @@
 
 	f->f_pos = 0;
 	f->f_flags = O_RDONLY;
-	f->f_op = &read_pipe_fops;
+	f->f_op = &read_fifo_fops;
 	f->f_mode = FMODE_READ;
 	f->f_version = 0;
 

Re: [PATCH] (resend) reuse xxx_fifo_fops for xxx_pipe_fops

[Denys Vlasenko]

On Tuesday 01 July 2008 10:10, Andrew Morton wrote:
> On Tue, 1 Jul 2008 12:03:50 +0200 Denys Vlasenko <vda.linux@googlemail.com> wrote:
> 
> > I think since XXX_pipe_fops are only used in this file,
> > just explaining this in the comment would be enough.
> 
> no, a comment is only needed when the code is unobvious.  Make
> the code obvious and we don't need a comment.
> 
> As Christoph pointed out, open-coding shared_read_fops everywhere
> might make sense too.  It'd make it harder to unshare them later
> on, but that's pretty improbable.

Ok.

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
--
vda

--- linux-2.6.26-rc8/fs.org/fifo.c	Thu Apr 17 04:49:44 2008
+++ linux-2.6.26-rc8/fs/fifo.c	Tue Jul  1 14:09:13 2008
@@ -57,7 +57,7 @@
 	 *  POSIX.1 says that O_NONBLOCK means return with the FIFO
 	 *  opened, even when there is no process writing the FIFO.
 	 */
-		filp->f_op = &read_fifo_fops;
+		filp->f_op = &read_pipefifo_fops;
 		pipe->r_counter++;
 		if (pipe->readers++ == 0)
 			wake_up_partner(inode);
@@ -86,7 +86,7 @@
 		if ((filp->f_flags & O_NONBLOCK) && !pipe->readers)
 			goto err;
 
-		filp->f_op = &write_fifo_fops;
+		filp->f_op = &write_pipefifo_fops;
 		pipe->w_counter++;
 		if (!pipe->writers++)
 			wake_up_partner(inode);
@@ -105,7 +105,7 @@
 	 *  This implementation will NEVER block on a O_RDWR open, since
 	 *  the process can at least talk to itself.
 	 */
-		filp->f_op = &rdwr_fifo_fops;
+		filp->f_op = &rdwr_pipefifo_fops;
 
 		pipe->readers++;
 		pipe->writers++;
@@ -151,5 +151,5 @@
  * depending on the access mode of the file...
  */
 const struct file_operations def_fifo_fops = {
-	.open		= fifo_open,	/* will set read or write pipe_fops */
+	.open		= fifo_open,	/* will set read_ or write_pipefifo_fops */
 };
--- linux-2.6.26-rc8/fs.org/pipe.c	Tue Jul  1 11:52:28 2008
+++ linux-2.6.26-rc8/fs/pipe.c	Tue Jul  1 14:08:16 2008
@@ -777,8 +777,10 @@
 /*
  * The file_operations structs are not static because they
  * are also used in linux/fs/fifo.c to do operations on FIFOs.
+ *
+ * Pipes reuse fifos' file_operations structs.
  */
-const struct file_operations read_fifo_fops = {
+const struct file_operations read_pipefifo_fops = {
 	.llseek		= no_llseek,
 	.read		= do_sync_read,
 	.aio_read	= pipe_read,
@@ -790,7 +792,7 @@
 	.fasync		= pipe_read_fasync,
 };
 
-const struct file_operations write_fifo_fops = {
+const struct file_operations write_pipefifo_fops = {
 	.llseek		= no_llseek,
 	.read		= bad_pipe_r,
 	.write		= do_sync_write,
@@ -802,7 +804,7 @@
 	.fasync		= pipe_write_fasync,
 };
 
-const struct file_operations rdwr_fifo_fops = {
+const struct file_operations rdwr_pipefifo_fops = {
 	.llseek		= no_llseek,
 	.read		= do_sync_read,
 	.aio_read	= pipe_read,
@@ -815,43 +817,6 @@
 	.fasync		= pipe_rdwr_fasync,
 };
 
-static const struct file_operations read_pipe_fops = {
-	.llseek		= no_llseek,
-	.read		= do_sync_read,
-	.aio_read	= pipe_read,
-	.write		= bad_pipe_w,
-	.poll		= pipe_poll,
-	.unlocked_ioctl	= pipe_ioctl,
-	.open		= pipe_read_open,
-	.release	= pipe_read_release,
-	.fasync		= pipe_read_fasync,
-};
-
-static const struct file_operations write_pipe_fops = {
-	.llseek		= no_llseek,
-	.read		= bad_pipe_r,
-	.write		= do_sync_write,
-	.aio_write	= pipe_write,
-	.poll		= pipe_poll,
-	.unlocked_ioctl	= pipe_ioctl,
-	.open		= pipe_write_open,
-	.release	= pipe_write_release,
-	.fasync		= pipe_write_fasync,
-};
-
-static const struct file_operations rdwr_pipe_fops = {
-	.llseek		= no_llseek,
-	.read		= do_sync_read,
-	.aio_read	= pipe_read,
-	.write		= do_sync_write,
-	.aio_write	= pipe_write,
-	.poll		= pipe_poll,
-	.unlocked_ioctl	= pipe_ioctl,
-	.open		= pipe_rdwr_open,
-	.release	= pipe_rdwr_release,
-	.fasync		= pipe_rdwr_fasync,
-};
-
 struct pipe_inode_info * alloc_pipe_info(struct inode *inode)
 {
 	struct pipe_inode_info *pipe;
@@ -927,7 +892,7 @@
 	inode->i_pipe = pipe;
 
 	pipe->readers = pipe->writers = 1;
-	inode->i_fop = &rdwr_pipe_fops;
+	inode->i_fop = &rdwr_pipefifo_fops;
 
 	/*
 	 * Mark the inode dirty from the very beginning,
@@ -978,7 +943,7 @@
 	d_instantiate(dentry, inode);
 
 	err = -ENFILE;
-	f = alloc_file(pipe_mnt, dentry, FMODE_WRITE, &write_pipe_fops);
+	f = alloc_file(pipe_mnt, dentry, FMODE_WRITE, &write_pipefifo_fops);
 	if (!f)
 		goto err_dentry;
 	f->f_mapping = inode->i_mapping;
@@ -1021,7 +986,7 @@
 
 	f->f_pos = 0;
 	f->f_flags = O_RDONLY;
-	f->f_op = &read_pipe_fops;
+	f->f_op = &read_pipefifo_fops;
 	f->f_mode = FMODE_READ;
 	f->f_version = 0;
 

Re: [PATCH] (resend) reuse xxx_fifo_fops for xxx_pipe_fops

[Denys Vlasenko]

On Tuesday 01 July 2008 14:11, Denys Vlasenko wrote:
> On Tuesday 01 July 2008 10:10, Andrew Morton wrote:
> > On Tue, 1 Jul 2008 12:03:50 +0200 Denys Vlasenko <vda.linux@googlemail.com> wrote:
> > 
> > > I think since XXX_pipe_fops are only used in this file,
> > > just explaining this in the comment would be enough.
> > 
> > no, a comment is only needed when the code is unobvious.  Make
> > the code obvious and we don't need a comment.
> > 
> > As Christoph pointed out, open-coding shared_read_fops everywhere
> > might make sense too.  It'd make it harder to unshare them later
> > on, but that's pretty improbable.
> 
> Ok.

And this one is even better [because it actually compiles :) ].
I forgot to change fs.h - XXX_pipefifo_fops live there.

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
--
vda

--- linux-2.6.26-rc8/fs.org/fifo.c	Thu Apr 17 04:49:44 2008
+++ linux-2.6.26-rc8/fs/fifo.c	Tue Jul  1 14:09:13 2008
@@ -57,7 +57,7 @@
 	 *  POSIX.1 says that O_NONBLOCK means return with the FIFO
 	 *  opened, even when there is no process writing the FIFO.
 	 */
-		filp->f_op = &read_fifo_fops;
+		filp->f_op = &read_pipefifo_fops;
 		pipe->r_counter++;
 		if (pipe->readers++ == 0)
 			wake_up_partner(inode);
@@ -86,7 +86,7 @@
 		if ((filp->f_flags & O_NONBLOCK) && !pipe->readers)
 			goto err;
 
-		filp->f_op = &write_fifo_fops;
+		filp->f_op = &write_pipefifo_fops;
 		pipe->w_counter++;
 		if (!pipe->writers++)
 			wake_up_partner(inode);
@@ -105,7 +105,7 @@
 	 *  This implementation will NEVER block on a O_RDWR open, since
 	 *  the process can at least talk to itself.
 	 */
-		filp->f_op = &rdwr_fifo_fops;
+		filp->f_op = &rdwr_pipefifo_fops;
 
 		pipe->readers++;
 		pipe->writers++;
@@ -151,5 +151,5 @@
  * depending on the access mode of the file...
  */
 const struct file_operations def_fifo_fops = {
-	.open		= fifo_open,	/* will set read or write pipe_fops */
+	.open		= fifo_open,	/* will set read_ or write_pipefifo_fops */
 };
--- linux-2.6.26-rc8/fs.org/pipe.c	Tue Jul  1 11:52:28 2008
+++ linux-2.6.26-rc8/fs/pipe.c	Tue Jul  1 14:08:16 2008
@@ -777,8 +777,10 @@
 /*
  * The file_operations structs are not static because they
  * are also used in linux/fs/fifo.c to do operations on FIFOs.
+ *
+ * Pipes reuse fifos' file_operations structs.
  */
-const struct file_operations read_fifo_fops = {
+const struct file_operations read_pipefifo_fops = {
 	.llseek		= no_llseek,
 	.read		= do_sync_read,
 	.aio_read	= pipe_read,
@@ -790,7 +792,7 @@
 	.fasync		= pipe_read_fasync,
 };
 
-const struct file_operations write_fifo_fops = {
+const struct file_operations write_pipefifo_fops = {
 	.llseek		= no_llseek,
 	.read		= bad_pipe_r,
 	.write		= do_sync_write,
@@ -802,7 +804,7 @@
 	.fasync		= pipe_write_fasync,
 };
 
-const struct file_operations rdwr_fifo_fops = {
+const struct file_operations rdwr_pipefifo_fops = {
 	.llseek		= no_llseek,
 	.read		= do_sync_read,
 	.aio_read	= pipe_read,
@@ -815,43 +817,6 @@
 	.fasync		= pipe_rdwr_fasync,
 };
 
-static const struct file_operations read_pipe_fops = {
-	.llseek		= no_llseek,
-	.read		= do_sync_read,
-	.aio_read	= pipe_read,
-	.write		= bad_pipe_w,
-	.poll		= pipe_poll,
-	.unlocked_ioctl	= pipe_ioctl,
-	.open		= pipe_read_open,
-	.release	= pipe_read_release,
-	.fasync		= pipe_read_fasync,
-};
-
-static const struct file_operations write_pipe_fops = {
-	.llseek		= no_llseek,
-	.read		= bad_pipe_r,
-	.write		= do_sync_write,
-	.aio_write	= pipe_write,
-	.poll		= pipe_poll,
-	.unlocked_ioctl	= pipe_ioctl,
-	.open		= pipe_write_open,
-	.release	= pipe_write_release,
-	.fasync		= pipe_write_fasync,
-};
-
-static const struct file_operations rdwr_pipe_fops = {
-	.llseek		= no_llseek,
-	.read		= do_sync_read,
-	.aio_read	= pipe_read,
-	.write		= do_sync_write,
-	.aio_write	= pipe_write,
-	.poll		= pipe_poll,
-	.unlocked_ioctl	= pipe_ioctl,
-	.open		= pipe_rdwr_open,
-	.release	= pipe_rdwr_release,
-	.fasync		= pipe_rdwr_fasync,
-};
-
 struct pipe_inode_info * alloc_pipe_info(struct inode *inode)
 {
 	struct pipe_inode_info *pipe;
@@ -927,7 +892,7 @@
 	inode->i_pipe = pipe;
 
 	pipe->readers = pipe->writers = 1;
-	inode->i_fop = &rdwr_pipe_fops;
+	inode->i_fop = &rdwr_pipefifo_fops;
 
 	/*
 	 * Mark the inode dirty from the very beginning,
@@ -978,7 +943,7 @@
 	d_instantiate(dentry, inode);
 
 	err = -ENFILE;
-	f = alloc_file(pipe_mnt, dentry, FMODE_WRITE, &write_pipe_fops);
+	f = alloc_file(pipe_mnt, dentry, FMODE_WRITE, &write_pipefifo_fops);
 	if (!f)
 		goto err_dentry;
 	f->f_mapping = inode->i_mapping;
@@ -1021,7 +986,7 @@
 
 	f->f_pos = 0;
 	f->f_flags = O_RDONLY;
-	f->f_op = &read_pipe_fops;
+	f->f_op = &read_pipefifo_fops;
 	f->f_mode = FMODE_READ;
 	f->f_version = 0;
 
--- linux-2.6.26-rc8/include.org/linux/fs.h	Mon Jun 30 15:45:56 2008
+++ linux-2.6.26-rc8/include/linux/fs.h	Tue Jul  1 14:13:22 2008
@@ -1687,9 +1687,9 @@
 extern void make_bad_inode(struct inode *);
 extern int is_bad_inode(struct inode *);
 
-extern const struct file_operations read_fifo_fops;
-extern const struct file_operations write_fifo_fops;
-extern const struct file_operations rdwr_fifo_fops;
+extern const struct file_operations read_pipefifo_fops;
+extern const struct file_operations write_pipefifo_fops;
+extern const struct file_operations rdwr_pipefifo_fops;
 
 extern int fs_may_remount_ro(struct super_block *);