Re: idea: user to user pipe copy

[Mark Borgerding <mark@borgerding.net>]

Mark Borgerding wrote:

> Would someone tell me why this
> a) won't work?
> b) shouldn't be done?
> c) is the dumbest idea since Microsoft Bob?
>
>
> Currently, piped data gets copied from user space to a kernel buffer 
> then back out to user space.
> This happens regardless of whether there is already a reader who is 
> blocked on that fd.
>
> Instead ...
>
> Why not keep track of blocked read()s on a pipe fd?
>
> When the writer writes something to the pipe, data could be copied 
> directly from one user process to another, rather than
> calling copy_from_user then copy_to_user.
> This alleged speed increase would benefit all blocking pipes & fifos, 
> roughly half the time (i.e. whenever the read happens before the write).
>
> -- Mark Borgerding


Here is a rough idea how I think it could be implemented in fs/pipe.c 
(diff from 2.6.5).  The patch is just comments to help me flesh out the 
concept.

I'd appreciate any suggestions on how to implement the user-to-user 
memory copy between processes. 

-- Mark Borgerding


@@ -156,7 +156,25 @@ pipe_readv(struct file *filp, const stru
                        wake_up_interruptible_sync(PIPE_WAIT(*inode));
                        kill_fasync(PIPE_FASYNC_WRITERS(*inode), SIGIO, 
POLL_OUT);
                }
+        /* MB-TODO
+         Put struct iovec* into a waiting_reader member of pipe_inode_info
+         so that the writer can write directly to this caller's buffer.
+
+         if ( inode->waiting_reader == NULL )
+            inode->waiting_reader = iov;
+        */
+
                pipe_wait(inode);
+        /* MB-TODO
+         Check the waiting_reader struct to see if a writer has changed it.
+         Adjust the  byte lengths accordingly.
+
+         if ( inode->waiting_reader == iov ) {
+            ret += total_len - iov_length(iov, nr_segs);
+            inode->waiting_reader = NULL;
+         }
+
+         */
        }
        up(PIPE_SEM(*inode));
        /* Signal writers asynchronously that there is more room.  */
@@ -224,13 +242,31 @@ pipe_writev(struct file *filp, const str
                        if (chars > free)
                                chars = free;

+            /*
+             MB-TODO
+             Check to see if there is a current waiting_reader
+             on this inode.  If so call ,
+                pipe_iov_copy_user_to_user( TBD )
+             rather than
+                pipe_iov_copy_from_user
+
+             if ( inode->waiting_reader ) {
+                if( pipe_iov_copy_user_to_user( inode->waiting_reader , 
iov,chars) ) {
+                    if (!ret) ret = -EFAULT;
+                    break;
+                }
+             }
+
+            */
                        if (pipe_iov_copy_from_user(pipebuf, iov, chars)) {
                                if (!ret) ret = -EFAULT;
                                break;
                        }
                        ret += chars;
-
-                       PIPE_LEN(*inode) += chars;
+
+            /*  The PIPE_LEN does not increase for user-to-user copies
+            if ( ! inode->waiting_reader ) */
+                       PIPE_LEN(*inode) += chars;
                        total_len -= chars;
                        if (!total_len)
                                break;