[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] missing vgic_unlock_rank in gic_remove_irq_from_guest



On Fri, 9 Dec 2016, Artem Mygaiev wrote:
> Hi Stefano
> 
> On 09.12.16 02:59, Stefano Stabellini wrote:
> > Add missing vgic_unlock_rank on the error path in
> > gic_remove_irq_from_guest.
> >
> > CID: 1381843
> >
> > Signed-off-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> >
> > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> > index 63c744a..a5348f2 100644
> > --- a/xen/arch/arm/gic.c
> > +++ b/xen/arch/arm/gic.c
> > @@ -205,7 +205,10 @@ int gic_remove_irq_from_guest(struct domain *d, 
> > unsigned int virq,
> >           */
> >          if ( test_bit(_IRQ_INPROGRESS, &desc->status) ||
> >               !test_bit(_IRQ_DISABLED, &desc->status) )
> > +        {
> > +            vgic_unlock_rank(v_target, rank, flags);
> >              return -EBUSY;
> > +        }
> >      }
> >  
> >      clear_bit(_IRQ_GUEST, &desc->status);
> >
> >
> would it be better to do it in the same way it is done in 
> gic_route_irq_to_guest() just above the patched function?

Yes, that is the preferred way of handling error paths, especially when
there are multiple. In this case, there is just one, so it doesn't make
a difference.


> Something like:
> 
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 63c744a..a9bf5d9 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -181,6 +181,7 @@ int gic_remove_irq_from_guest(struct domain *d, unsigned 
> int virq,
>      struct vgic_irq_rank *rank = vgic_rank_irq(v_target, virq);
>      struct pending_irq *p = irq_to_pending(v_target, virq);
>      unsigned long flags;
> +    int res = -EBUSY;
>  
>      ASSERT(spin_is_locked(&desc->lock));
>      ASSERT(test_bit(_IRQ_GUEST, &desc->status));
> @@ -205,17 +206,19 @@ int gic_remove_irq_from_guest(struct domain *d, 
> unsigned int virq,
>           */
>          if ( test_bit(_IRQ_INPROGRESS, &desc->status) ||
>               !test_bit(_IRQ_DISABLED, &desc->status) )
> -            return -EBUSY;
> +            goto out;
>      }
>  
>      clear_bit(_IRQ_GUEST, &desc->status);
>      desc->handler = &no_irq_type;
>  
>      p->desc = NULL;
> +    res = 0;
>  
> +out:
>      vgic_unlock_rank(v_target, rank, flags);
>  
> -    return 0;
> +    return res;
>  }
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> https://lists.xen.org/xen-devel
> 

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.