Re: USB gadgets with configfs hang reboot

[Michal Nazarewicz <mina86-deATy8a+UHjQT0dZR+AlfA@public.gmane.org>]

> On Fri, 1 Apr 2016, Michal Nazarewicz wrote:
>> @@ -3050,9 +3053,11 @@ 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)
>> -                       return ret;
>> +               if (common->thread_task) {
>
On Fri, Apr 01 2016, Alan Stern wrote:
> Do you mean: if (!common->thread_task)?

Correct.

>> +                       ret = fsg_common_run_thread(fsg->common);
>> +                       if (ret)
>> +                               return ret;
>> +               }
>>         }
>>  
>>         fsg->gadget = gadget;
>> ---- >8 ----------------------------------------------------------------
>> 
>> With that, the whole fsg_common_run_thread call should be moved outside
>> of the if (!no_configfs) and legacy should no longer call it IMO.
>
> I'm not so sure about this.  Earlier I was undecided about what to do
> if there are multiple mass-storage interfaces in the same config, but
> now I realize that one thread won't work in that situation.  The
> difficulty is exception handling.  If an error occurs in one of the
> interfaces, it shouldn't affect the others -- but with only one thread
> you can't make that work.
>
> I don't know the details of how the composite core works (I don't even
> remember the difference between a usb_function and
> a usb_function_instance).

configfs made things quite confusing, yes.  IIRC, usb_function roughly
corresponds to an interface while usb_function_instance corresponds to
a module/function driver, except you can easily create multiple
instances.

> Suppose there are several interfaces in one config, each bound to the
> mass-storage driver.  Then won't the driver's bind routine get called
> several times whenever the config is installed?
>
> If so, then each of those calls should create a new thread.  There's no
> need to check if the thread has already been started.
>
> If not, then there's no way to use f_mass_storage on multiple
> interfaces in a single config.  The driver should refuse to run on
> more than one interface at a time.

Right, but that’s not what is happening.  We have a situation where
single mass storage instance is bound to two interfaces on two different
configs:

	mkdir functions/mass_storage.0
	echo $file > functions/mass_storage.0/lun.0/file
	ln -s functions/mass_storage.0 configs/c.1
	ln -s functions/mass_storage.0 configs/c.2

configfs doesn’t even allows to express situation where a single
instance of a function is bound to multiple interfaces in a single
configuration (IIRC).

At the same time, mass storage should work fine if it’s bound to
multiple configurations.  Only one configuration can be active at any
given time so interfaces on different configurations cannot interfere
with each other.

The problem we are having is that when mass storage is added to
a configuration, fsg_bind is called and it starts the thread.

If someone wanted to have two mass storage interfaces on a single
configuration they would have to create two mass storage instances and
each instance has it’s own fsg_common struct (see fsg_alloc_inst
function).

PS. It seems that legacy/multi is broken because it calls
    fsg_common_run_thread twice.

-- 
Best regards
ミハウ “𝓶𝓲𝓷𝓪86” ナザレヴイツ
«If at first you don’t succeed, give up skydiving»
--
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