| Message ID | 20210216055022.25248-2-elic@nvidia.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | vdpa/mlx5: Extract correct pointer from driver data | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Tue, Feb 16, 2021 at 07:50:22AM +0200, Eli Cohen wrote: > struct mlx5_vdpa_net pointer was stored in drvdata. Extract it as well > in mlx5v_remove(). > > Fixes: 74c9729dd892 ("vdpa/mlx5: Connect mlx5_vdpa to auxiliary bus") > Signed-off-by: Eli Cohen <elic@nvidia.com> > --- > drivers/vdpa/mlx5/net/mlx5_vnet.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c > index 6b0a42183622..4103d3b64a2a 100644 > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c > @@ -2036,9 +2036,9 @@ static int mlx5v_probe(struct auxiliary_device *adev, > > static void mlx5v_remove(struct auxiliary_device *adev) > { > - struct mlx5_vdpa_dev *mvdev = dev_get_drvdata(&adev->dev); > + struct mlx5_vdpa_net *ndev = dev_get_drvdata(&adev->dev); > > - vdpa_unregister_device(&mvdev->vdev); > + vdpa_unregister_device(&ndev->mvdev.vdev); > } IMHO, The more correct solution is to fix dev_set_drvdata() call, because we are regustering/unregistering/allocating "struct mlx5_vdpa_dev". diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c index 88dde3455bfd..079b8fe669af 100644 --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c @@ -1995,7 +1995,7 @@ static int mlx5v_probe(struct auxiliary_device *adev, if (err) goto err_reg; - dev_set_drvdata(&adev->dev, ndev); + dev_set_drvdata(&adev->dev, mvdev); return 0; err_reg: > > static const struct auxiliary_device_id mlx5v_id_table[] = { > -- > 2.29.2 >
On Tue, Feb 16, 2021 at 08:35:51AM +0200, Leon Romanovsky wrote: > On Tue, Feb 16, 2021 at 07:50:22AM +0200, Eli Cohen wrote: > > struct mlx5_vdpa_net pointer was stored in drvdata. Extract it as well > > in mlx5v_remove(). > > > > Fixes: 74c9729dd892 ("vdpa/mlx5: Connect mlx5_vdpa to auxiliary bus") > > Signed-off-by: Eli Cohen <elic@nvidia.com> > > --- > > drivers/vdpa/mlx5/net/mlx5_vnet.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c > > index 6b0a42183622..4103d3b64a2a 100644 > > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c > > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c > > @@ -2036,9 +2036,9 @@ static int mlx5v_probe(struct auxiliary_device *adev, > > > > static void mlx5v_remove(struct auxiliary_device *adev) > > { > > - struct mlx5_vdpa_dev *mvdev = dev_get_drvdata(&adev->dev); > > + struct mlx5_vdpa_net *ndev = dev_get_drvdata(&adev->dev); > > > > - vdpa_unregister_device(&mvdev->vdev); > > + vdpa_unregister_device(&ndev->mvdev.vdev); > > } > > IMHO, The more correct solution is to fix dev_set_drvdata() call, > because we are regustering/unregistering/allocating "struct mlx5_vdpa_dev". > We're allocating "struct mlx5_vdpa_net". "struct mlx5_vdpa_dev" is just a member field of "struct mlx5_vdpa_net". > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c > index 88dde3455bfd..079b8fe669af 100644 > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c > @@ -1995,7 +1995,7 @@ static int mlx5v_probe(struct auxiliary_device *adev, > if (err) > goto err_reg; > > - dev_set_drvdata(&adev->dev, ndev); > + dev_set_drvdata(&adev->dev, mvdev); > return 0; > > err_reg: > > > > > static const struct auxiliary_device_id mlx5v_id_table[] = { > > > -- > > 2.29.2 > >
On Tue, Feb 16, 2021 at 08:42:26AM +0200, Eli Cohen wrote: > On Tue, Feb 16, 2021 at 08:35:51AM +0200, Leon Romanovsky wrote: > > On Tue, Feb 16, 2021 at 07:50:22AM +0200, Eli Cohen wrote: > > > struct mlx5_vdpa_net pointer was stored in drvdata. Extract it as well > > > in mlx5v_remove(). > > > > > > Fixes: 74c9729dd892 ("vdpa/mlx5: Connect mlx5_vdpa to auxiliary bus") > > > Signed-off-by: Eli Cohen <elic@nvidia.com> > > > --- > > > drivers/vdpa/mlx5/net/mlx5_vnet.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c > > > index 6b0a42183622..4103d3b64a2a 100644 > > > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c > > > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c > > > @@ -2036,9 +2036,9 @@ static int mlx5v_probe(struct auxiliary_device *adev, > > > > > > static void mlx5v_remove(struct auxiliary_device *adev) > > > { > > > - struct mlx5_vdpa_dev *mvdev = dev_get_drvdata(&adev->dev); > > > + struct mlx5_vdpa_net *ndev = dev_get_drvdata(&adev->dev); > > > > > > - vdpa_unregister_device(&mvdev->vdev); > > > + vdpa_unregister_device(&ndev->mvdev.vdev); > > > } > > > > IMHO, The more correct solution is to fix dev_set_drvdata() call, > > because we are regustering/unregistering/allocating "struct mlx5_vdpa_dev". > > > > We're allocating "struct mlx5_vdpa_net". "struct mlx5_vdpa_dev" is just > a member field of "struct mlx5_vdpa_net". I referred to these lines in the mlx5v_probe(): 1986 err = mlx5_vdpa_alloc_resources(&ndev->mvdev); 1987 if (err) 1988 goto err_mtu; 1989 1990 err = alloc_resources(ndev); 1991 if (err) 1992 goto err_res; 1993 1994 err = vdpa_register_device(&mvdev->vdev); So mlx5v_remove() is better to be symmetrical. Thanks > > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c > > index 88dde3455bfd..079b8fe669af 100644 > > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c > > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c > > @@ -1995,7 +1995,7 @@ static int mlx5v_probe(struct auxiliary_device *adev, > > if (err) > > goto err_reg; > > > > - dev_set_drvdata(&adev->dev, ndev); > > + dev_set_drvdata(&adev->dev, mvdev); > > return 0; > > > > err_reg: > > > > > > > > static const struct auxiliary_device_id mlx5v_id_table[] = { > > > > > -- > > > 2.29.2 > > >
On Tue, Feb 16, 2021 at 09:37:34AM +0200, Leon Romanovsky wrote: > On Tue, Feb 16, 2021 at 08:42:26AM +0200, Eli Cohen wrote: > > On Tue, Feb 16, 2021 at 08:35:51AM +0200, Leon Romanovsky wrote: > > > On Tue, Feb 16, 2021 at 07:50:22AM +0200, Eli Cohen wrote: > > > > struct mlx5_vdpa_net pointer was stored in drvdata. Extract it as well > > > > in mlx5v_remove(). > > > > > > > > Fixes: 74c9729dd892 ("vdpa/mlx5: Connect mlx5_vdpa to auxiliary bus") > > > > Signed-off-by: Eli Cohen <elic@nvidia.com> > > > > --- > > > > drivers/vdpa/mlx5/net/mlx5_vnet.c | 4 ++-- > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c > > > > index 6b0a42183622..4103d3b64a2a 100644 > > > > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c > > > > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c > > > > @@ -2036,9 +2036,9 @@ static int mlx5v_probe(struct auxiliary_device *adev, > > > > > > > > static void mlx5v_remove(struct auxiliary_device *adev) > > > > { > > > > - struct mlx5_vdpa_dev *mvdev = dev_get_drvdata(&adev->dev); > > > > + struct mlx5_vdpa_net *ndev = dev_get_drvdata(&adev->dev); > > > > > > > > - vdpa_unregister_device(&mvdev->vdev); > > > > + vdpa_unregister_device(&ndev->mvdev.vdev); > > > > } > > > > > > IMHO, The more correct solution is to fix dev_set_drvdata() call, > > > because we are regustering/unregistering/allocating "struct mlx5_vdpa_dev". > > > > > > > We're allocating "struct mlx5_vdpa_net". "struct mlx5_vdpa_dev" is just > > a member field of "struct mlx5_vdpa_net". > > I referred to these lines in the mlx5v_probe(): > 1986 err = mlx5_vdpa_alloc_resources(&ndev->mvdev); > 1987 if (err) > 1988 goto err_mtu; > 1989 > 1990 err = alloc_resources(ndev); > 1991 if (err) > 1992 goto err_res; > 1993 > 1994 err = vdpa_register_device(&mvdev->vdev); > > So mlx5v_remove() is better to be symmetrical. > It's "struct mlx5_vdpa_net" that is being allocated here so it makes sense to set this pointer as the the driver data. > Thanks > > > > > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c > > > index 88dde3455bfd..079b8fe669af 100644 > > > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c > > > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c > > > @@ -1995,7 +1995,7 @@ static int mlx5v_probe(struct auxiliary_device *adev, > > > if (err) > > > goto err_reg; > > > > > > - dev_set_drvdata(&adev->dev, ndev); > > > + dev_set_drvdata(&adev->dev, mvdev); > > > return 0; > > > > > > err_reg: > > > > > > > > > > > static const struct auxiliary_device_id mlx5v_id_table[] = { > > > > > > > -- > > > > 2.29.2 > > > >
On Tue, Feb 16, 2021 at 02:45:40PM +0200, Eli Cohen wrote: > On Tue, Feb 16, 2021 at 09:37:34AM +0200, Leon Romanovsky wrote: > > On Tue, Feb 16, 2021 at 08:42:26AM +0200, Eli Cohen wrote: > > > On Tue, Feb 16, 2021 at 08:35:51AM +0200, Leon Romanovsky wrote: > > > > On Tue, Feb 16, 2021 at 07:50:22AM +0200, Eli Cohen wrote: > > > > > struct mlx5_vdpa_net pointer was stored in drvdata. Extract it as well > > > > > in mlx5v_remove(). > > > > > > > > > > Fixes: 74c9729dd892 ("vdpa/mlx5: Connect mlx5_vdpa to auxiliary bus") > > > > > Signed-off-by: Eli Cohen <elic@nvidia.com> > > > > > --- > > > > > drivers/vdpa/mlx5/net/mlx5_vnet.c | 4 ++-- > > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c > > > > > index 6b0a42183622..4103d3b64a2a 100644 > > > > > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c > > > > > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c > > > > > @@ -2036,9 +2036,9 @@ static int mlx5v_probe(struct auxiliary_device *adev, > > > > > > > > > > static void mlx5v_remove(struct auxiliary_device *adev) > > > > > { > > > > > - struct mlx5_vdpa_dev *mvdev = dev_get_drvdata(&adev->dev); > > > > > + struct mlx5_vdpa_net *ndev = dev_get_drvdata(&adev->dev); > > > > > > > > > > - vdpa_unregister_device(&mvdev->vdev); > > > > > + vdpa_unregister_device(&ndev->mvdev.vdev); > > > > > } > > > > > > > > IMHO, The more correct solution is to fix dev_set_drvdata() call, > > > > because we are regustering/unregistering/allocating "struct mlx5_vdpa_dev". > > > > > > > > > > We're allocating "struct mlx5_vdpa_net". "struct mlx5_vdpa_dev" is just > > > a member field of "struct mlx5_vdpa_net". > > > > I referred to these lines in the mlx5v_probe(): > > 1986 err = mlx5_vdpa_alloc_resources(&ndev->mvdev); > > 1987 if (err) > > 1988 goto err_mtu; > > 1989 > > 1990 err = alloc_resources(ndev); > > 1991 if (err) > > 1992 goto err_res; > > 1993 > > 1994 err = vdpa_register_device(&mvdev->vdev); > > > > So mlx5v_remove() is better to be symmetrical. > > > > It's "struct mlx5_vdpa_net" that is being allocated here so it makes > sense to set this pointer as the the driver data. Anyway, it doesn't important. Thanks, for the original patch. Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c index 6b0a42183622..4103d3b64a2a 100644 --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c @@ -2036,9 +2036,9 @@ static int mlx5v_probe(struct auxiliary_device *adev, static void mlx5v_remove(struct auxiliary_device *adev) { - struct mlx5_vdpa_dev *mvdev = dev_get_drvdata(&adev->dev); + struct mlx5_vdpa_net *ndev = dev_get_drvdata(&adev->dev); - vdpa_unregister_device(&mvdev->vdev); + vdpa_unregister_device(&ndev->mvdev.vdev); } static const struct auxiliary_device_id mlx5v_id_table[] = {
struct mlx5_vdpa_net pointer was stored in drvdata. Extract it as well in mlx5v_remove(). Fixes: 74c9729dd892 ("vdpa/mlx5: Connect mlx5_vdpa to auxiliary bus") Signed-off-by: Eli Cohen <elic@nvidia.com> --- drivers/vdpa/mlx5/net/mlx5_vnet.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)