[PATCH 1/2] mlx4: remove unnecessary pci_set_drvdata()

[PATCH 1/2] mlx4: remove unnecessary pci_set_drvdata()

[Zhu Yanjun]

The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not necessary to manually clear the
device driver data to NULL.

Cc: Joe Jin <joe.jin-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Cc: Junxiao Bi <junxiao.bi-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Zhu Yanjun <yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 drivers/net/ethernet/mellanox/mlx4/main.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index 09b9bc1..df9b0ef 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -3782,7 +3782,6 @@ static int __mlx4_init_one(struct pci_dev *pdev, int pci_dev_data,
 
 err_disable_pdev:
 	mlx4_pci_disable_device(&priv->dev);
-	pci_set_drvdata(pdev, NULL);
 	return err;
 }
 
@@ -3997,7 +3996,6 @@ static void mlx4_remove_one(struct pci_dev *pdev)
 	devlink_unregister(devlink);
 	kfree(dev->persist);
 	devlink_free(devlink);
-	pci_set_drvdata(pdev, NULL);
 }
 
 static int restore_current_port_types(struct mlx4_dev *dev,
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] mlx5: remove unnecessary pci_set_drvdata()

[Zhu Yanjun]

The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not necessary to manually clear the
device driver data to NULL.

Cc: Joe Jin <joe.jin-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Cc: Junxiao Bi <junxiao.bi-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Zhu Yanjun <yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 drivers/net/ethernet/mellanox/mlx5/core/main.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index c065132..2cafbdc 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -1407,7 +1407,6 @@ static int init_one(struct pci_dev *pdev,
 	cleanup_srcu_struct(&priv->pfault_srcu);
 clean_dev:
 #endif
-	pci_set_drvdata(pdev, NULL);
 	devlink_free(devlink);
 
 	return err;
@@ -1434,7 +1433,6 @@ static void remove_one(struct pci_dev *pdev)
 #ifdef CONFIG_INFINIBAND_ON_DEMAND_PAGING
 	cleanup_srcu_struct(&priv->pfault_srcu);
 #endif
-	pci_set_drvdata(pdev, NULL);
 	devlink_free(devlink);
 }
 
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] mlx4: remove unnecessary pci_set_drvdata()

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 443 bytes --]

On Tue, Aug 15, 2017 at 02:33:05AM -0400, Zhu Yanjun wrote:
> The driver core clears the driver data to NULL after device_release
> or on probe failure. Thus, it is not necessary to manually clear the
> device driver data to NULL.
>

It makes sense and I'm pretty sure that you are right, but I'm failing
to find the function in device core which sets it to NULL. Can you help
me and present the actual call stack to that code place?

Thanks,

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/2] mlx4: remove unnecessary pci_set_drvdata()

[Jason Gunthorpe]

On Tue, Aug 15, 2017 at 10:13:36AM +0300, Leon Romanovsky wrote:
> On Tue, Aug 15, 2017 at 02:33:05AM -0400, Zhu Yanjun wrote:
> > The driver core clears the driver data to NULL after device_release
> > or on probe failure. Thus, it is not necessary to manually clear the
> > device driver data to NULL.
> >
> 
> It makes sense and I'm pretty sure that you are right, but I'm failing
> to find the function in device core which sets it to NULL. Can you help
> me and present the actual call stack to that code place?

http://elixir.free-electrons.com/linux/v4.13-rc1/source/drivers/base/dd.c#L840

The call to the remove callback is on line 833.

This is done after dropping devres, so you could allocate the drv data
inside a devm object and everything would unwind correctly.

In this case, the kfree is explicit, so I would advocate for still putting
the null near the kfree to minimize the time where a free'd pointer is
present - eg incase a devm callback or some other bug accidently
touches it.

Jason

Re: [PATCH 1/2] mlx4: remove unnecessary pci_set_drvdata()

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1206 bytes --]

On Tue, Aug 15, 2017 at 11:21:49AM -0600, Jason Gunthorpe wrote:
> On Tue, Aug 15, 2017 at 10:13:36AM +0300, Leon Romanovsky wrote:
> > On Tue, Aug 15, 2017 at 02:33:05AM -0400, Zhu Yanjun wrote:
> > > The driver core clears the driver data to NULL after device_release
> > > or on probe failure. Thus, it is not necessary to manually clear the
> > > device driver data to NULL.
> > >
> >
> > It makes sense and I'm pretty sure that you are right, but I'm failing
> > to find the function in device core which sets it to NULL. Can you help
> > me and present the actual call stack to that code place?
>
> http://elixir.free-electrons.com/linux/v4.13-rc1/source/drivers/base/dd.c#L840
>
> The call to the remove callback is on line 833.
>
> This is done after dropping devres, so you could allocate the drv data
> inside a devm object and everything would unwind correctly.

Thanks for the pointer. Will it be called in case of failure during
initialization too?

>
> In this case, the kfree is explicit, so I would advocate for still putting
> the null near the kfree to minimize the time where a free'd pointer is
> present - eg incase a devm callback or some other bug accidently
> touches it.
>
> Jason

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/2] mlx4: remove unnecessary pci_set_drvdata()

[David Miller]

From: Zhu Yanjun <yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Date: Tue, 15 Aug 2017 02:33:05 -0400

> The driver core clears the driver data to NULL after device_release
> or on probe failure. Thus, it is not necessary to manually clear the
> device driver data to NULL.
> 
> Cc: Joe Jin <joe.jin-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Cc: Junxiao Bi <junxiao.bi-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Zhu Yanjun <yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

Applied.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] mlx5: remove unnecessary pci_set_drvdata()

[David Miller]

From: Zhu Yanjun <yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Date: Tue, 15 Aug 2017 02:33:06 -0400

> The driver core clears the driver data to NULL after device_release
> or on probe failure. Thus, it is not necessary to manually clear the
> device driver data to NULL.
> 
> Cc: Joe Jin <joe.jin-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Cc: Junxiao Bi <junxiao.bi-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Zhu Yanjun <yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

Applied.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html