[PATCH v3 1/3] Revert "drivers: core: synchronize really_probe() and dev_uevent()"

[PATCH v3 1/3] Revert "drivers: core: synchronize really_probe() and dev_uevent()"

[Dmitry Torokhov]

This reverts commit c0a40097f0bc81deafc15f9195d1fb54595cd6d0.

Probing a device can take arbitrary long time. In the field we observed
that, for example, probing a bad micro-SD cards in an external USB card
reader (or maybe cards were good but cables were flaky) sometimes takes
longer than 2 minutes due to multiple retries at various levels of the
stack. We can not block uevent_show() method for that long because udev
is reading that attribute very often and that blocks udev and interferes
with booting of the system.

The change that introduced locking was concerned with dev_uevent()
racing with unbinding the driver. However we can handle it without
locking (which will be done in subsequent patch).

There was also claim that synchronization with probe() is needed to
properly load USB drivers, however this is a red herring: the change
adding the lock was introduced in May of last year and USB loading and
probing worked properly for many years before that.

Revert the harmful locking.

Cc: stable@vger.kernel.org
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/base/core.c | 3 ---
 1 file changed, 3 deletions(-)

v3: no changes.

v2: added Cc: stable, no code changes.

diff --git a/drivers/base/core.c b/drivers/base/core.c
index d2f9d3a59d6b..f9c1c623bca5 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -2726,11 +2726,8 @@ static ssize_t uevent_show(struct device *dev, struct device_attribute *attr,
 	if (!env)
 		return -ENOMEM;
 
-	/* Synchronize with really_probe() */
-	device_lock(dev);
 	/* let the kset specific function add its keys */
 	retval = kset->uevent_ops->uevent(&dev->kobj, env);
-	device_unlock(dev);
 	if (retval)
 		goto out;
 
-- 
2.49.0.rc0.332.g42c0ae87b1-goog

Re: [PATCH v3 1/3] Revert "drivers: core: synchronize really_probe() and dev_uevent()"

[Masami Hiramatsu]

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

> This reverts commit c0a40097f0bc81deafc15f9195d1fb54595cd6d0.
> 
> Probing a device can take arbitrary long time. In the field we observed
> that, for example, probing a bad micro-SD cards in an external USB card
> reader (or maybe cards were good but cables were flaky) sometimes takes
> longer than 2 minutes due to multiple retries at various levels of the
> stack. We can not block uevent_show() method for that long because udev
> is reading that attribute very often and that blocks udev and interferes
> with booting of the system.
> 
> The change that introduced locking was concerned with dev_uevent()
> racing with unbinding the driver. However we can handle it without
> locking (which will be done in subsequent patch).
> 
> There was also claim that synchronization with probe() is needed to
> properly load USB drivers, however this is a red herring: the change
> adding the lock was introduced in May of last year and USB loading and
> probing worked properly for many years before that.
> 
> Revert the harmful locking.
> 

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

Thanks,

> Cc: stable@vger.kernel.org
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/base/core.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> v3: no changes.
> 
> v2: added Cc: stable, no code changes.
> 
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index d2f9d3a59d6b..f9c1c623bca5 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -2726,11 +2726,8 @@ static ssize_t uevent_show(struct device *dev, struct device_attribute *attr,
>  	if (!env)
>  		return -ENOMEM;
>  
> -	/* Synchronize with really_probe() */
> -	device_lock(dev);
>  	/* let the kset specific function add its keys */
>  	retval = kset->uevent_ops->uevent(&dev->kobj, env);
> -	device_unlock(dev);
>  	if (retval)
>  		goto out;
>  
> -- 
> 2.49.0.rc0.332.g42c0ae87b1-goog
> 


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

Re: [PATCH v3 1/3] Revert "drivers: core: synchronize really_probe() and dev_uevent()"

[Aditya Garg]

> On 11 Mar 2025, at 10:54 AM, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> 
> This reverts commit c0a40097f0bc81deafc15f9195d1fb54595cd6d0.
> 
> Probing a device can take arbitrary long time. In the field we observed
> that, for example, probing a bad micro-SD cards in an external USB card
> reader (or maybe cards were good but cables were flaky) sometimes takes
> longer than 2 minutes due to multiple retries at various levels of the
> stack. We can not block uevent_show() method for that long because udev
> is reading that attribute very often and that blocks udev and interferes
> with booting of the system.
> 
> The change that introduced locking was concerned with dev_uevent()
> racing with unbinding the driver. However we can handle it without
> locking (which will be done in subsequent patch).
> 
> There was also claim that synchronization with probe() is needed to
> properly load USB drivers, however this is a red herring: the change
> adding the lock was introduced in May of last year and USB loading and
> probing worked properly for many years before that.
> 
> Revert the harmful locking.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---

Hi

For sometime users of the appletbdrm driver used for Touch Bar support on T2 Macs (https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/tiny/appletbdrm.c?h=v7.0-rc4) have been facing a regression that caused the Touch Bar to no longer work on resume. A person tried to bisect and this commit was found to be the reason.

The person has tried to explain the whole situation in this GitHub issue: https://github.com/t2linux/wiki/issues/635#issuecomment-4092907335

And this seems to be the most plausible explanation: https://github.com/t2linux/wiki/issues/635#issuecomment-4071720148

Thanks
Aditya

Re: [PATCH v3 1/3] Revert "drivers: core: synchronize really_probe() and dev_uevent()"

[Dmitry Torokhov]

On Fri, Mar 20, 2026 at 06:16:33AM +0000, Aditya Garg wrote:
> 
> 
> > On 11 Mar 2025, at 10:54 AM, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > 
> > This reverts commit c0a40097f0bc81deafc15f9195d1fb54595cd6d0.
> > 
> > Probing a device can take arbitrary long time. In the field we observed
> > that, for example, probing a bad micro-SD cards in an external USB card
> > reader (or maybe cards were good but cables were flaky) sometimes takes
> > longer than 2 minutes due to multiple retries at various levels of the
> > stack. We can not block uevent_show() method for that long because udev
> > is reading that attribute very often and that blocks udev and interferes
> > with booting of the system.
> > 
> > The change that introduced locking was concerned with dev_uevent()
> > racing with unbinding the driver. However we can handle it without
> > locking (which will be done in subsequent patch).
> > 
> > There was also claim that synchronization with probe() is needed to
> > properly load USB drivers, however this is a red herring: the change
> > adding the lock was introduced in May of last year and USB loading and
> > probing worked properly for many years before that.
> > 
> > Revert the harmful locking.
> > 
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> 
> Hi
> 

> For sometime users of the appletbdrm driver used for Touch Bar support
> on T2 Macs
> (https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/tiny/appletbdrm.c?h=v7.0-rc4)
> have been facing a regression that caused the Touch Bar to no longer
> work on resume. A person tried to bisect and this commit was found to
> be the reason.
> 
> The person has tried to explain the whole situation in this GitHub
> issue:
> https://github.com/t2linux/wiki/issues/635#issuecomment-4092907335
> 
> And this seems to be the most plausible explanation:
> https://github.com/t2linux/wiki/issues/635#issuecomment-4071720148

Hi,

I believe the person who did the root cause analysis told exactly how to
fix the issue. Your driver should not assume that the device is in given
configuration and if the device needs to be transitioned into particular
configuration which given timing it is on your driver to implement it.

It appears you have rule that triggers on "ADD" event for the device to
not only load the driver, but also toggle the configuration, and relied
on it be executed only after initial probe (that sets configuration 0)
is done. This is not supported and should not be expected to work. There
are dedicated "bind" and "unbind" events to signal when a device binds
or unbinds from a driver, but again, the proper fix is actually in your
driver's probe to configure the device properly.

Thanks.

-- 
Dmitry