[patch 1/2] getattr - fill the size of pipes

[patch 1/2] getattr - fill the size of pipes

[Jan Engelhardt]

[PATCH]: Fill the size of pipes

Instead of reporting 0 in size when stating() a pipe, we give the number of
queued bytes. This might avoid using ioctl(FIONREAD) to get this information.

References and derived from: http://lkml.org/lkml/2007/4/2/138
Cc: Eric Dumazet <dada1@cosmosbay.com>
Signed-off-by: Jan Engelhardt <jengelh@gmx.de>

---
 fs/pipe.c |   49 ++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 13 deletions(-)

Index: linux-2.6.23/fs/pipe.c
===================================================================
--- linux-2.6.23.orig/fs/pipe.c
+++ linux-2.6.23/fs/pipe.c
@@ -577,27 +577,35 @@ bad_pipe_w(struct file *filp, const char
 	return -EBADF;
 }
 
+static unsigned int pipe_size(struct inode *inode)
+{
+	struct pipe_inode_info *pipe;
+	unsigned int count, buf;
+	int nrbufs;
+
+	mutex_lock(&inode->i_mutex);
+	pipe   = inode->i_pipe;
+	count  = 0;
+	buf    = pipe->curbuf;
+	nrbufs = pipe->nrbufs;
+	while (--nrbufs >= 0) {
+		count += pipe->bufs[buf].len;
+		buf = (buf + 1) & (PIPE_BUFFERS - 1);
+	}
+	mutex_unlock(&inode->i_mutex);
+	return count;
+}
+
 static int
 pipe_ioctl(struct inode *pino, struct file *filp,
 	   unsigned int cmd, unsigned long arg)
 {
 	struct inode *inode = filp->f_path.dentry->d_inode;
-	struct pipe_inode_info *pipe;
-	int count, buf, nrbufs;
+	unsigned int count;
 
 	switch (cmd) {
 		case FIONREAD:
-			mutex_lock(&inode->i_mutex);
-			pipe = inode->i_pipe;
-			count = 0;
-			buf = pipe->curbuf;
-			nrbufs = pipe->nrbufs;
-			while (--nrbufs >= 0) {
-				count += pipe->bufs[buf].len;
-				buf = (buf+1) & (PIPE_BUFFERS-1);
-			}
-			mutex_unlock(&inode->i_mutex);
-
+			count = pipe_size(inode);
 			return put_user(count, (int __user *)arg);
 		default:
 			return -EINVAL;
@@ -915,6 +923,20 @@ static struct dentry_operations pipefs_d
 	.d_dname	= pipefs_dname,
 };
 
+int pipe_getattr(struct vfsmount *mnt, struct dentry *dentry,
+                 struct kstat *stat)
+{
+	struct inode *inode = dentry->d_inode;
+
+	generic_fillattr(inode, stat);
+	stat->size = pipe_size(inode);
+	return 0;
+}
+
+const struct inode_operations pipe_inode_operations = {
+	.getattr = pipe_getattr,
+};
+
 static struct inode * get_pipe_inode(void)
 {
 	struct inode *inode = new_inode(pipe_mnt->mnt_sb);
@@ -928,6 +950,7 @@ static struct inode * get_pipe_inode(voi
 		goto fail_iput;
 	inode->i_pipe = pipe;
 
+	inode->i_op = &pipe_inode_operations;
 	pipe->readers = pipe->writers = 1;
 	inode->i_fop = &rdwr_pipe_fops;
 

[patch 2/2] getattr - fill the size of FIFOs

[Jan Engelhardt]

[PATCH]: Fill the size of FIFOs

Instead of reporting 0 in size when stating() a pipe, we give the number of
queued bytes. This might avoid using ioctl(FIONREAD) to get this information.

References: http://lkml.org/lkml/2007/4/2/138
Cc: Eric Dumazet <dada1@cosmosbay.com>
Signed-off-by: Jan Engelhardt <jengelh@gmx.de>

---
 fs/pipe.c                 |   20 +++-----------------
 fs/stat.c                 |   14 ++++++++++----
 include/linux/pipe_fs_i.h |    2 ++
 3 files changed, 15 insertions(+), 21 deletions(-)

Index: linux-2.6.23/fs/pipe.c
===================================================================
--- linux-2.6.23.orig/fs/pipe.c
+++ linux-2.6.23/fs/pipe.c
@@ -577,14 +577,15 @@ bad_pipe_w(struct file *filp, const char
 	return -EBADF;
 }
 
-static unsigned int pipe_size(struct inode *inode)
+unsigned int pipe_size(struct inode *inode)
 {
 	struct pipe_inode_info *pipe;
 	unsigned int count, buf;
 	int nrbufs;
 
+	if ((pipe = inode->i_pipe) == NULL)
+		return 0;
 	mutex_lock(&inode->i_mutex);
-	pipe   = inode->i_pipe;
 	count  = 0;
 	buf    = pipe->curbuf;
 	nrbufs = pipe->nrbufs;
@@ -923,20 +924,6 @@ static struct dentry_operations pipefs_d
 	.d_dname	= pipefs_dname,
 };
 
-int pipe_getattr(struct vfsmount *mnt, struct dentry *dentry,
-                 struct kstat *stat)
-{
-	struct inode *inode = dentry->d_inode;
-
-	generic_fillattr(inode, stat);
-	stat->size = pipe_size(inode);
-	return 0;
-}
-
-const struct inode_operations pipe_inode_operations = {
-	.getattr = pipe_getattr,
-};
-
 static struct inode * get_pipe_inode(void)
 {
 	struct inode *inode = new_inode(pipe_mnt->mnt_sb);
@@ -950,7 +937,6 @@ static struct inode * get_pipe_inode(voi
 		goto fail_iput;
 	inode->i_pipe = pipe;
 
-	inode->i_op = &pipe_inode_operations;
 	pipe->readers = pipe->writers = 1;
 	inode->i_fop = &rdwr_pipe_fops;
 
Index: linux-2.6.23/fs/stat.c
===================================================================
--- linux-2.6.23.orig/fs/stat.c
+++ linux-2.6.23/fs/stat.c
@@ -11,6 +11,7 @@
 #include <linux/highuid.h>
 #include <linux/fs.h>
 #include <linux/namei.h>
+#include <linux/pipe_fs_i.h>
 #include <linux/security.h>
 #include <linux/syscalls.h>
 #include <linux/pagemap.h>
@@ -46,11 +47,16 @@ int vfs_getattr(struct vfsmount *mnt, st
 	if (retval)
 		return retval;
 
-	if (inode->i_op->getattr)
-		return inode->i_op->getattr(mnt, dentry, stat);
+	if (inode->i_op->getattr) {
+		retval = inode->i_op->getattr(mnt, dentry, stat);
+	} else {
+		generic_fillattr(inode, stat);
+		retval = 0;
+	}
 
-	generic_fillattr(inode, stat);
-	return 0;
+	if (retval == 0 && S_ISFIFO(inode->i_mode))
+		stat->size = pipe_size(inode);
+	return retval;
 }
 
 EXPORT_SYMBOL(vfs_getattr);
Index: linux-2.6.23/include/linux/pipe_fs_i.h
===================================================================
--- linux-2.6.23.orig/include/linux/pipe_fs_i.h
+++ linux-2.6.23/include/linux/pipe_fs_i.h
@@ -134,6 +134,8 @@ struct pipe_buf_operations {
    memory allocation, whereas PIPE_BUF makes atomicity guarantees.  */
 #define PIPE_SIZE		PAGE_SIZE
 
+extern unsigned int pipe_size(struct inode *);
+
 /* Drop the inode semaphore and wait for a pipe event, atomically */
 void pipe_wait(struct pipe_inode_info *pipe);
 

Re: [patch 1/2] getattr - fill the size of pipes

[Andrew Morton]

On Tue, 2 Oct 2007 19:54:53 +0200 (CEST)
Jan Engelhardt <jengelh@computergmbh.de> wrote:

> [PATCH]: Fill the size of pipes
> 
> Instead of reporting 0 in size when stating() a pipe, we give the number of
> queued bytes. This might avoid using ioctl(FIONREAD) to get this information.
> 
> References and derived from: http://lkml.org/lkml/2007/4/2/138
> Cc: Eric Dumazet <dada1@cosmosbay.com>
> Signed-off-by: Jan Engelhardt <jengelh@gmx.de>


Cute feature, but it is (I assume) a Linux-specific extension and is
something which we'll need to maintain for ever and it invites
unportability to older Linuxes and other OSes and it introduces some risk
of breakage of existing applications.  And it slows down fstat on a pipe.

Given that the info can already be obtained via ioctl(FIONREAD) anyway, I
don't think that (gain > pain)?

Re: [patch 1/2] getattr - fill the size of pipes

[Alan Cox]

> Cute feature, but it is (I assume) a Linux-specific extension and is
> something which we'll need to maintain for ever and it invites

Actually it used to work on the old old Linux pipe code.

> unportability to older Linuxes and other OSes and it introduces some risk
> of breakage of existing applications.  And it slows down fstat on a pipe.

Most Sys5 based boxes happen to put the right value there but not
everyone and its not guaranteed in the slightest
> 
> Given that the info can already be obtained via ioctl(FIONREAD) anyway, I
> don't think that (gain > pain)?

Nor me - any application trying to reduce the syscall count would just do
a very large read and get the data and size in one go.