All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: dmukhin@xen.org
Cc: andrew.cooper3@citrix.com, anthony.perard@vates.tech,
	julien@xen.org, michal.orzel@amd.com, roger.pau@citrix.com,
	sstabellini@kernel.org, dmukhin@ford.com,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH v5 6/6] xen/console: add conring buffer size alignment setting
Date: Tue, 10 Feb 2026 16:10:42 +0100	[thread overview]
Message-ID: <75df20e0-70f0-454c-9797-e62725f53b63@suse.com> (raw)
In-Reply-To: <20260205013606.3384798-7-dmukhin@ford.com>

On 05.02.2026 02:36, dmukhin@xen.org wrote:
> From: Denis Mukhin <dmukhin@ford.com> 
> 
> Introduce CONFIG_CONRING_ALIGN_PAGE_SIZE to control rounding down of the
> user-defined conring buffer size.

What's wrong with the rounding? The more that, with the original behavior
properly restored in patch 5, it'll be a power-of-2 multiple of PAGE_SIZE
anyway?

> Also, update the logline reporting the final conring buffer size to report
> bytes instead of kilobytes, since the user-defined size may not necessarily
> be kilobyte-alined.

Yet making the number harder to grok.

> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>

Having talked to him, I don't think he meant what you're doing here. All he
apparently meant is to stop using alloc_*heap_pages(), which needlessly
supplies order-aligned memory.

> --- a/xen/drivers/char/Kconfig
> +++ b/xen/drivers/char/Kconfig
> @@ -95,6 +95,13 @@ config SERIAL_TX_BUFSIZE
>  
>  	  Default value is 32768 (32KiB).
>  
> +config CONRING_ALIGN_PAGE_SIZE
> +	bool
> +	default y

IOW "def_bool y". But what's the point of the option when there's no prompt?

> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -470,12 +470,15 @@ void __init console_init_ring(void)
>          return;
>  
>      opt_conring_size = max(opt_conring_size, conring_size);
> -    size = ROUNDDOWN(opt_conring_size, PAGE_SIZE);
> -    if ( size != opt_conring_size )
> +    if ( IS_ENABLED(CONFIG_CONRING_ALIGN_PAGE_SIZE) )
>      {
> -        opt_conring_size = size;
> -        printk(XENLOG_WARNING "Rounding down console ring size to multiple of %lu KiB.\n",
> -               PAGE_SIZE >> 10);
> +        size = ROUNDDOWN(opt_conring_size, PAGE_SIZE);
> +        if ( size != opt_conring_size )
> +        {
> +            opt_conring_size = size;
> +            printk(XENLOG_WARNING "Rounding down console ring size to multiple of %lu KiB.\n",

This line wants splitting at the start of the string literal, and the full
stop wants removing if already it is being touched.

Jan


  reply	other threads:[~2026-02-10 15:10 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-05  1:36 [PATCH v5 0/6] xen/console: configurable conring size dmukhin
2026-02-05  1:36 ` [PATCH v5 1/6] xen/console: group conring code together dmukhin
2026-02-09 16:38   ` Jan Beulich
2026-02-11 19:47     ` dmukhin
2026-02-05  1:36 ` [PATCH v5 2/6] xen/console: make console buffer size configurable dmukhin
2026-02-05  8:43   ` Jan Beulich
2026-02-06  1:52     ` dmukhin
2026-02-05  1:36 ` [PATCH v5 3/6] xen/console: promote conring{,_size} to __ro_after_init dmukhin
2026-02-09 16:40   ` Jan Beulich
2026-02-11 19:47     ` dmukhin
2026-02-05  1:36 ` [PATCH v5 4/6] xen/console: use memcpy() in console_init_ring() dmukhin
2026-02-09 16:56   ` Jan Beulich
2026-02-05  1:36 ` [PATCH v5 5/6] xen/console: update conring memory allocation dmukhin
2026-02-09 17:02   ` Jan Beulich
2026-05-08 21:45     ` dmukhin
2026-02-10 15:08   ` Jan Beulich
2026-02-05  1:36 ` [PATCH v5 6/6] xen/console: add conring buffer size alignment setting dmukhin
2026-02-10 15:10   ` Jan Beulich [this message]
2026-05-08 21:46     ` dmukhin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=75df20e0-70f0-454c-9797-e62725f53b63@suse.com \
    --to=jbeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=anthony.perard@vates.tech \
    --cc=dmukhin@ford.com \
    --cc=dmukhin@xen.org \
    --cc=julien@xen.org \
    --cc=michal.orzel@amd.com \
    --cc=roger.pau@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.