Re: [PATCH 4/5] usb,early_printk: unregister early usb beforerest_init()

[Ingo Molnar]

* Jason Wessel <jason.wessel@xxxxxxxxxxxxx> wrote:

> Ingo Molnar wrote:
> > * Jason Wessel <jason.wessel@xxxxxxxxxxxxx> wrote:
> >
> >> The early_printk EHCI debug driver can hang the system or cause
> >> unpredictable results because two separate APIs will attempt to write
> >> to the mapped PCI region.
> [clip]
> >> +postcore_initcall(usb_early_debug_cleanup);
> >
> > We already have CON_BOOT which allows the unregistering of early
> > consoles, via disable_boot_consoles() initcall in kernel/printk.c.
> >
> > The hang should be solved differently: either by calling
> > disable_boot_consoles() explicitly after console_init() - or by
> > introducing another CON_ flag that marks the console for early
> > forced unregistering.
> 
> I thought about adding another flag in console_init() such that an
> early console which cannot safely be used can elect to unregister.
> There are two problems with this.
> 
> 1) We actually need a call back to unset ehci_debug, or the
>    console_unregister needs to set/clear a flag which then must be
>    checked on each early_printk write, else any further call laying
>    around for early_printk can crash the system.

yeah, it should definitely be unregistered in the 'blackout' period.

> 
> 2) Ideally you want to keep printk alive for as long as possible,
>    which means you can have it all the way through start_kernel()
>    before the "blackout period" while waiting for pci and usb init.
> 
> Given these two issues, is it your preference still to see a patch 
> that changes console_init or add another call back to 
> start_kernel() ?
> 
> An earlier iteration of this patch added a call back in 
> start_kernel just before rest_init(), but it seemed like it was 
> not needed since we can hook on to part of the relevent function 
> init chain.
> 
> I am open to fixing this in what ever manner is acceptable.

My main point is to please use clean init sequences, instead of 
relying on the current (pretty random) semantics of postcore_init().

It is a property of the USB code that it has trouble with the early 
console - so an explicit call to unregister_usb_early_consoles() 
call would solve it.

Probably followed by a reregister_usb_early_consoles() call - your 
patch AFAICS breaks earlyprint=dbg,...,keep functionality, right?

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/