Re: [PATCH v2 1/7] drivers/base: Fix a race condition in the device probing code

2018-10-18 Thread Bart Van Assche 
On Thu, 2018-10-18 at 07:10 +0200, Greg Kroah-Hartman wrote:
> > diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> > index edfc9f0b1180..b4212154a94b 100644
> > --- a/drivers/base/dd.c
> > +++ b/drivers/base/dd.c
> > @@ -645,6 +645,14 @@ int driver_probe_device(struct device_driver *drv, 
> > struct device *dev)
> >  {
> > int ret = 0;
> >  
> > +   /*
> > +* Several callers check the driver pointer without holding the
> > +* device mutex. Hence check the driver pointer again while holding
> > +* the device mutex.
> > +*/
> > +   if (dev->driver)
> > +   return dev->driver == drv;
> 
> I do not understand, who is calling probe twice?  What is the sequence
> of events that is causing that error message being printed out?  Wh is
> not grabbing the mutex properly?

Hi Greg,

If I drop patch 3/7 from my patch series and use Alexander Duyck's patches
instead I don't need this patch anymore. So I don't think we have to spend
more time on this patch.

Thanks,

Bart.

Re: [PATCH v2 1/7] drivers/base: Fix a race condition in the device probing code

2018-10-17 Thread Greg Kroah-Hartman 
On Wed, Oct 17, 2018 at 04:40:00PM -0700, Bart Van Assche wrote:
> This patch avoids that complaints similar to the following appear in the
> system log:
> 
> sysfs: cannot create duplicate filename 
> '/devices/pseudo_0/adapter0/host3/target3:0:0/3:0:0:133/driver'
> 
> Cc: Lee Duncan 
> Cc: Hannes Reinecke 
> Cc: Luis Chamberlain 
> Cc: Johannes Thumshirn 
> Cc: Christoph Hellwig 
> Cc: Greg Kroah-Hartman 
> Signed-off-by: Bart Van Assche 
> ---
>  drivers/base/dd.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index edfc9f0b1180..b4212154a94b 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -645,6 +645,14 @@ int driver_probe_device(struct device_driver *drv, 
> struct device *dev)
>  {
>   int ret = 0;
>  
> + /*
> +  * Several callers check the driver pointer without holding the
> +  * device mutex. Hence check the driver pointer again while holding
> +  * the device mutex.
> +  */
> + if (dev->driver)
> + return dev->driver == drv;

I do not understand, who is calling probe twice?  What is the sequence
of events that is causing that error message being printed out?  Wh is
not grabbing the mutex properly?

thanks,

greg k-h

[PATCH v2 1/7] drivers/base: Fix a race condition in the device probing code

2018-10-17 Thread Bart Van Assche 
This patch avoids that complaints similar to the following appear in the
system log:

sysfs: cannot create duplicate filename 
'/devices/pseudo_0/adapter0/host3/target3:0:0/3:0:0:133/driver'

Cc: Lee Duncan 
Cc: Hannes Reinecke 
Cc: Luis Chamberlain 
Cc: Johannes Thumshirn 
Cc: Christoph Hellwig 
Cc: Greg Kroah-Hartman 
Signed-off-by: Bart Van Assche 
---
 drivers/base/dd.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index edfc9f0b1180..b4212154a94b 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -645,6 +645,14 @@ int driver_probe_device(struct device_driver *drv, struct 
device *dev)
 {
int ret = 0;
 
+   /*
+* Several callers check the driver pointer without holding the
+* device mutex. Hence check the driver pointer again while holding
+* the device mutex.
+*/
+   if (dev->driver)
+   return dev->driver == drv;
+
if (!device_is_registered(dev))
return -ENODEV;
 
-- 
2.19.1.568.g152ad8e336-goog