Re: [PATCH] xen/console: make console buffer size configurable

[Jan Beulich <jbeulich@suse.com>]

On 25.02.2025 03:45, Denis Mukhin wrote:
> On Monday, February 24th, 2025 at 2:44 AM, Jan Beulich <jbeulich@suse.com> wrote:
> 
>>
>>
>> On 21.02.2025 21:52, Denis Mukhin wrote:
>>
>>> On Tuesday, February 18th, 2025 at 8:05 AM, Jan Beulich jbeulich@suse.com wrote:
>>>
>>>> On 12.02.2025 23:31, dmkhn@proton.me wrote:
>>>>
>>>>> --- a/xen/drivers/char/Kconfig
>>>>> +++ b/xen/drivers/char/Kconfig
>>>>> @@ -96,6 +96,18 @@ config SERIAL_TX_BUFSIZE
>>>>>
>>>>> Default value is 32768 (32KiB).
>>>>>
>>>>> +config CONRING_SIZE
>>>>> + int "Console buffer size"
>>>>> + default 32768
>>>>> + range 16384 134217728
>>>>> + help
>>>>> + Select the boot console buffer size (in bytes).
>>>>
>>>> Why in bytes when ...
>>>>
>>>>> + Note, the value provided will be rounded down to the nearest power of 2.
>>>>
>>>> ... this rounding is done anyway? Why have people type in complicated numbers?
>>>> A granularity of 1k would already be an improvement; yet better would be if
>>>> this was a power-of-two value altogether.
>>>
>>> My understanding that the semantics of new CONFIG_CONRING_SIZE build-time setting
>>> should be consistent with existing boot-time conring_size= behavior (string value
>>> converted to number of bytes).
>>>
>>> I can update both to round up to 1k boundary.
>>>
>>> I also agree that having power of 2s for both (e.g. similar to Linux'es CONFIG_LOG_BUF_SHIFT)
>>> will be the simplest (implementation) and non-ambigous.
>>> I had it done earlier:
>>> https://lore.kernel.org/xen-devel/20241205-vuart-ns8250-v1-26-e9aa923127eb@ford.com/
>>
>>
>> I'd prefer the power-of-2 approach, yet I could live with the Kb-based one as
>> was suggested by Roger.
> 
> Just to double check: I think it makes sense to switch both build-time and run-time
> settings to use the same size calculation algorithm (e.g. Kb-based) to avoid
> confusion during building hypervisor configuration.
> 
> Will that be OK to do such change?

No, you can't change existing command line options like this, at least not
without a very good reason. You'd break existing uses. Plus there is

size_param("conring_size", opt_conring_size);

already anyway, so the command line option can be used with Kb and other
granularities without any adjustments to the code.

>>> Also, since there's a build-time configuration parameter along with the boot-time
>>> configuration, perhaps it makes sense to retire boot-time setting in favor of
>>> build-time setting?
>>
>>
>> Why would that be? Build-time settings can only ever be defaults. We don't
>> know what people need in their configurations.
> 
> I was thinking about few reasons.
> In embedded setup run-time settings are unlikely to change, it is mostly
> built-time configuration.
> On a server setup, bumping the size of console buffer morelikely means some
> debugging, which to me means new xen binary can be re-generated and re-deployed.

That depends on who's doing the debugging and who's giving the instructions.
A developer telling a customer to increase the buffer size is certainly
easier when it simply means adding/changing a command line option, for
example.

Jan