On Tue, 5 Apr 2016, Michal Nazarewicz wrote: > > On Tue, 5 Apr 2016, Michal Nazarewicz wrote: > >> When binding the function to usb_configuration, check whether the thread > >> is running before starting another one. Without that, when function > >> instance is added to multiple configurations, fsg_bing starts multiple > >> threads with all but the latest one being forgotten by the driver. This > >> leads to obvious thread leaks, possible lockups when trying to halt the > >> machine and possible more issues. > >> > >> This fixes issues with legacy/multi¹ gadget as well as configfs gadgets > >> when mass_storage function is added to multiple configurations. > >> > >> This change also simplifies API since the legacy gadgets no longer need > >> to worry about starting the thread by themselves (which was where bug > >> in legacy/multi was in the first place). > >> > >> ¹ I have no example failure though. Conclusion that legacy/multi has > >> a bug is based purely on me reading the code. > >> > >> Signed-off-by: Michal Nazarewicz <mina86@xxxxxxxxxx> > > On Tue, Apr 05 2016, Alan Stern wrote: > > This doesn't address the problem I raised in a previous email. > > Sharing one thread among several function instances in the same config > > will not work if one of them encounters an error. > > Each usb_function_instance has its own fsg_common and its own thread. > This was true in the past and is true with this patch as well. We're getting confused by the stupid terminology again. Yes, each usb_function_instance has its own fsg_common and its own thread. But multiple usb_function structures (each one being a separate function instance) can belong to the same usb_function_instance and they will all share the same fsg_common. That's what happened with the nokia driver. It creates one usb_function_instance with two usb_function structures. In this case they are in different configs, so sharing a thread doesn't matter. But it would matter if they were in the same config. > And unless I’m missing something, sharing a thread among multiple > usb_function’s does not prevent the driver from working correctly. Suppose one usb_function is carrying out an I/O operation while another one in the same config gets a Set-Interface request from the host. The request causes the driver to raise an FSG_STATE_CONFIG_CHANGE exception. When the thread sees this exception, it will abort the I/O that it is carrying out for the first usb_function. In other words, exceptions raised by one instance will affect the shared thread even when it's doing work for a different instance. > Having the thread run even when it’s not used may be considered wasteful > but that’s an orthogonal issue to the configfs failure. Yes. Having extra unused threads isn't terrible. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
