[PATCH] x86/console: process softirqs between warning prints

Roger Pau Monne posted 1 patch 2 years, 2 months ago
Test gitlab-ci failed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20220217082850.19407-1-roger.pau@citrix.com
xen/common/warning.c | 1 +
1 file changed, 1 insertion(+)
[PATCH] x86/console: process softirqs between warning prints
Posted by Roger Pau Monne 2 years, 2 months ago
Process softirqs while printing end of boot warnings. Each warning can
be several lines long, and on slow consoles printing multiple ones
without processing softirqs can result in the watchdog triggering:

(XEN) [   22.277806] ***************************************************
(XEN) [   22.417802] WARNING: CONSOLE OUTPUT IS SYNCHRONOUS
(XEN) [   22.556029] This option is intended to aid debugging of Xen by ensuring
(XEN) [   22.696802] that all output is synchronously delivered on the serial line.
(XEN) [   22.838024] However it can introduce SIGNIFICANT latencies and affect
(XEN) [   22.978710] timekeeping. It is NOT recommended for production use!
(XEN) [   23.119066] ***************************************************
(XEN) [   23.258865] Booted on L1TF-vulnerable hardware with SMT/Hyperthreading
(XEN) [   23.399560] enabled.  Please assess your configuration and choose an
(XEN) [   23.539925] explicit 'smt=<bool>' setting.  See XSA-273.
(XEN) [   23.678860] ***************************************************
(XEN) [   23.818492] Booted on MLPDS/MFBDS-vulnerable hardware with SMT/Hyperthreading
(XEN) [   23.959811] enabled.  Mitigations will not be fully effective.  Please
(XEN) [   24.100396] choose an explicit smt=<bool> setting.  See XSA-297.
(XEN) [   24.240254] *************************************************(XEN) [   24.247302] Watchdog timer detects that CPU0 is stuck!
(XEN) [   24.386785] ----[ Xen-4.17-unstable  x86_64  debug=y  Tainted:   C    ]----
(XEN) [   24.527874] CPU:    0
(XEN) [   24.662422] RIP:    e008:[<ffff82d04025b84a>] drivers/char/ns16550.c#ns16550_tx_ready+0x3a/0x90

Fixes: ee3fd57acd ('xen: add warning infrastructure')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/common/warning.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/common/warning.c b/xen/common/warning.c
index 0269c6715c..e6e1404baf 100644
--- a/xen/common/warning.c
+++ b/xen/common/warning.c
@@ -30,6 +30,7 @@ void __init warning_print(void)
     {
         printk("%s", warnings[i]);
         printk("***************************************************\n");
+        process_pending_softirqs();
     }
 
     for ( i = 0; i < 3; i++ )
-- 
2.34.1


Re: [PATCH] x86/console: process softirqs between warning prints
Posted by Jan Beulich 2 years, 2 months ago
On 17.02.2022 09:28, Roger Pau Monne wrote:
> Process softirqs while printing end of boot warnings. Each warning can
> be several lines long, and on slow consoles printing multiple ones
> without processing softirqs can result in the watchdog triggering:
> 
> (XEN) [   22.277806] ***************************************************
> (XEN) [   22.417802] WARNING: CONSOLE OUTPUT IS SYNCHRONOUS
> (XEN) [   22.556029] This option is intended to aid debugging of Xen by ensuring
> (XEN) [   22.696802] that all output is synchronously delivered on the serial line.
> (XEN) [   22.838024] However it can introduce SIGNIFICANT latencies and affect
> (XEN) [   22.978710] timekeeping. It is NOT recommended for production use!
> (XEN) [   23.119066] ***************************************************
> (XEN) [   23.258865] Booted on L1TF-vulnerable hardware with SMT/Hyperthreading
> (XEN) [   23.399560] enabled.  Please assess your configuration and choose an
> (XEN) [   23.539925] explicit 'smt=<bool>' setting.  See XSA-273.
> (XEN) [   23.678860] ***************************************************
> (XEN) [   23.818492] Booted on MLPDS/MFBDS-vulnerable hardware with SMT/Hyperthreading
> (XEN) [   23.959811] enabled.  Mitigations will not be fully effective.  Please
> (XEN) [   24.100396] choose an explicit smt=<bool> setting.  See XSA-297.
> (XEN) [   24.240254] *************************************************(XEN) [   24.247302] Watchdog timer detects that CPU0 is stuck!
> (XEN) [   24.386785] ----[ Xen-4.17-unstable  x86_64  debug=y  Tainted:   C    ]----
> (XEN) [   24.527874] CPU:    0
> (XEN) [   24.662422] RIP:    e008:[<ffff82d04025b84a>] drivers/char/ns16550.c#ns16550_tx_ready+0x3a/0x90
> 
> Fixes: ee3fd57acd ('xen: add warning infrastructure')
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
>  xen/common/warning.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/xen/common/warning.c b/xen/common/warning.c
> index 0269c6715c..e6e1404baf 100644
> --- a/xen/common/warning.c
> +++ b/xen/common/warning.c
> @@ -30,6 +30,7 @@ void __init warning_print(void)
>      {
>          printk("%s", warnings[i]);
>          printk("***************************************************\n");
> +        process_pending_softirqs();
>      }

