Hi, On Thu, Mar 21, 2013 at 09:35:24AM -0700, Kent Overstreet wrote: > From: Zach Brown <z...@redhat.com> > > This removes the only in-tree user of aio retry. This will let us remove > the retry code from the aio core. > > Removing retry is relatively easy as the USB gadget wasn't using it to > retry IOs at all. It always fully submitted the IO in the context of the > initial io_submit() call. It only used the AIO retry facility to get the > submitter's mm context for copying the result of a read back to user > space. This is easy to implement with use_mm() and a work struct, much > like kvm does with async_pf_execute() for get_user_pages(). > > Signed-off-by: Zach Brown <z...@redhat.com> > Signed-off-by: Kent Overstreet <koverstr...@google.com> > Cc: Felipe Balbi <ba...@ti.com> > Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org> > Cc: Mark Fasheh <mfas...@suse.com> > Cc: Joel Becker <jl...@evilplan.org> > Cc: Rusty Russell <ru...@rustcorp.com.au> > Cc: Jens Axboe <ax...@kernel.dk> > Cc: Asai Thambi S P <asamymuth...@micron.com> > Cc: Selvan Mani <sm...@micron.com> > Cc: Sam Bradshaw <sbrads...@micron.com> > Cc: Jeff Moyer <jmo...@redhat.com> > Cc: Al Viro <v...@zeniv.linux.org.uk> > Cc: Benjamin LaHaise <b...@kvack.org> > Cc: Theodore Ts'o <ty...@mit.edu> > Signed-off-by: Andrew Morton <a...@linux-foundation.org>
I don't have any objections with the approach. Let's see if anyone from
linux-usb has anything to say, though
> drivers/usb/gadget/inode.c | 38 +++++++++++++++++++++++++++++---------
> 1 file changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/gadget/inode.c b/drivers/usb/gadget/inode.c
> index e2b2e9c..a1aad43 100644
> --- a/drivers/usb/gadget/inode.c
> +++ b/drivers/usb/gadget/inode.c
> @@ -24,6 +24,7 @@
> #include <linux/sched.h>
> #include <linux/slab.h>
> #include <linux/poll.h>
> +#include <linux/mmu_context.h>
>
> #include <linux/device.h>
> #include <linux/moduleparam.h>
> @@ -513,6 +514,9 @@ static long ep_ioctl(struct file *fd, unsigned code,
> unsigned long value)
> struct kiocb_priv {
> struct usb_request *req;
> struct ep_data *epdata;
> + struct kiocb *iocb;
> + struct mm_struct *mm;
> + struct work_struct work;
> void *buf;
> const struct iovec *iv;
> unsigned long nr_segs;
> @@ -540,15 +544,12 @@ static int ep_aio_cancel(struct kiocb *iocb, struct
> io_event *e)
> return value;
> }
>
> -static ssize_t ep_aio_read_retry(struct kiocb *iocb)
> +static ssize_t ep_copy_to_user(struct kiocb_priv *priv)
> {
> - struct kiocb_priv *priv = iocb->private;
> ssize_t len, total;
> void *to_copy;
> int i;
>
> - /* we "retry" to get the right mm context for this: */
> -
> /* copy stuff into user buffers */
> total = priv->actual;
> len = 0;
> @@ -568,9 +569,26 @@ static ssize_t ep_aio_read_retry(struct kiocb *iocb)
> if (total == 0)
> break;
> }
> +
> + return len;
> +}
> +
> +static void ep_user_copy_worker(struct work_struct *work)
> +{
> + struct kiocb_priv *priv = container_of(work, struct kiocb_priv, work);
> + struct mm_struct *mm = priv->mm;
> + struct kiocb *iocb = priv->iocb;
> + size_t ret;
> +
> + use_mm(mm);
> + ret = ep_copy_to_user(priv);
> + unuse_mm(mm);
> +
> + /* completing the iocb can drop the ctx and mm, don't touch mm after */
> + aio_complete(iocb, ret, ret);
> +
> kfree(priv->buf);
> kfree(priv);
> - return len;
> }
>
> static void ep_aio_complete(struct usb_ep *ep, struct usb_request *req)
> @@ -596,14 +614,14 @@ static void ep_aio_complete(struct usb_ep *ep, struct
> usb_request *req)
> aio_complete(iocb, req->actual ? req->actual : req->status,
> req->status);
> } else {
> - /* retry() won't report both; so we hide some faults */
> + /* ep_copy_to_user() won't report both; we hide some faults */
> if (unlikely(0 != req->status))
> DBG(epdata->dev, "%s fault %d len %d\n",
> ep->name, req->status, req->actual);
>
> priv->buf = req->buf;
> priv->actual = req->actual;
> - kick_iocb(iocb);
> + schedule_work(&priv->work);
> }
> spin_unlock(&epdata->dev->lock);
>
> @@ -633,8 +651,10 @@ fail:
> return value;
> }
> iocb->private = priv;
> + priv->iocb = iocb;
> priv->iv = iv;
> priv->nr_segs = nr_segs;
> + INIT_WORK(&priv->work, ep_user_copy_worker);
>
> value = get_ready_ep(iocb->ki_filp->f_flags, epdata);
> if (unlikely(value < 0)) {
> @@ -646,6 +666,7 @@ fail:
> get_ep(epdata);
> priv->epdata = epdata;
> priv->actual = 0;
> + priv->mm = current->mm; /* mm teardown waits for iocbs in exit_aio() */
>
> /* each kiocb is coupled to one usb_request, but we can't
> * allocate or submit those if the host disconnected.
> @@ -674,7 +695,7 @@ fail:
> kfree(priv);
> put_ep(epdata);
> } else
> - value = (iv ? -EIOCBRETRY : -EIOCBQUEUED);
> + value = -EIOCBQUEUED;
> return value;
> }
>
> @@ -692,7 +713,6 @@ ep_aio_read(struct kiocb *iocb, const struct iovec *iov,
> if (unlikely(!buf))
> return -ENOMEM;
>
> - iocb->ki_retry = ep_aio_read_retry;
> return ep_aio_rwtail(iocb, buf, iocb->ki_left, epdata, iov, nr_segs);
> }
>
> --
> 1.8.1.3
>
--
balbi
signature.asc
Description: Digital signature
