Re: [PATCH] xen/console: do not drop serial output from the hardware domain

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.06.2022 14:32, Roger Pau Monné wrote:
> On Mon, Jun 13, 2022 at 11:18:49AM +0200, Jan Beulich wrote:
>> On 13.06.2022 11:04, Roger Pau Monné wrote:
>>> On Mon, Jun 13, 2022 at 10:29:43AM +0200, Jan Beulich wrote:
>>>> On 13.06.2022 10:21, Roger Pau Monné wrote:
>>>>> On Mon, Jun 13, 2022 at 09:30:06AM +0200, Jan Beulich wrote:
>>>>>> On 10.06.2022 17:06, Roger Pau Monne wrote:
>>>>>>> Prevent dropping console output from the hardware domain, since it's
>>>>>>> likely important to have all the output if the boot fails without
>>>>>>> having to resort to sync_console (which also affects the output from
>>>>>>> other guests).
>>>>>>>
>>>>>>> Do so by pairing the console_serial_puts() with
>>>>>>> serial_{start,end}_log_everything(), so that no output is dropped.
>>>>>>
>>>>>> While I can see the goal, why would Dom0 output be (effectively) more
>>>>>> important than Xen's own one (which isn't "forced")? And with this
>>>>>> aiming at boot output only, wouldn't you want to stop the overriding
>>>>>> once boot has completed (of which, if I'm not mistaken, we don't
>>>>>> really have any signal coming from Dom0)? And even during boot I'm
>>>>>> not convinced we'd want to let through everything, but perhaps just
>>>>>> Dom0's kernel messages?
>>>>>
>>>>> I normally use sync_console on all the boxes I'm doing dev work, so
>>>>> this request is something that come up internally.
>>>>>
>>>>> Didn't realize Xen output wasn't forced, since we already have rate
>>>>> limiting based on log levels I was assuming that non-ratelimited
>>>>> messages wouldn't be dropped.  But yes, I agree that Xen (non-guest
>>>>> triggered) output shouldn't be rate limited either.
>>>>
>>>> Which would raise the question of why we have log levels for non-guest
>>>> messages.
>>>
>>> Hm, maybe I'm confused, but I don't see a direct relation between log
>>> levels and rate limiting.  If I set log level to WARNING I would
>>> expect to not loose _any_ non-guest log messages with level WARNING or
>>> above.  It's still useful to have log levels for non-guest messages,
>>> since user might want to filter out DEBUG non-guest messages for
>>> example.
>>
>> It was me who was confused, because of the two log-everything variants
>> we have (console and serial). You're right that your change is unrelated
>> to log levels. However, when there are e.g. many warnings or when an
>> admin has lowered the log level, what you (would) do is effectively
>> force sync_console mode transiently (for a subset of messages, but
>> that's secondary, especially because the "forced" output would still
>> be waiting for earlier output to make it out).
> 
> Right, it would have to wait for any previous output on the buffer to
> go out first.  In any case we can guarantee that no more output will
> be added to the buffer while Xen waits for it to be flushed.
> 
> So for the hardware domain it might make sense to wait for the TX
> buffers to be half empty (the current tx_quench logic) by preempting
> the hypercall.  That however could cause issues if guests manage to
> keep filling the buffer while the hardware domain is being preempted.
> 
> Alternatively we could always reserve half of the buffer for the
> hardware domain, and allow it to be preempted while waiting for space
> (since it's guaranteed non hardware domains won't be able to steal the
> allocation from the hardware domain).

Getting complicated it seems. I have to admit that I wonder whether we
wouldn't be better off leaving the current logic as is.

> For Xen it's not trivial to prevent messages from being dropped. At
> least during Xen boot (system_state < SYS_STATE_active) we could also
> active the sync mode and make the spin wait in __serial_putc process
> softirqs.

Yeah, that would seem doable _and_ safe (enough).

Jan