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