Re: [Xen-devel] [RFC PATCH 04/10] ARM: vGIC: add struct pending_irq locking

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 4 May 2017, Julien Grall wrote:
> > @@ -545,14 +549,30 @@ static void gic_restore_pending_irqs(struct vcpu *v)
> >              /* No more free LRs: find a lower priority irq to evict */
> >              list_for_each_entry_reverse( p_r, inflight_r, inflight )
> >              {
> > +                if ( p_r->irq < p->irq )
> > +                {
> > +                    spin_lock(&p_r->lock);
> > +                    spin_lock(&p->lock);
> > +                }
> > +                else
> > +                {
> > +                    spin_lock(&p->lock);
> > +                    spin_lock(&p_r->lock);
> > +                }
> 
> Please explain in the commit message and the code why this locking order.

Yes, please add a couple of lines of comment to the code too. However,
the code looks correct.


> >                  if ( p_r->priority == p->priority )
> > +                {
> > +                    spin_unlock(&p->lock);
> > +                    spin_unlock(&p_r->lock);
> >                      goto out;
> > +                }
> >                  if ( test_bit(GIC_IRQ_GUEST_VISIBLE, &p_r->status) &&
> >                       !test_bit(GIC_IRQ_GUEST_ACTIVE, &p_r->status) )
> >                      goto found;
> >              }
> >              /* We didn't find a victim this time, and we won't next
> >               * time, so quit */
> > +            spin_unlock(&p->lock);
> > +            spin_unlock(&p_r->lock);
> >              goto out;
> > 
> >  found:
> > @@ -562,12 +582,18 @@ found:
> >              clear_bit(GIC_IRQ_GUEST_VISIBLE, &p_r->status);
> >              gic_add_to_lr_pending(v, p_r);
> >              inflight_r = &p_r->inflight;
> > +
> > +            spin_unlock(&p_r->lock);
> >          }
> > +        else
> > +            spin_lock(&p->lock);
> > 
> >          gic_set_lr(lr, p, GICH_LR_PENDING);
> >          list_del_init(&p->lr_queue);
> >          set_bit(lr, &this_cpu(lr_mask));
> > 
> > +        spin_unlock(&p->lock);
> > +
> >          /* We can only evict nr_lrs entries */
> >          lrs--;
> >          if ( lrs == 0 )

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel