Re: [RFC] Delay EHCI initialization until khubd is idle

[Frans Pop <elendil@xxxxxxxxx>]

On Tuesday 02 September 2008, Greg KH wrote:
> > > If the messages are annoying people, can we just change them to
> > > debugging levels instead?
> >
> > I suppose so.  But what happens if someone plugs in a device and it
> > really does fail to initialize or enumerate?  Shouldn't that cause an
> > error message to appear?
>
> Yes, that is true.
>
> If people really are annoyed by the messages, they can easily turn them
> all off :)

I'll forgive you that statement because of the smiley :-)

Obviously it's not true as most users 1) will just get and take what 
messages are displayed with the default settings of their distro, which 
may or may not include booting with 'quiet' 2) haven't got a clue how to 
turn all messages off 3) will actually _want_ to see other "real" error 
messages.

Let me reiterate why I'm somewhat fierce when it comes to messages with 
error severity in situations where they are expected and harmless.

* Especially when a user boots with 'quiet', which I expect to be default 
for most distros, such error messages really stand out between other boot 
info displayed on the console. Quite often they will also interfere with 
other messages (e.g. get displayed on the same line as some other, 
unrelated message).

* A lot of regular users who see such messages will think that something 
bad is happening and will report such errors to their distribution or 
kernel bugzilla or lkml, thereby wasting a lot of developer time as each 
report will require at least some time to be checked and closed and at 
worst lead to wild goose chases.

* It's just bad from a consistency PoV: error level messages should 
indicate real errors that a user _does_ need to act on and should not be 
triggered in scenarios that are a predictable part of "normal" system 
boots or other "normal" use cases. The kernel is actually in general 
quite good at that!

What makes it even worse is that the implementation is so sensitive to 
minute timing differences: the user may see unpredictable differences in 
errors or get the errors on different devices from one boot to the next, 
or as a result of minor and outwardly unrelated changes in his setup. 
When such errors are reported the first reaction from a lot of people 
handling bug reports will be "broken hardware", even though in fact there 
is nothing wrong.

> other than that, I don't have any suggestions, except that I don't
> think we should apply this kind of change to the tree.

I can see why this patch by itself is not enough and even undesirable, 
although I can also see it being the start point of a more complex series 
of status cross-checks.
For example, if this patch has triggered (or more generally, when ehci has 
started initializing) then there really is no reason to allow the loading 
of drivers for devices connected to the ohci/uhci hubs to proceed as we 
already know they'll get disconnected again anyway.

The "dead keyboard" issue discussed in the other thread does IMO 
demonstrate that there is a real risk of issues because of the current 
lack of coordination between drivers which makes the outcome sensitive to 
miniscule timing differences and races.

If better coordination between various drivers is not possible then my 
humble opinion is still that the kernel should be made aware _why_ the 
error is being triggered so that, at least during system boot, it can be 
downgraded to debug level if that reason is interruption for ehci 
initialization.

As I cannot contribute to any actual solution to the issue I will stop 
posting now and resign myself to whatever you folks decide. I suspect 
though that this subject will be revisited in the future :-)
I remain available to test any patches of course.

And, whatever the outcome, my sincere thanks to Alan for his patience, 
excellent explanations and willingness to investigate the issue.

Cheers,
FJP
--
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