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