Re: [PATCH v5 04/13] xen/spinlock: add rspin_[un]lock_irq[save|restore]()

[Jürgen Groß <jgross@xxxxxxxx>]

On 18.03.24 16:59, Jan Beulich wrote:
> On 18.03.2024 16:55, Jürgen Groß wrote:
> > On 18.03.24 15:43, Jan Beulich wrote:
> > > On 14.03.2024 08:20, Juergen Gross wrote:
> > > > Instead of special casing rspin_lock_irqsave() and
> > > > rspin_unlock_irqrestore() for the console lock, add those functions
> > > > to spinlock handling and use them where needed.
> > > >
> > > > Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> > > Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> > > with two remarks:
> > >
> > > > --- a/xen/common/spinlock.c
> > > > +++ b/xen/common/spinlock.c
> > > > @@ -475,15 +475,31 @@ void _rspin_lock(rspinlock_t *lock)
> > > >        lock->recurse_cnt++;
> > > >    }
> > > >    
> > > > +unsigned long _rspin_lock_irqsave(rspinlock_t *lock)
> > > > +{
> > > > +    unsigned long flags;
> > > > +
> > > > +    local_irq_save(flags);
> > > > +    _rspin_lock(lock);
> > > > +
> > > > +    return flags;
> > > > +}
> > > > +
> > > >    void _rspin_unlock(rspinlock_t *lock)
> > > >    {
> > > >        if ( likely(--lock->recurse_cnt == 0) )
> > > >        {
> > > >            lock->recurse_cpu = SPINLOCK_NO_CPU;
> > > > -        spin_unlock(lock);
> > > > +        _spin_unlock(lock);
> > > This looks like an unrelated change. I think I can guess the purpose, but
> > > it would be nice if such along-the-way changes could be mentioned in the
> > > description.
> > I think it would be better to move that change to patch 3.
> Hmm, it would be a secondary change there, too. I was actually meaning to
> commit patches 2-5, but if things want moving around I guess I better
> wait with doing so?
Hmm, maybe just drop this hunk and let patch 7 handle it?


Juergen