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

From: Felipe Balbi
Date: Tue Oct 07 2014 - 14:57:19 EST


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