Re: [Xen-devel] [PATCH v2 4/4] The locking order is: first rank lock, then vgic lock. The order is respected everywhere, except for gic_update_one_lr.

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 28 Dec 2016, Julien Grall wrote:
> Hi Stefano,
> 
> On 22/12/16 02:15, Stefano Stabellini wrote:
> > gic_update_one_lr is called with the vgic lock held, but it calls
> > vgic_get_target_vcpu, which tries to obtain the rank lock. This can
> > cause deadlocks.
> > 
> > We already have a version of vgic_get_target_vcpu that doesn't take the
> > rank lock: __vgic_get_target_vcpu.
> > 
> > Solve the lock inversion problem, by not taking the rank lock in
> > gic_update_one_lr (calling __vgic_get_target_vcpu instead of
> > vgic_get_target_vcpu).  This is safe, because vcpu target modifications
> > are protected by the same vgic vcpu lock.
> 
> I maintain what I said on the first version of this patch. All this patch
> could be simplified by moving the call to irq_set_affinity in
> vgic_irq_migrate.
> 
> There are no restriction to do that and no need to have any lock taken but the
> rank lock.
 
All right. I'll submit a patch that does exactly that. It is not
perfect, because it drops interrupts in the problematic scenario, but it
should be a good place to start for a technical discussion.

If it turns out that this is the best approach, we can come back to it.

Cheers,

Stefano

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