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

Re: [XEN][PATCH v5 08/17] xen/iommu: Move spin_lock from iommu_dt_device_is_assigned to caller


  • To: Vikram Garhwal <vikram.garhwal@xxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Michal Orzel <michal.orzel@xxxxxxx>
  • Date: Mon, 17 Apr 2023 11:14:46 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=lists.xenproject.org smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=FZrJ9heFh4JrBPCgTPG9XVKHRb96aOeDIXYtPn+itCg=; b=ndkicFCyq0sB3pm6wpYp7aqoqJTPPpLe3n/p6a7WHri+uHQBVF28Dmyjccllqzsj6sGGJYU5RmvmJKuRFho+Skdk6ZXWjsJUlJvgJQytQsribxjH16t106YAqCMXoEmtrs0A2TVGZ2a+PvwbxhTcAC4OK1GFblaZRiqZEqG/e4FLVZIo+momlHv2T0M/sifpJ2l0RXXM1Ci/y/I67wVyDF6fLT/nnyvWFemST3EACT1yxd/F8mmKyTaB6V4Cvr331bw5sPshOiYfmJIAWTf3nUlggOxdVbZD4VtCDTSiVmAqyj+ppEubKhRbHU/1w7lCzLa2SQGynLspElXxAsyiFw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=fIzLYJWBGan3eQCeTnX+dk6lz0ch4c0hDh+aourpwyDZNtasE0BctTrwDLfv9/WSvvO76YM0DBv94kP0p+/5nIMmSd5dVeAKII1035kzLPJVqTZLsOLdX4zu463DZf71XjzQCvY2PkGJ+7OK9SQiDwN6mb0uaQllt/mpdEtDJszYhmA48IeSlU7Ol/q9Jqt5hcZB9TitFx4aRGYB6O58qqMM95Ox7qCv7LfVI9q6hGxkt5INsOUZXsaTZRTgU9ReEWs3xY+2RgxmHPDhE3sBOEiE39Sj8EzJu+cyGaVP9sVR9xxr2qusxNXj/q7akquKQkYTZB70kIihAt9q0lYTVQ==
  • Cc: <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>
  • Delivery-date: Mon, 17 Apr 2023 09:15:05 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Hi Vikram,

It looks like you didn't take into account any of my remarks made for v4.
I will repeat them here.

On 11/04/2023 21:16, Vikram Garhwal wrote:
> 
> 
> Rename iommu_dt_device_is_assigned() to iommu_dt_device_is_assigned_lock().
s/lock/locked

> 
> Moving spin_lock to caller was done to prevent the concurrent access to
> iommu_dt_device_is_assigned while doing add/remove/assign/deassign.
> 
> Signed-off-by: Vikram Garhwal <vikram.garhwal@xxxxxxx>
> Reviewed-by: Luca Fancellu <luca.fancellu@xxxxxxx>
> ---
>  xen/drivers/passthrough/device_tree.c | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/device_tree.c 
> b/xen/drivers/passthrough/device_tree.c
> index 1c32d7b50c..bb4cf7784d 100644
> --- a/xen/drivers/passthrough/device_tree.c
> +++ b/xen/drivers/passthrough/device_tree.c
> @@ -83,16 +83,15 @@ fail:
>      return rc;
>  }
> 
> -static bool_t iommu_dt_device_is_assigned(const struct dt_device_node *dev)
> +static bool_t
> +    iommu_dt_device_is_assigned_locked(const struct dt_device_node *dev)
This should not be indented.

>  {
>      bool_t assigned = 0;
> 
>      if ( !dt_device_is_protected(dev) )
>          return 0;
> 
> -    spin_lock(&dtdevs_lock);
>      assigned = !list_empty(&dev->domain_list);
> -    spin_unlock(&dtdevs_lock);
> 
>      return assigned;
>  }
> @@ -213,27 +212,43 @@ int iommu_do_dt_domctl(struct xen_domctl *domctl, 
> struct domain *d,
>          if ( (d && d->is_dying) || domctl->u.assign_device.flags )
>              break;
> 
> +        spin_lock(&dtdevs_lock);
> +
>          ret = dt_find_node_by_gpath(domctl->u.assign_device.u.dt.path,
>                                      domctl->u.assign_device.u.dt.size,
>                                      &dev);
>          if ( ret )
> +        {
> +            spin_unlock(&dtdevs_lock);
> +
Please, do not add blank line between spin_unlock and break. It does not 
improve readability.

>              break;
> +        }
> 
>          ret = xsm_assign_dtdevice(XSM_HOOK, d, dt_node_full_name(dev));
>          if ( ret )
> +        {
> +            spin_unlock(&dtdevs_lock);
> +
same here.

~Michal



 


Rackspace

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