To be honest, I'm not convinced. This gets us pretty close to needing
to process softirqs after _every_ line of output. If a console is this
slow, the problem imo needs dealing with there (and according to irc
we appear on a helpful track there already), not by sprinkling more
process_pending_softirqs() all over the code.

Jan


Re: [PATCH] x86/console: process softirqs between warning prints
Posted by Roger Pau Monné 2 years, 2 months ago
On Thu, Feb 17, 2022 at 12:54:57PM +0100, Jan Beulich wrote:
> On 17.02.2022 09:28, Roger Pau Monne wrote:
> > Process softirqs while printing end of boot warnings. Each warning can
> > be several lines long, and on slow consoles printing multiple ones
> > without processing softirqs can result in the watchdog triggering:
> > 
> > (XEN) [   22.277806] ***************************************************
> > (XEN) [   22.417802] WARNING: CONSOLE OUTPUT IS SYNCHRONOUS
> > (XEN) [   22.556029] This option is intended to aid debugging of Xen by ensuring
> > (XEN) [   22.696802] that all output is synchronously delivered on the serial line.
> > (XEN) [   22.838024] However it can introduce SIGNIFICANT latencies and affect
> > (XEN) [   22.978710] timekeeping. It is NOT recommended for production use!
> > (XEN) [   23.119066] ***************************************************
> > (XEN) [   23.258865] Booted on L1TF-vulnerable hardware with SMT/Hyperthreading
> > (XEN) [   23.399560] enabled.  Please assess your configuration and choose an
> > (XEN) [   23.539925] explicit 'smt=<bool>' setting.  See XSA-273.
> > (XEN) [   23.678860] ***************************************************
> > (XEN) [   23.818492] Booted on MLPDS/MFBDS-vulnerable hardware with SMT/Hyperthreading
> > (XEN) [   23.959811] enabled.  Mitigations will not be fully effective.  Please
> > (XEN) [   24.100396] choose an explicit smt=<bool> setting.  See XSA-297.
> > (XEN) [   24.240254] *************************************************(XEN) [   24.247302] Watchdog timer detects that CPU0 is stuck!
> > (XEN) [   24.386785] ----[ Xen-4.17-unstable  x86_64  debug=y  Tainted:   C    ]----
> > (XEN) [   24.527874] CPU:    0
> > (XEN) [   24.662422] RIP:    e008:[<ffff82d04025b84a>] drivers/char/ns16550.c#ns16550_tx_ready+0x3a/0x90
> > 
> > Fixes: ee3fd57acd ('xen: add warning infrastructure')
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > ---
> >  xen/common/warning.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/xen/common/warning.c b/xen/common/warning.c
> > index 0269c6715c..e6e1404baf 100644
> > --- a/xen/common/warning.c
> > +++ b/xen/common/warning.c
> > @@ -30,6 +30,7 @@ void __init warning_print(void)
> >      {
> >          printk("%s", warnings[i]);
> >          printk("***************************************************\n");
> > +        process_pending_softirqs();
> >      }
> 
> To be honest, I'm not convinced. This gets us pretty close to needing
> to process softirqs after _every_ line of output. If a console is this
> slow, the problem imo needs dealing with there (and according to irc
> we appear on a helpful track there already), not by sprinkling more
> process_pending_softirqs() all over the code.

