On Mon, Jun 08 2015, Sanjay Singh Rawat wrote:
> After the worker thread is launched, bind function is doing further
> configuration. In case of failure stop the thread.
>
> Signed-off-by: Sanjay Singh Rawat <[email protected]>
> Acked-by: Michal Nazarewicz <[email protected]>

I must say, I’m starting to have second thoughts about this patch.

Did you actually experience a thread being leaked? The thread is being
stopped in fsg_common_release which I think is called even if fsg_bind
fails, no?

> ---
>  drivers/usb/gadget/function/f_mass_storage.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_mass_storage.c 
> b/drivers/usb/gadget/function/f_mass_storage.c
> index 3cc109f..0e90e38 100644
> --- a/drivers/usb/gadget/function/f_mass_storage.c
> +++ b/drivers/usb/gadget/function/f_mass_storage.c
> @@ -3082,7 +3082,7 @@ static int fsg_bind(struct usb_configuration *c, struct 
> usb_function *f)
>       /* New interface */
>       i = usb_interface_id(c, f);
>       if (i < 0)
> -             return i;
> +             goto fail;
>       fsg_intf_desc.bInterfaceNumber = i;
>       fsg->interface_number = i;
>  
> @@ -3125,7 +3125,10 @@ static int fsg_bind(struct usb_configuration *c, 
> struct usb_function *f)
>  
>  autoconf_fail:
>       ERROR(fsg, "unable to autoconfigure all endpoints\n");
> -     return -ENOTSUPP;
> +     i = -ENOTSUPP;
> +fail:
> +     kthread_stop(fsg->common->thread_task);

I’ve read up on kthread API a little and I’m no longer sure this is
valid.  This sets kthread_should_stop bit for the thread and waits for
it to finish.  fsg_main_thread does not check kthread_should_stop so
IIUC kthread_stop(fsg->common->thread_task) will actually never return.

Instead it seems we would have to do what is done in fsg_common_release

        if (common->state != FSG_STATE_TERMINATED) {
                raise_exception(common, FSG_STATE_EXIT);
                wait_for_completion(&common->thread_notifier);
        }


> +     return i;
>  }
>  
>  /****************************** ALLOCATE FUNCTION *************************/
> -- 
> 1.8.3.2
>

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +--<[email protected]>--<xmpp:[email protected]>--ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to