Re: USB gadgets with configfs hang reboot

[Ivaylo Dimitrov <ivo.g.dimitrov.75-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

Hi,

On  4.04.2016 15:57, Michal Nazarewicz wrote:
> On Sat, Apr 02 2016, Alan Stern wrote:
>> On Sat, 2 Apr 2016, Michal Nazarewicz wrote:
>
> Because of all that, I think the best course of action is to just check
> whether the thread is running and conditionally start it in fsg_bind,
> i.e.:
>
> --- a/drivers/usb/gadget/function/f_mass_storage.c
> +++ b/drivers/usb/gadget/function/f_mass_storage.c
> @@ -2979,20 +2979,7 @@ EXPORT_SYMBOL_GPL(fsg_common_set_inquiry_string);
>
>   int fsg_common_run_thread(struct fsg_common *common)
>   {
> -       common->state = FSG_STATE_IDLE;
> -       /* Tell the thread to start working */
> -       common->thread_task =
> -               kthread_create(fsg_main_thread, common, "file-storage");
> -       if (IS_ERR(common->thread_task)) {
> -               common->state = FSG_STATE_TERMINATED;
> -               return PTR_ERR(common->thread_task);
> -       }
> -
> -       DBG(common, "I/O thread pid: %d\n", task_pid_nr(common->thread_task));
> -
> -       wake_up_process(common->thread_task);
> -
> -       return 0;
> +       /* kill this function and all call sites */
>   }
>   EXPORT_SYMBOL_GPL(fsg_common_run_thread);
>
> @@ -3005,6 +2992,7 @@ static void fsg_common_release(struct kref *ref)
>          if (common->state != FSG_STATE_TERMINATED) {
>                  raise_exception(common, FSG_STATE_EXIT);
>                  wait_for_completion(&common->thread_notifier);
> +               common->thread_task = NULL;
>          }
>
>          for (i = 0; i < ARRAY_SIZE(common->luns); ++i) {
> @@ -3050,9 +3038,21 @@ static int fsg_bind(struct usb_configuration *c, struct usb_function *f)
>                  if (ret)
>                          return ret;
>                  fsg_common_set_inquiry_string(fsg->common, NULL, NULL);
> -               ret = fsg_common_run_thread(fsg->common);
> -               if (ret)
> +       }
> +
> +       if (!common->thread_task) {
> +               common->state = FSG_STATE_IDLE;
> +               common->thread_task =
> +                       kthread_create(fsg_main_thread, common, "file-storage");
> +               if (IS_ERR(common->thread_task)) {
> +                       int ret = PTR_ERR(common->thread_task);
> +                       common->thread_task = NULL;
> +                       common->state = FSG_STATE_TERMINATED;
>                          return ret;
> +               }
> +               DBG(common, "I/O thread pid: %d\n",
> +                   task_pid_nr(common->thread_task));
> +               wake_up_process(common->thread_task);
>          }
>
>          fsg->gadget = gadget;
>
> This should get rid of all the confusion and just do the right thing.
>

Who and when is going to destroy the thread if one does 
"/sys/bus/platform/drivers/musb-hdrc# echo musb-hdrc.0.auto > unbind"? 
Wouldn't some kind of refcounting make sense here?

Regards,
Ivo
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html