Re: [PATCH v3 2/3] driver core: introduce device_set_driver() helper

[Masami Hiramatsu (Google) <mhiramat@kernel.org>]

On Mon, 10 Mar 2025 22:24:15 -0700
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> In preparation to closing a race when reading driver pointer in
> dev_uevent() code, instead of setting device->driver pointer directly
> introduce device_set_driver() helper.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

This looks good to me.

Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thanks,

> ---
> 
> v3: new patch introducing device_set_driver() helper (suggested by
> Rafael).
> 
>  drivers/base/base.h | 6 ++++++
>  drivers/base/core.c | 2 +-
>  drivers/base/dd.c   | 7 +++----
>  3 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/base/base.h b/drivers/base/base.h
> index 0042e4774b0c..eb203cf8370b 100644
> --- a/drivers/base/base.h
> +++ b/drivers/base/base.h
> @@ -180,6 +180,12 @@ int driver_add_groups(const struct device_driver *drv, const struct attribute_gr
>  void driver_remove_groups(const struct device_driver *drv, const struct attribute_group **groups);
>  void device_driver_detach(struct device *dev);
>  
> +static inline void device_set_driver(struct device *dev, const struct device_driver *drv)
> +{
> +	// FIXME - this cast should not be needed "soon"
> +	dev->driver = (struct device_driver *)drv;
> +}
> +
>  int devres_release_all(struct device *dev);
>  void device_block_probing(void);
>  void device_unblock_probing(void);
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index f9c1c623bca5..b000ee61c149 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -3697,7 +3697,7 @@ int device_add(struct device *dev)
>  	device_pm_remove(dev);
>  	dpm_sysfs_remove(dev);
>   DPMError:
> -	dev->driver = NULL;
> +	device_set_driver(dev, NULL);
>  	bus_remove_device(dev);
>   BusError:
>  	device_remove_attrs(dev);
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index f0e4b4aba885..b526e0e0f52d 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -550,7 +550,7 @@ static void device_unbind_cleanup(struct device *dev)
>  	arch_teardown_dma_ops(dev);
>  	kfree(dev->dma_range_map);
>  	dev->dma_range_map = NULL;
> -	dev->driver = NULL;
> +	device_set_driver(dev, NULL);
>  	dev_set_drvdata(dev, NULL);
>  	if (dev->pm_domain && dev->pm_domain->dismiss)
>  		dev->pm_domain->dismiss(dev);
> @@ -629,8 +629,7 @@ static int really_probe(struct device *dev, const struct device_driver *drv)
>  	}
>  
>  re_probe:
> -	// FIXME - this cast should not be needed "soon"
> -	dev->driver = (struct device_driver *)drv;
> +	device_set_driver(dev, drv);
>  
>  	/* If using pinctrl, bind pins now before probing */
>  	ret = pinctrl_bind_pins(dev);
> @@ -1014,7 +1013,7 @@ static int __device_attach(struct device *dev, bool allow_async)
>  		if (ret == 0)
>  			ret = 1;
>  		else {
> -			dev->driver = NULL;
> +			device_set_driver(dev, NULL);
>  			ret = 0;
>  		}
>  	} else {
> -- 
> 2.49.0.rc0.332.g42c0ae87b1-goog
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>