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

[Andrew Morton]

On Tue, 2 Oct 2007 19:54:53 +0200 (CEST)
Jan Engelhardt [EMAIL PROTECTED] 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 [EMAIL PROTECTED]
 Signed-off-by: Jan Engelhardt [EMAIL PROTECTED]


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)?

-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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.
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[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 [EMAIL PROTECTED]
Signed-off-by: Jan Engelhardt [EMAIL PROTECTED]

---
 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;
 

-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html