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

[dmkhn@xxxxxxxxx]

On Mon, Apr 28, 2025 at 08:50:52AM +0200, Jan Beulich wrote:
> 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.

Sorry, I did not mean to ignore the v1 comment.
I am preparing another iteration, which, I think looks better.

> 
> Jan