Re: [PATCH vhost 2/7] vdpa/mlx5: Split function into locked and unlocked variants

2023-12-01 Thread Eugenio Perez Martin 
On Fri, Dec 1, 2023 at 11:49 AM Dragos Tatulea  wrote:
>
> mlx5_vdpa_destroy_mr contains more logic than _mlx5_vdpa_destroy_mr.
> There is no reason for this to be the case. All the logic can go into
> the unlocked variant.
>
> Using the unlocked version is needed in a follow-up patch. And it also
> makes it more consistent with mlx5_vdpa_create_mr.
>
> Signed-off-by: Dragos Tatulea 

Acked-by: Eugenio Pérez 

> ---
>  drivers/vdpa/mlx5/core/mr.c | 31 ---
>  1 file changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c
> index 2197c46e563a..8c80d9e77935 100644
> --- a/drivers/vdpa/mlx5/core/mr.c
> +++ b/drivers/vdpa/mlx5/core/mr.c
> @@ -498,32 +498,32 @@ static void destroy_user_mr(struct mlx5_vdpa_dev 
> *mvdev, struct mlx5_vdpa_mr *mr
>
>  static void _mlx5_vdpa_destroy_mr(struct mlx5_vdpa_dev *mvdev, struct 
> mlx5_vdpa_mr *mr)
>  {
> +   if (!mr)
> +   return;
> +
> if (mr->user_mr)
> destroy_user_mr(mvdev, mr);
> else
> destroy_dma_mr(mvdev, mr);
>
> +   for (int i = 0; i < MLX5_VDPA_NUM_AS; i++) {
> +   if (mvdev->mr[i] == mr)
> +   mvdev->mr[i] = NULL;
> +   }
> +
> vhost_iotlb_free(mr->iotlb);
> +
> +   kfree(mr);
>  }
>
>  void mlx5_vdpa_destroy_mr(struct mlx5_vdpa_dev *mvdev,
>   struct mlx5_vdpa_mr *mr)
>  {
> -   if (!mr)
> -   return;
> -
> mutex_lock(>mr_mtx);
>
> _mlx5_vdpa_destroy_mr(mvdev, mr);
>
> -   for (int i = 0; i < MLX5_VDPA_NUM_AS; i++) {
> -   if (mvdev->mr[i] == mr)
> -   mvdev->mr[i] = NULL;
> -   }
> -
> mutex_unlock(>mr_mtx);
> -
> -   kfree(mr);
>  }
>
>  void mlx5_vdpa_update_mr(struct mlx5_vdpa_dev *mvdev,
> @@ -535,10 +535,7 @@ void mlx5_vdpa_update_mr(struct mlx5_vdpa_dev *mvdev,
> mutex_lock(>mr_mtx);
>
> mvdev->mr[asid] = new_mr;
> -   if (old_mr) {
> -   _mlx5_vdpa_destroy_mr(mvdev, old_mr);
> -   kfree(old_mr);
> -   }
> +   _mlx5_vdpa_destroy_mr(mvdev, old_mr);
>
> mutex_unlock(>mr_mtx);
>
> @@ -546,8 +543,12 @@ void mlx5_vdpa_update_mr(struct mlx5_vdpa_dev *mvdev,
>
>  void mlx5_vdpa_destroy_mr_resources(struct mlx5_vdpa_dev *mvdev)
>  {
> +   mutex_lock(>mr_mtx);
> +
> for (int i = 0; i < MLX5_VDPA_NUM_AS; i++)
> -   mlx5_vdpa_destroy_mr(mvdev, mvdev->mr[i]);
> +   _mlx5_vdpa_destroy_mr(mvdev, mvdev->mr[i]);
> +
> +   mutex_unlock(>mr_mtx);
>
> prune_iotlb(mvdev->cvq.iotlb);
>  }
> --
> 2.42.0
>

[PATCH vhost 2/7] vdpa/mlx5: Split function into locked and unlocked variants

2023-12-01 Thread Dragos Tatulea 
mlx5_vdpa_destroy_mr contains more logic than _mlx5_vdpa_destroy_mr.
There is no reason for this to be the case. All the logic can go into
the unlocked variant.

Using the unlocked version is needed in a follow-up patch. And it also
makes it more consistent with mlx5_vdpa_create_mr.

Signed-off-by: Dragos Tatulea 
---
 drivers/vdpa/mlx5/core/mr.c | 31 ---
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c
index 2197c46e563a..8c80d9e77935 100644
--- a/drivers/vdpa/mlx5/core/mr.c
+++ b/drivers/vdpa/mlx5/core/mr.c
@@ -498,32 +498,32 @@ static void destroy_user_mr(struct mlx5_vdpa_dev *mvdev, 
struct mlx5_vdpa_mr *mr
 
 static void _mlx5_vdpa_destroy_mr(struct mlx5_vdpa_dev *mvdev, struct 
mlx5_vdpa_mr *mr)
 {
+   if (!mr)
+   return;
+
if (mr->user_mr)
destroy_user_mr(mvdev, mr);
else
destroy_dma_mr(mvdev, mr);
 
+   for (int i = 0; i < MLX5_VDPA_NUM_AS; i++) {
+   if (mvdev->mr[i] == mr)
+   mvdev->mr[i] = NULL;
+   }
+
vhost_iotlb_free(mr->iotlb);
+
+   kfree(mr);
 }
 
 void mlx5_vdpa_destroy_mr(struct mlx5_vdpa_dev *mvdev,
  struct mlx5_vdpa_mr *mr)
 {
-   if (!mr)
-   return;
-
mutex_lock(>mr_mtx);
 
_mlx5_vdpa_destroy_mr(mvdev, mr);
 
-   for (int i = 0; i < MLX5_VDPA_NUM_AS; i++) {
-   if (mvdev->mr[i] == mr)
-   mvdev->mr[i] = NULL;
-   }
-
mutex_unlock(>mr_mtx);
-
-   kfree(mr);
 }
 
 void mlx5_vdpa_update_mr(struct mlx5_vdpa_dev *mvdev,
@@ -535,10 +535,7 @@ void mlx5_vdpa_update_mr(struct mlx5_vdpa_dev *mvdev,
mutex_lock(>mr_mtx);
 
mvdev->mr[asid] = new_mr;
-   if (old_mr) {
-   _mlx5_vdpa_destroy_mr(mvdev, old_mr);
-   kfree(old_mr);
-   }
+   _mlx5_vdpa_destroy_mr(mvdev, old_mr);
 
mutex_unlock(>mr_mtx);
 
@@ -546,8 +543,12 @@ void mlx5_vdpa_update_mr(struct mlx5_vdpa_dev *mvdev,
 
 void mlx5_vdpa_destroy_mr_resources(struct mlx5_vdpa_dev *mvdev)
 {
+   mutex_lock(>mr_mtx);
+
for (int i = 0; i < MLX5_VDPA_NUM_AS; i++)
-   mlx5_vdpa_destroy_mr(mvdev, mvdev->mr[i]);
+   _mlx5_vdpa_destroy_mr(mvdev, mvdev->mr[i]);
+
+   mutex_unlock(>mr_mtx);
 
prune_iotlb(mvdev->cvq.iotlb);
 }
-- 
2.42.0