Re: [Xen-devel] [PATCH v3 2/2] IOMMU/spinlock: Make the pcidevs_lock a recursive one

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.03.16 at 14:17, <quan.xu@xxxxxxxxx> wrote:
> Signed-off-by: Quan Xu <quan.xu@xxxxxxxxx>
> Acked-by: Kevin Tian <kevin.tian@xxxxxxxxx>

The patch itself looks mostly fine now (see below for the minor left
issues), but the complete lack of a description (which should state
why this change is being done) makes this not ready to go in
anyway.

> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -48,7 +48,7 @@ struct pci_seg {
>      } bus2bridge[MAX_BUSES];
>  };
>  
> -spinlock_t pcidevs_lock = SPIN_LOCK_UNLOCKED;
> +static spinlock_t _pcidevs_lock = SPIN_LOCK_UNLOCKED;

Why the renaming?

> @@ -103,6 +103,26 @@ static int pci_segments_iterate(
>      return rc;
>  }
>  
> +void pcidevs_lock(void)
> +{
> +    spin_lock_recursive(&_pcidevs_lock);
> +}
> +
> +void pcidevs_unlock(void)
> +{
> +    spin_unlock_recursive(&_pcidevs_lock);
> +}
> +
> +int pcidevs_is_locked(void)

bool_t

> +{
> +    return spin_is_locked(&_pcidevs_lock);
> +}
> +
> +int pcidevs_trylock(void)

bool_t

(To avoid another round, please be aware that the underlying
spin lock primitives still [wrongly] use "int", so to be fully correct
you will need to use !! in both return statements, unless you
feel like (in another prereq patch) to adjust those primitives too.

> +{
> +    return spin_trylock_recursive(&_pcidevs_lock);
> +}

I also think that it would be a good idea to place these helpers
and the lock definition next to each other.

Jan


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