On Fri, Jan 17 2014, Robert Baldyga wrote: > @@ -623,8 +644,55 @@ static void ffs_epfile_io_complete(struct usb_ep *_ep, > struct usb_request *req) > } > } > > -static ssize_t ffs_epfile_io(struct file *file, > - char __user *buf, size_t len, int read) > +static void ffs_user_copy_worker(struct work_struct *work) > +{ > + size_t len = 0;
Nit: This would be better named “pos” since it's a position in a io_data->buf buffer. > + int i = 0; > + int ret; > + > + struct ffs_io_data *io_data = container_of(work, struct ffs_io_data, > + work); > + ret = io_data->len; > + > + use_mm(io_data->mm); > + for (i = 0; i < io_data->nr_segs; i++) { > + if (unlikely(copy_to_user(io_data->iovec[i].iov_base, > + &io_data->buf[len], Nit: IMO this is a bit confusing way of writing “io_data->buf + len”. I wouldn't mention it if you cared about pointing to a single character at position len, but here you just move the pointer of the buffer to position len. > + io_data->iovec[i].iov_len))) { > + ret = -EFAULT; > + break; > + } > + len += io_data->iovec[i].iov_len; > + } > + unuse_mm(io_data->mm); > + > + aio_complete(io_data->kiocb, ret, ret); > + > + kfree(io_data->iovec); > + kfree(io_data->buf); > + kfree(io_data); > +} > @@ -667,15 +735,32 @@ static ssize_t ffs_epfile_io(struct file *file, > * Controller may require buffer size to be aligned to > * maxpacketsize of an out endpoint. > */ > - data_len = read ? usb_ep_align_maybe(gadget, ep->ep, len) : len; > + data_len = io_data->read ? usb_ep_align_maybe(gadget, ep->ep, > + io_data->len) : > + io_data->len; Nit: The following is easier to read IMO: data_len = io_data->read ? usb_ep_align_maybe(gadget, ep->ep, io_data->len) : io_data->len; > > data = kmalloc(data_len, GFP_KERNEL); > if (unlikely(!data)) > return -ENOMEM; > - > - if (!read && unlikely(copy_from_user(data, buf, len))) { > - ret = -EFAULT; > - goto error; > + if (io_data->aio && !io_data->read) { > + int i; > + size_t len = 0; > + for (i = 0; i < io_data->nr_segs; i++) { > + if (unlikely(copy_from_user(&data[len], > + io_data->iovec[i].iov_base, > + io_data->iovec[i].iov_len) != 0)) { Nit: != 0 is not necessary here. > + ret = -EFAULT; > + goto error; > + } > + len += io_data->iovec[i].iov_len; > + } > + } else { > + if (!io_data->read && > + unlikely(__copy_from_user(data, io_data->buf, > + io_data->len))) { > + ret = -EFAULT; > + goto error; > + } > } > } > @@ -741,17 +858,31 @@ static ssize_t > ffs_epfile_write(struct file *file, const char __user *buf, size_t len, > loff_t *ptr) > { > + struct ffs_io_data io_data; > + > ENTER(); > > - return ffs_epfile_io(file, (char __user *)buf, len, 0); > + io_data.aio = 0; > + io_data.read = 0; Use “false” since those are now bools. Same for the rest of the code. > + io_data.buf = (char * __user)buf; > + io_data.len = len; > + > + return ffs_epfile_io(file, &io_data); > } > @@ -770,6 +901,80 @@ ffs_epfile_open(struct inode *inode, struct file *file) > return 0; > } > > +static int ffs_aio_cancel(struct kiocb *kiocb) > +{ > + struct ffs_io_data *io_data = kiocb->private; > + struct ffs_epfile *epfile = kiocb->ki_filp->private_data; > + int value; > + > + ENTER(); > + > + spin_lock_irq(&epfile->ffs->eps_lock); > + > + if (likely(io_data && io_data->ep && io_data->req)) > + value = usb_ep_dequeue(io_data->ep, io_data->req); > + else > + value = -EINVAL; > + Nit: Trailing whitespace. > + usb_ep_free_request(io_data->ep, io_data->req); > + > + spin_unlock_irq(&epfile->ffs->eps_lock); > + > + return value; > +} > + > +static ssize_t ffs_epfile_aio_write(struct kiocb *kiocb, > + const struct iovec *iovec, > + unsigned long nr_segs, loff_t loff) > +{ > + struct ffs_io_data *io_data; > + > + ENTER(); > + > + io_data = kmalloc(sizeof(struct ffs_io_data), GFP_KERNEL); io_data = kmalloc(sizeof(*io_data), GFP_KERNEL); And you need to check if allocation succeeded. > + io_data->aio = 1; > + io_data->read = 0; > + io_data->kiocb = kiocb; > + io_data->iovec = iovec; > + io_data->nr_segs = nr_segs; > + io_data->len = kiocb->ki_nbytes; > + io_data->mm = current->mm; > + > + kiocb->private = io_data; > + > + kiocb_set_cancel_fn(kiocb, ffs_aio_cancel); > + > + return ffs_epfile_io(kiocb->ki_filp, io_data); > +} > + > +static ssize_t ffs_epfile_aio_read(struct kiocb *kiocb, > + const struct iovec *iovec, > + unsigned long nr_segs, loff_t loff) > +{ > + struct ffs_io_data *io_data; > + struct iovec *iovec_copy; > + > + ENTER(); > + > + iovec_copy = kmalloc(sizeof(struct iovec)*nr_segs, GFP_KERNEL); > + memcpy(iovec_copy, iovec, sizeof(struct iovec)*nr_segs); > + > + io_data = kmalloc(sizeof(struct ffs_io_data), GFP_KERNEL); iovec_copy = kmalloc_array(nr_segs, sizeof(*iovec_copy), GFP_KERNEL); memcpy(iovec_copy, iovec, sizeof(struct iovec)*nr_segs); io_data = kmalloc(sizeof(*io_data), GFP_KERNEL); Plus you need to check whether allocation succeeded. > + io_data->aio = 1; > + io_data->read = 1; > + io_data->kiocb = kiocb; > + io_data->iovec = iovec_copy; > + io_data->nr_segs = nr_segs; > + io_data->len = kiocb->ki_nbytes; > + io_data->mm = current->mm; > + > + kiocb->private = io_data; > + > + kiocb_set_cancel_fn(kiocb, ffs_aio_cancel); > + > + return ffs_epfile_io(kiocb->ki_filp, io_data); > +} > + > static int > ffs_epfile_release(struct inode *inode, struct file *file) > { -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz (o o) ooo +--<m...@google.com>--<xmpp:min...@jabber.org>--ooO--(_)--Ooo--
signature.asc
Description: PGP signature