There could be up to 20 warning messages of unknown length, so I do
think we need some processing of softirqs in the loop.

If you prefer I could do softirq processing only every 4? messages,
but I'm not sure it's worth it. Also there's no indication of the
length of messages, so IMO it's safer to just process softirqs after
each.

Thanks, Roger.

Re: [PATCH] x86/console: process softirqs between warning prints
Posted by Jan Beulich 2 years, 2 months ago
On 17.02.2022 13:07, Roger Pau Monné wrote:
> On Thu, Feb 17, 2022 at 12:54:57PM +0100, Jan Beulich wrote:
>> On 17.02.2022 09:28, Roger Pau Monne wrote:
>>> Process softirqs while printing end of boot warnings. Each warning can
>>> be several lines long, and on slow consoles printing multiple ones
>>> without processing softirqs can result in the watchdog triggering:
>>>
>>> (XEN) [   22.277806] ***************************************************
>>> (XEN) [   22.417802] WARNING: CONSOLE OUTPUT IS SYNCHRONOUS
>>> (XEN) [   22.556029] This option is intended to aid debugging of Xen by ensuring
>>> (XEN) [   22.696802] that all output is synchronously delivered on the serial line.
>>> (XEN) [   22.838024] However it can introduce SIGNIFICANT latencies and affect
>>> (XEN) [   22.978710] timekeeping. It is NOT recommended for production use!
>>> (XEN) [   23.119066] ***************************************************
>>> (XEN) [   23.258865] Booted on L1TF-vulnerable hardware with SMT/Hyperthreading
>>> (XEN) [   23.399560] enabled.  Please assess your configuration and choose an
>>> (XEN) [   23.539925] explicit 'smt=<bool>' setting.  See XSA-273.
>>> (XEN) [   23.678860] ***************************************************
>>> (XEN) [   23.818492] Booted on MLPDS/MFBDS-vulnerable hardware with SMT/Hyperthreading
>>> (XEN) [   23.959811] enabled.  Mitigations will not be fully effective.  Please
>>> (XEN) [   24.100396] choose an explicit smt=<bool> setting.  See XSA-297.
>>> (XEN) [   24.240254] *************************************************(XEN) [   24.247302] Watchdog timer detects that CPU0 is stuck!
>>> (XEN) [   24.386785] ----[ Xen-4.17-unstable  x86_64  debug=y  Tainted:   C    ]----
>>> (XEN) [   24.527874] CPU:    0
>>> (XEN) [   24.662422] RIP:    e008:[<ffff82d04025b84a>] drivers/char/ns16550.c#ns16550_tx_ready+0x3a/0x90
>>>
>>> Fixes: ee3fd57acd ('xen: add warning infrastructure')
>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>> ---
>>>  xen/common/warning.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/xen/common/warning.c b/xen/common/warning.c
>>> index 0269c6715c..e6e1404baf 100644
>>> --- a/xen/common/warning.c
>>> +++ b/xen/common/warning.c
>>> @@ -30,6 +30,7 @@ void __init warning_print(void)
>>>      {
>>>          printk("%s", warnings[i]);
>>>          printk("***************************************************\n");
>>> +        process_pending_softirqs();
>>>      }
>>
>> To be honest, I'm not convinced. This gets us pretty close to needing
>> to process softirqs after _every_ line of output. If a console is this
>> slow, the problem imo needs dealing with there (and according to irc
>> we appear on a helpful track there already), not by sprinkling more
>> process_pending_softirqs() all over the code.
> 
> There could be up to 20 warning messages of unknown length, so I do
> think we need some processing of softirqs in the loop.

Hmm, yes, you have a point there.

> If you prefer I could do softirq processing only every 4? messages,
> but I'm not sure it's worth it. Also there's no indication of the
> length of messages, so IMO it's safer to just process softirqs after
> each.

No, that's indeed not worth it.

Acked-by: Jan Beulich <jbeulich@suse.com>

Jan