| Message ID | 20240821114100.2261167-8-dtatulea@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vdpa/mlx5: Optimze MKEY operations | expand |
On Wed, Aug 21, 2024 at 1:42 PM Dragos Tatulea <dtatulea@nvidia.com> wrote: > > There's currently not a lot of action happening during > the init/destroy of MR resources. But more will be added > in the upcoming patches. If the series doesn't receive new patches, it is just the next patch :). > > Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com> > Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com> > --- > drivers/vdpa/mlx5/core/mlx5_vdpa.h | 2 ++ > drivers/vdpa/mlx5/core/mr.c | 17 +++++++++++++++++ > drivers/vdpa/mlx5/core/resources.c | 3 --- > drivers/vdpa/mlx5/net/mlx5_vnet.c | 10 ++++++++-- > 4 files changed, 27 insertions(+), 5 deletions(-) > > diff --git a/drivers/vdpa/mlx5/core/mlx5_vdpa.h b/drivers/vdpa/mlx5/core/mlx5_vdpa.h > index 89b564cecddf..c3e17bc888e8 100644 > --- a/drivers/vdpa/mlx5/core/mlx5_vdpa.h > +++ b/drivers/vdpa/mlx5/core/mlx5_vdpa.h > @@ -138,6 +138,8 @@ int mlx5_vdpa_create_mkey(struct mlx5_vdpa_dev *mvdev, u32 *mkey, u32 *in, > int mlx5_vdpa_destroy_mkey(struct mlx5_vdpa_dev *mvdev, u32 mkey); > struct mlx5_vdpa_mr *mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, > struct vhost_iotlb *iotlb); > +int mlx5_vdpa_init_mr_resources(struct mlx5_vdpa_dev *mvdev); > +void mlx5_vdpa_destroy_mr_resources(struct mlx5_vdpa_dev *mvdev); > void mlx5_vdpa_clean_mrs(struct mlx5_vdpa_dev *mvdev); > void mlx5_vdpa_get_mr(struct mlx5_vdpa_dev *mvdev, > struct mlx5_vdpa_mr *mr); > diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c > index f20f2a8a701d..ec75f165f832 100644 > --- a/drivers/vdpa/mlx5/core/mr.c > +++ b/drivers/vdpa/mlx5/core/mr.c > @@ -843,3 +843,20 @@ int mlx5_vdpa_reset_mr(struct mlx5_vdpa_dev *mvdev, unsigned int asid) > > return 0; > } > + > +int mlx5_vdpa_init_mr_resources(struct mlx5_vdpa_dev *mvdev) > +{ > + struct mlx5_vdpa_mr_resources *mres = &mvdev->mres; > + > + INIT_LIST_HEAD(&mres->mr_list_head); > + mutex_init(&mres->lock); > + > + return 0; I'd leave this function return void here and remove the caller error control path. > +} > + > +void mlx5_vdpa_destroy_mr_resources(struct mlx5_vdpa_dev *mvdev) > +{ > + struct mlx5_vdpa_mr_resources *mres = &mvdev->mres; > + > + mutex_destroy(&mres->lock); > +} > diff --git a/drivers/vdpa/mlx5/core/resources.c b/drivers/vdpa/mlx5/core/resources.c > index fe2ca3458f6c..aeae31d0cefa 100644 > --- a/drivers/vdpa/mlx5/core/resources.c > +++ b/drivers/vdpa/mlx5/core/resources.c > @@ -256,7 +256,6 @@ int mlx5_vdpa_alloc_resources(struct mlx5_vdpa_dev *mvdev) > mlx5_vdpa_warn(mvdev, "resources already allocated\n"); > return -EINVAL; > } > - mutex_init(&mvdev->mres.lock); > res->uar = mlx5_get_uars_page(mdev); > if (IS_ERR(res->uar)) { > err = PTR_ERR(res->uar); > @@ -301,7 +300,6 @@ int mlx5_vdpa_alloc_resources(struct mlx5_vdpa_dev *mvdev) > err_uctx: > mlx5_put_uars_page(mdev, res->uar); > err_uars: > - mutex_destroy(&mvdev->mres.lock); Maybe it is just me, but this patch is also moving the lock lifetime from mlx5_vdpa_alloc_resources / mlx5_vdpa_free_resources to mlx5_vdpa_dev_add / mlx5_vdpa_free. I guess it has a justification we can either clarify in the patch message or split in its own patch. > return err; > } > > @@ -318,7 +316,6 @@ void mlx5_vdpa_free_resources(struct mlx5_vdpa_dev *mvdev) > dealloc_pd(mvdev, res->pdn, res->uid); > destroy_uctx(mvdev, res->uid); > mlx5_put_uars_page(mvdev->mdev, res->uar); > - mutex_destroy(&mvdev->mres.lock); > res->valid = false; > } > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c > index 8a51c492a62a..1cadcb05a5c7 100644 > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c > @@ -3434,6 +3434,7 @@ static void mlx5_vdpa_free(struct vdpa_device *vdev) > > free_fixed_resources(ndev); > mlx5_vdpa_clean_mrs(mvdev); > + mlx5_vdpa_destroy_mr_resources(&ndev->mvdev); > if (!is_zero_ether_addr(ndev->config.mac)) { > pfmdev = pci_get_drvdata(pci_physfn(mvdev->mdev->pdev)); > mlx5_mpfs_del_mac(pfmdev, ndev->config.mac); > @@ -3962,12 +3963,15 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name, > if (err) > goto err_mpfs; > > - INIT_LIST_HEAD(&mvdev->mres.mr_list_head); > + err = mlx5_vdpa_init_mr_resources(mvdev); > + if (err) > + goto err_res; > + Extra newline here. > > if (MLX5_CAP_GEN(mvdev->mdev, umem_uid_0)) { > err = mlx5_vdpa_create_dma_mr(mvdev); > if (err) > - goto err_res; > + goto err_mr_res; > } > > err = alloc_fixed_resources(ndev); > @@ -4009,6 +4013,8 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name, > free_fixed_resources(ndev); > err_mr: > mlx5_vdpa_clean_mrs(mvdev); > +err_mr_res: > + mlx5_vdpa_destroy_mr_resources(mvdev); > err_res: > mlx5_vdpa_free_resources(&ndev->mvdev); > err_mpfs: > -- > 2.45.1 >
On 29.08.24 16:37, Eugenio Perez Martin wrote: > On Wed, Aug 21, 2024 at 1:42 PM Dragos Tatulea <dtatulea@nvidia.com> wrote: >> >> There's currently not a lot of action happening during >> the init/destroy of MR resources. But more will be added >> in the upcoming patches. > > If the series doesn't receive new patches, it is just the next patch :). > >> >> Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com> >> Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com> >> --- >> drivers/vdpa/mlx5/core/mlx5_vdpa.h | 2 ++ >> drivers/vdpa/mlx5/core/mr.c | 17 +++++++++++++++++ >> drivers/vdpa/mlx5/core/resources.c | 3 --- >> drivers/vdpa/mlx5/net/mlx5_vnet.c | 10 ++++++++-- >> 4 files changed, 27 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/vdpa/mlx5/core/mlx5_vdpa.h b/drivers/vdpa/mlx5/core/mlx5_vdpa.h >> index 89b564cecddf..c3e17bc888e8 100644 >> --- a/drivers/vdpa/mlx5/core/mlx5_vdpa.h >> +++ b/drivers/vdpa/mlx5/core/mlx5_vdpa.h >> @@ -138,6 +138,8 @@ int mlx5_vdpa_create_mkey(struct mlx5_vdpa_dev *mvdev, u32 *mkey, u32 *in, >> int mlx5_vdpa_destroy_mkey(struct mlx5_vdpa_dev *mvdev, u32 mkey); >> struct mlx5_vdpa_mr *mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, >> struct vhost_iotlb *iotlb); >> +int mlx5_vdpa_init_mr_resources(struct mlx5_vdpa_dev *mvdev); >> +void mlx5_vdpa_destroy_mr_resources(struct mlx5_vdpa_dev *mvdev); >> void mlx5_vdpa_clean_mrs(struct mlx5_vdpa_dev *mvdev); >> void mlx5_vdpa_get_mr(struct mlx5_vdpa_dev *mvdev, >> struct mlx5_vdpa_mr *mr); >> diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c >> index f20f2a8a701d..ec75f165f832 100644 >> --- a/drivers/vdpa/mlx5/core/mr.c >> +++ b/drivers/vdpa/mlx5/core/mr.c >> @@ -843,3 +843,20 @@ int mlx5_vdpa_reset_mr(struct mlx5_vdpa_dev *mvdev, unsigned int asid) >> >> return 0; >> } >> + >> +int mlx5_vdpa_init_mr_resources(struct mlx5_vdpa_dev *mvdev) >> +{ >> + struct mlx5_vdpa_mr_resources *mres = &mvdev->mres; >> + >> + INIT_LIST_HEAD(&mres->mr_list_head); >> + mutex_init(&mres->lock); >> + >> + return 0; > > I'd leave this function return void here and remove the caller error > control path. > It is like this because the next patch adds an error path. >> +} >> + >> +void mlx5_vdpa_destroy_mr_resources(struct mlx5_vdpa_dev *mvdev) >> +{ >> + struct mlx5_vdpa_mr_resources *mres = &mvdev->mres; >> + >> + mutex_destroy(&mres->lock); >> +} >> diff --git a/drivers/vdpa/mlx5/core/resources.c b/drivers/vdpa/mlx5/core/resources.c >> index fe2ca3458f6c..aeae31d0cefa 100644 >> --- a/drivers/vdpa/mlx5/core/resources.c >> +++ b/drivers/vdpa/mlx5/core/resources.c >> @@ -256,7 +256,6 @@ int mlx5_vdpa_alloc_resources(struct mlx5_vdpa_dev *mvdev) >> mlx5_vdpa_warn(mvdev, "resources already allocated\n"); >> return -EINVAL; >> } >> - mutex_init(&mvdev->mres.lock); >> res->uar = mlx5_get_uars_page(mdev); >> if (IS_ERR(res->uar)) { >> err = PTR_ERR(res->uar); >> @@ -301,7 +300,6 @@ int mlx5_vdpa_alloc_resources(struct mlx5_vdpa_dev *mvdev) >> err_uctx: >> mlx5_put_uars_page(mdev, res->uar); >> err_uars: >> - mutex_destroy(&mvdev->mres.lock); > > Maybe it is just me, but this patch is also moving the lock lifetime > from mlx5_vdpa_alloc_resources / mlx5_vdpa_free_resources to > mlx5_vdpa_dev_add / mlx5_vdpa_free. I guess it has a justification we > can either clarify in the patch message or split in its own patch. > Good point. Will do. >> return err; >> } >> >> @@ -318,7 +316,6 @@ void mlx5_vdpa_free_resources(struct mlx5_vdpa_dev *mvdev) >> dealloc_pd(mvdev, res->pdn, res->uid); >> destroy_uctx(mvdev, res->uid); >> mlx5_put_uars_page(mvdev->mdev, res->uar); >> - mutex_destroy(&mvdev->mres.lock); >> res->valid = false; >> } >> >> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c >> index 8a51c492a62a..1cadcb05a5c7 100644 >> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c >> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c >> @@ -3434,6 +3434,7 @@ static void mlx5_vdpa_free(struct vdpa_device *vdev) >> >> free_fixed_resources(ndev); >> mlx5_vdpa_clean_mrs(mvdev); >> + mlx5_vdpa_destroy_mr_resources(&ndev->mvdev); >> if (!is_zero_ether_addr(ndev->config.mac)) { >> pfmdev = pci_get_drvdata(pci_physfn(mvdev->mdev->pdev)); >> mlx5_mpfs_del_mac(pfmdev, ndev->config.mac); >> @@ -3962,12 +3963,15 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name, >> if (err) >> goto err_mpfs; >> >> - INIT_LIST_HEAD(&mvdev->mres.mr_list_head); >> + err = mlx5_vdpa_init_mr_resources(mvdev); >> + if (err) >> + goto err_res; >> + > > Extra newline here. > Seems like I'm keen on these extra newlines. Will fix. Thanks, Dragos >> >> if (MLX5_CAP_GEN(mvdev->mdev, umem_uid_0)) { >> err = mlx5_vdpa_create_dma_mr(mvdev); >> if (err) >> - goto err_res; >> + goto err_mr_res; >> } >> >> err = alloc_fixed_resources(ndev); >> @@ -4009,6 +4013,8 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name, >> free_fixed_resources(ndev); >> err_mr: >> mlx5_vdpa_clean_mrs(mvdev); >> +err_mr_res: >> + mlx5_vdpa_destroy_mr_resources(mvdev); >> err_res: >> mlx5_vdpa_free_resources(&ndev->mvdev); >> err_mpfs: >> -- >> 2.45.1 >> >
diff --git a/drivers/vdpa/mlx5/core/mlx5_vdpa.h b/drivers/vdpa/mlx5/core/mlx5_vdpa.h index 89b564cecddf..c3e17bc888e8 100644 --- a/drivers/vdpa/mlx5/core/mlx5_vdpa.h +++ b/drivers/vdpa/mlx5/core/mlx5_vdpa.h @@ -138,6 +138,8 @@ int mlx5_vdpa_create_mkey(struct mlx5_vdpa_dev *mvdev, u32 *mkey, u32 *in, int mlx5_vdpa_destroy_mkey(struct mlx5_vdpa_dev *mvdev, u32 mkey); struct mlx5_vdpa_mr *mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb); +int mlx5_vdpa_init_mr_resources(struct mlx5_vdpa_dev *mvdev); +void mlx5_vdpa_destroy_mr_resources(struct mlx5_vdpa_dev *mvdev); void mlx5_vdpa_clean_mrs(struct mlx5_vdpa_dev *mvdev); void mlx5_vdpa_get_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_mr *mr); diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c index f20f2a8a701d..ec75f165f832 100644 --- a/drivers/vdpa/mlx5/core/mr.c +++ b/drivers/vdpa/mlx5/core/mr.c @@ -843,3 +843,20 @@ int mlx5_vdpa_reset_mr(struct mlx5_vdpa_dev *mvdev, unsigned int asid) return 0; } + +int mlx5_vdpa_init_mr_resources(struct mlx5_vdpa_dev *mvdev) +{ + struct mlx5_vdpa_mr_resources *mres = &mvdev->mres; + + INIT_LIST_HEAD(&mres->mr_list_head); + mutex_init(&mres->lock); + + return 0; +} + +void mlx5_vdpa_destroy_mr_resources(struct mlx5_vdpa_dev *mvdev) +{ + struct mlx5_vdpa_mr_resources *mres = &mvdev->mres; + + mutex_destroy(&mres->lock); +} diff --git a/drivers/vdpa/mlx5/core/resources.c b/drivers/vdpa/mlx5/core/resources.c index fe2ca3458f6c..aeae31d0cefa 100644 --- a/drivers/vdpa/mlx5/core/resources.c +++ b/drivers/vdpa/mlx5/core/resources.c @@ -256,7 +256,6 @@ int mlx5_vdpa_alloc_resources(struct mlx5_vdpa_dev *mvdev) mlx5_vdpa_warn(mvdev, "resources already allocated\n"); return -EINVAL; } - mutex_init(&mvdev->mres.lock); res->uar = mlx5_get_uars_page(mdev); if (IS_ERR(res->uar)) { err = PTR_ERR(res->uar); @@ -301,7 +300,6 @@ int mlx5_vdpa_alloc_resources(struct mlx5_vdpa_dev *mvdev) err_uctx: mlx5_put_uars_page(mdev, res->uar); err_uars: - mutex_destroy(&mvdev->mres.lock); return err; } @@ -318,7 +316,6 @@ void mlx5_vdpa_free_resources(struct mlx5_vdpa_dev *mvdev) dealloc_pd(mvdev, res->pdn, res->uid); destroy_uctx(mvdev, res->uid); mlx5_put_uars_page(mvdev->mdev, res->uar); - mutex_destroy(&mvdev->mres.lock); res->valid = false; } diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c index 8a51c492a62a..1cadcb05a5c7 100644 --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c @@ -3434,6 +3434,7 @@ static void mlx5_vdpa_free(struct vdpa_device *vdev) free_fixed_resources(ndev); mlx5_vdpa_clean_mrs(mvdev); + mlx5_vdpa_destroy_mr_resources(&ndev->mvdev); if (!is_zero_ether_addr(ndev->config.mac)) { pfmdev = pci_get_drvdata(pci_physfn(mvdev->mdev->pdev)); mlx5_mpfs_del_mac(pfmdev, ndev->config.mac); @@ -3962,12 +3963,15 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name, if (err) goto err_mpfs; - INIT_LIST_HEAD(&mvdev->mres.mr_list_head); + err = mlx5_vdpa_init_mr_resources(mvdev); + if (err) + goto err_res; + if (MLX5_CAP_GEN(mvdev->mdev, umem_uid_0)) { err = mlx5_vdpa_create_dma_mr(mvdev); if (err) - goto err_res; + goto err_mr_res; } err = alloc_fixed_resources(ndev); @@ -4009,6 +4013,8 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name, free_fixed_resources(ndev); err_mr: mlx5_vdpa_clean_mrs(mvdev); +err_mr_res: + mlx5_vdpa_destroy_mr_resources(mvdev); err_res: mlx5_vdpa_free_resources(&ndev->mvdev); err_mpfs: