Re: [PATCH] usb: gadget: f_fs: add "zombie" mode

[Felipe Balbi]

Hi,

On Tue, Oct 07, 2014 at 02:42:33PM -0400, Alan Stern wrote:
> > > It seems to me that we should imitate what an ordinary USB device would
> > > do.  If part of the firmware crashes, generally you would expect none
> > > of the endpoints associated with that function to work.  Either they
> > > refuse to accept output from the host or they stall everything.  But
> > > endpoints associated with other parts of the firmware might very well
> > > continue to work okay.
> > 
> > dunno, I have never seen a USB device firmware crash and I don't think
> > anybody deliberately does anything to make sure other parts of the
> > device work. If it _does_ work, I'd assume it's really by chance.
> 
> I've seen it happen lots of times, but only on single-function devices.  
> When it somes to multi-function devices, who knows?
> 
> Still, with the single-function devices, firmware crashes generally 
> don't lead to disconnections.  Sometimes they do, but usually they 
> don't.
> 
> > > Don't buffer requests.  Either allow the internal FIFOs to fill up or
> > > else reject everything.  Any reasonable host will start getting timeout
> > > expirations and will realize that something is wrong.
> > 
> > Right, but if we allow this, I can already see folks abusing to connect
> > to the host early and only when necessary do some trickery to e.g. start
> > adbd (not saying Android will do this, just using it as an easy
> > example).
> 
> We can still keep the pullup turned off until all the functions are
> ready.  That's a part of normal behavior -- unlike what happens when a
> userspace component crashes or is killed.
> 
> > Sure, we can deactivate and only activate when files are opened but is
> > there any guarantee that when a process receives segfault that we will
> > have, from FFS point of view, any information to know that the thing
> > crashed ? I mean, a userland application can register its own handler
> > for SIGSEGV/SIGKILL, right ? And that handler could very well just call
> > close() on all file descriptors. Then how do we differentiate a normal
> > close() from a "oh-crap-I-died" close() ?
> 
> We can't, so why worry about it?

because on close(), I want to disconnect data pullups :-) Everything has
been tore down and there's nothing else to do.

> If a file handle was closed for normal reasons then userspace probably 
> in the middle of shutting down the gadget anyway.  If not then the 
> user will get what they deserve.

yeah, I think the same way about a crashing functionfs daemon :-)

> If the file handle was closed for abnormal reasons, we can behave like 
> crashed firmware.  Which means, in the end, doing the same thing as in 
> the normal-reason case -- i.e., do nothing.  In particular, don't 
> disconnect.
> 
> If you want to allow for the possibility of orderly shutdown (and maybe 
> even possible restart) of a userspace handler, the function library 
> should first tell the kernel explicitly to disconnect.  Then function 
> components can be changed around completely, and when everything is 
> ready, userspace can tell the kernel to connect again.

I still feel iffy about it, but I must say I understand where you're
coming from. It's weird to force a disconnect, sure. I guess we could
accept this with a new option (just not 'zombie', perhaps no_disconnect
:-) but only if we still have the same "delay pullups until daemon is
running" requirement.

/me hides

-- 
balbi

Attachment: signature.asc
Description: Digital signature