Revert "usb: gadget: composite: dequeue cdev->req before free it in composite_dev_cleanup"

Message ID 1411050692-25763-1-git-send-email-balbi@ti.com
State Accepted
Commit bf17eba7ae1e813b0ad67cb1078dcbd7083b906e
Headers show

Commit Message

Felipe Balbi Sept. 18, 2014, 2:31 p.m.
This reverts commit f2267089ea17fa97b796b1b4247e3f8957655df3.

That commit causes more problem than fixes. Firstly, kfree()
should be called after usb_ep_dequeue() and secondly, the way
things are, we will try to dequeue a request that has already
completed much more frequently than one which is pending.

Cc: Li Jun <b47624@freescale.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
---

Greg, can you still apply this for v3.17 final ? Please take it as a patch
directly so we avoid a pull request for a single patch. If you prefer a pull,
let me know.

Li Jun, a proper change to guarantee there will be no pending requests when we
unload a composite gadget driver needs to be more involved. I'll send patches
(which I plan on adding to v3.19 only) shortly. Please give me your Tested-by
once I send such patches.

 drivers/usb/gadget/composite.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Li Jun Sept. 19, 2014, 1:04 p.m. | #1
On Thu, Sep 18, 2014 at 09:31:32AM -0500, Felipe Balbi wrote:
> This reverts commit f2267089ea17fa97b796b1b4247e3f8957655df3.
> 
> That commit causes more problem than fixes. Firstly, kfree()
> should be called after usb_ep_dequeue() and secondly, the way
> things are, we will try to dequeue a request that has already
> completed much more frequently than one which is pending.
> 
> Cc: Li Jun <b47624@freescale.com>
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---
> 
> Greg, can you still apply this for v3.17 final ? Please take it as a patch
> directly so we avoid a pull request for a single patch. If you prefer a pull,
> let me know.
> 
> Li Jun, a proper change to guarantee there will be no pending requests when we
> unload a composite gadget driver needs to be more involved. I'll send patches
> (which I plan on adding to v3.19 only) shortly. Please give me your Tested-by
> once I send such patches.
> 
okay, wait your patches.

Li Jun
>  drivers/usb/gadget/composite.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index 6935a82..f801519 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -1956,7 +1956,6 @@ void composite_dev_cleanup(struct usb_composite_dev *cdev)
>  	}
>  	if (cdev->req) {
>  		kfree(cdev->req->buf);
> -		usb_ep_dequeue(cdev->gadget->ep0, cdev->req);
>  		usb_ep_free_request(cdev->gadget->ep0, cdev->req);
>  	}
>  	cdev->next_string_id = 0;
> -- 
> 2.1.0.243.g30d45f7
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Felipe Balbi Sept. 22, 2014, 2:28 p.m. | #2
Hi,

On Thu, Sep 18, 2014 at 09:31:32AM -0500, Felipe Balbi wrote:
> This reverts commit f2267089ea17fa97b796b1b4247e3f8957655df3.
> 
> That commit causes more problem than fixes. Firstly, kfree()
> should be called after usb_ep_dequeue() and secondly, the way
> things are, we will try to dequeue a request that has already
> completed much more frequently than one which is pending.
> 
> Cc: Li Jun <b47624@freescale.com>
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---
> 
> Greg, can you still apply this for v3.17 final ? Please take it as a patch
> directly so we avoid a pull request for a single patch. If you prefer a pull,
> let me know.

looks like -rc6 will be the last -rc for v3.17. If we can't get this on
v3.17-final, do you mind merging this on usb-next and add a Cc stable
#3.17 ?

leaving patch below for reference only.

> Li Jun, a proper change to guarantee there will be no pending requests when we
> unload a composite gadget driver needs to be more involved. I'll send patches
> (which I plan on adding to v3.19 only) shortly. Please give me your Tested-by
> once I send such patches.
> 
>  drivers/usb/gadget/composite.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index 6935a82..f801519 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -1956,7 +1956,6 @@ void composite_dev_cleanup(struct usb_composite_dev *cdev)
>  	}
>  	if (cdev->req) {
>  		kfree(cdev->req->buf);
> -		usb_ep_dequeue(cdev->gadget->ep0, cdev->req);
>  		usb_ep_free_request(cdev->gadget->ep0, cdev->req);
>  	}
>  	cdev->next_string_id = 0;
> -- 
> 2.1.0.243.g30d45f7
>
Greg Kroah-Hartman Sept. 23, 2014, 2:55 p.m. | #3
On Mon, Sep 22, 2014 at 09:28:26AM -0500, Felipe Balbi wrote:
> Hi,
> 
> On Thu, Sep 18, 2014 at 09:31:32AM -0500, Felipe Balbi wrote:
> > This reverts commit f2267089ea17fa97b796b1b4247e3f8957655df3.
> > 
> > That commit causes more problem than fixes. Firstly, kfree()
> > should be called after usb_ep_dequeue() and secondly, the way
> > things are, we will try to dequeue a request that has already
> > completed much more frequently than one which is pending.
> > 
> > Cc: Li Jun <b47624@freescale.com>
> > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > ---
> > 
> > Greg, can you still apply this for v3.17 final ? Please take it as a patch
> > directly so we avoid a pull request for a single patch. If you prefer a pull,
> > let me know.
> 
> looks like -rc6 will be the last -rc for v3.17. If we can't get this on
> v3.17-final, do you mind merging this on usb-next and add a Cc stable
> #3.17 ?

Ick, sorry, missed this for an earlier release, will go do this now and
queue it up that way.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Felipe Balbi Sept. 23, 2014, 3:28 p.m. | #4
On Tue, Sep 23, 2014 at 07:55:38AM -0700, Greg KH wrote:
> On Mon, Sep 22, 2014 at 09:28:26AM -0500, Felipe Balbi wrote:
> > Hi,
> > 
> > On Thu, Sep 18, 2014 at 09:31:32AM -0500, Felipe Balbi wrote:
> > > This reverts commit f2267089ea17fa97b796b1b4247e3f8957655df3.
> > > 
> > > That commit causes more problem than fixes. Firstly, kfree()
> > > should be called after usb_ep_dequeue() and secondly, the way
> > > things are, we will try to dequeue a request that has already
> > > completed much more frequently than one which is pending.
> > > 
> > > Cc: Li Jun <b47624@freescale.com>
> > > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > > ---
> > > 
> > > Greg, can you still apply this for v3.17 final ? Please take it as a patch
> > > directly so we avoid a pull request for a single patch. If you prefer a pull,
> > > let me know.
> > 
> > looks like -rc6 will be the last -rc for v3.17. If we can't get this on
> > v3.17-final, do you mind merging this on usb-next and add a Cc stable
> > #3.17 ?
> 
> Ick, sorry, missed this for an earlier release, will go do this now and
> queue it up that way.

Thanks

Patch

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 6935a82..f801519 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -1956,7 +1956,6 @@  void composite_dev_cleanup(struct usb_composite_dev *cdev)
 	}
 	if (cdev->req) {
 		kfree(cdev->req->buf);
-		usb_ep_dequeue(cdev->gadget->ep0, cdev->req);
 		usb_ep_free_request(cdev->gadget->ep0, cdev->req);
 	}
 	cdev->next_string_id = 0;