On Fri, 2006-02-03 at 17:40 -0800, David Brownell wrote:
> > Thinking about it, (like regular vectored IO) it makes sense to return
> > success if we already queued up part of IO, in case of and error.
> > Isn't it ? This way, one can go and wait for those IOs to finish.
> 
> Doesn't make a lot of sense to me, but then I'm probably missing
> something by having seen about zero context for this notion of
> merging two radically different I/O models.  Got any URLs for LKML
> discussion about that?
> 
> AIO is all about one thing:  decoupling the submission of an I/O request
> from its completion.  Applications can issue any number of requests and
> let them complete whenever the OS manages it ... then apps will collect
> completion status whenever it's convenient to them.  Usefully, it's an
> exact match for the low level I/O model of usb_request (on the peripheral
> or gadget side) or URB (on the host side).  Call it non-threaded I/O and
> you'd be on the track of what folk like most about it.  And likewise,
> what bothers a few relgious nuts most deeply about it ... but generally,
> not anyone who writes code that talks directly with the hardware.  ;)
> 
> readv/writev is all about a different thing:  coupling the synchronous
> issuance and completion of a _group_ of related requests.  It does not map
> directly to any of the controller level interfaces of USB, and the closest
> match is the synchronous scatterlist API ... a library on top of usbcore.
> The synchronization is explicitly threaded, and there's some very careful
> attention paid to fault semantics (at least, in USB there is).
> 
> I think you can see why I'm wondering about the basic concept underlying
> those patches.  I could understand writing a library to map readv/writev
> calls into aio, and arranging to use that for filetypes that don't have
> any direct implementation for those syscalls; easy to agree to that.

Dave,

To give you little context here..

Zach Brown proposed vectored AIO interfaces and support. He added
aio_readv/aio_writev file-operations to support this. Christoph Hellwig
pointed out that, we are adding yet another file-op and proposed to
cleanup file-ops and collapse aio_read/aio_readv/readv into a single
file-op which is aio_read() to support all of those.

http://marc.theaimsgroup.com/?l=linux-kernel&m=113097423105516&w=2

I am doing this collapsing, as part of it - I modified aio_read
()/aio_write() methods to take a vector. 

http://marc.theaimsgroup.com/?l=linux-kernel&m=113889744019486&w=2

If usb/gadget never be called to handle more than a single vector,
my original (attached) patch would be good enough. But if it need
to support more vectors, I am not sure whats the best way to support
it.

Does this make sense ?

Thanks,
Badari


This patch vectorizes aio_read() and aio_write() methods to prepare
for colapsing all the vectored operations into one interface -
which is aio_read()/aio_write().


Signed-off-by: Badari Pulavarty <[EMAIL PROTECTED]>
Index: linux-2.6.16-rc1.quilt/Documentation/filesystems/Locking
===================================================================
--- linux-2.6.16-rc1.quilt.orig/Documentation/filesystems/Locking       
2006-02-01 16:36:55.000000000 -0800
+++ linux-2.6.16-rc1.quilt/Documentation/filesystems/Locking    2006-02-01 
16:38:14.000000000 -0800
@@ -355,10 +355,9 @@
 prototypes:
        loff_t (*llseek) (struct file *, loff_t, int);
        ssize_t (*read) (struct file *, char __user *, size_t, loff_t *);
-       ssize_t (*aio_read) (struct kiocb *, char __user *, size_t, loff_t);
        ssize_t (*write) (struct file *, const char __user *, size_t, loff_t *);
-       ssize_t (*aio_write) (struct kiocb *, const char __user *, size_t,
-                       loff_t);
+       ssize_t (*aio_read) (struct kiocb *, const struct iovec *, unsigned 
long, loff_t);
+       ssize_t (*aio_write) (struct kiocb *, const struct iovec *, unsigned 
long, loff_t);
        int (*readdir) (struct file *, void *, filldir_t);
        unsigned int (*poll) (struct file *, struct poll_table_struct *);
        int (*ioctl) (struct inode *, struct file *, unsigned int,
Index: linux-2.6.16-rc1.quilt/drivers/usb/gadget/inode.c
===================================================================
--- linux-2.6.16-rc1.quilt.orig/drivers/usb/gadget/inode.c      2006-02-01 
16:16:50.000000000 -0800
+++ linux-2.6.16-rc1.quilt/drivers/usb/gadget/inode.c   2006-02-01 
16:39:06.000000000 -0800
@@ -675,32 +675,46 @@
 }
 
 static ssize_t
-ep_aio_read(struct kiocb *iocb, char __user *ubuf, size_t len, loff_t o)
+ep_aio_read(struct kiocb *iocb, const struct iovec *iv,
+               unsigned long count, loff_t o)
 {
        struct ep_data          *epdata = iocb->ki_filp->private_data;
        char                    *buf;
+       size_t                  len;
 
        if (unlikely(epdata->desc.bEndpointAddress & USB_DIR_IN))
                return -EINVAL;
+
+       /* FIXME: Can we really get a vector here ? If so, handle it */
+       BUG_ON(count != 1);
+       len = iv->iov_len;
+
        buf = kmalloc(len, GFP_KERNEL);
        if (unlikely(!buf))
                return -ENOMEM;
        iocb->ki_retry = ep_aio_read_retry;
-       return ep_aio_rwtail(iocb, buf, len, epdata, ubuf);
+       return ep_aio_rwtail(iocb, buf, len, epdata, iv->iov_base);
 }
 
 static ssize_t
-ep_aio_write(struct kiocb *iocb, const char __user *ubuf, size_t len, loff_t o)
+ep_aio_write(struct kiocb *iocb, const struct iovec *iv,
+               unsigned long count, loff_t o)
 {
        struct ep_data          *epdata = iocb->ki_filp->private_data;
        char                    *buf;
+       size_t                  len;
 
        if (unlikely(!(epdata->desc.bEndpointAddress & USB_DIR_IN)))
                return -EINVAL;
+
+       /* FIXME: Can we really get a vector here ? If so, handle it */
+       BUG_ON(count != 1);
+       len = iv->iov_len;
+
        buf = kmalloc(len, GFP_KERNEL);
        if (unlikely(!buf))
                return -ENOMEM;
-       if (unlikely(copy_from_user(buf, ubuf, len) != 0)) {
+       if (unlikely(copy_from_user(buf, iv->iov_base, len) != 0)) {
                kfree(buf);
                return -EFAULT;
        }

Reply via email to