Re: [PATCH v2 1/3] xen/console: cleanup conring management

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.04.2025 20:50, dmkhn@xxxxxxxxx wrote:
> From: Denis Mukhin <dmukhin@xxxxxxxx>
> 
> Move console_locks_busted handling inside conring_puts() to remove
> tasklet code duplication.
> 
> Signed-off-by: Denis Mukhin <dmukhin@xxxxxxxx>
> Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> ---
> Changes v1->v2:
> - added Stefano's R-b
> ---
>  xen/drivers/char/console.c | 29 ++++++++++++++---------------
>  1 file changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index c3150fbdb7..aaa97088aa 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -325,6 +325,17 @@ static void cf_check do_dec_thresh(unsigned char key, 
> bool unused)
>   * ********************************************************
>   */
>  
> +static void cf_check notify_dom0_con_ring(void *unused)
> +{
> +    send_global_virq(VIRQ_CON_RING);
> +}
> +
> +static DECLARE_SOFTIRQ_TASKLET(notify_dom0_con_ring_tasklet,
> +                               notify_dom0_con_ring,
> +                               NULL);
> +
> +static bool console_locks_busted;
> +
>  static void conring_puts(const char *str, size_t len)
>  {
>      ASSERT(rspin_is_locked(&console_lock));
> @@ -334,6 +345,9 @@ static void conring_puts(const char *str, size_t len)
>  
>      if ( conringp - conringc > conring_size )
>          conringc = conringp - conring_size;
> +
> +    if ( !console_locks_busted )
> +        tasklet_schedule(&notify_dom0_con_ring_tasklet);
>  }

As before I'm unconvinced it is a good idea to have tasklet scheduling here.
You also didn't address that v1 comment of mine verbally, iirc.

Jan