Re: [PATCH v2 3/3] driver core: take parent lock in async device attach

[Alan Stern]

On Wed, Oct 14, 2026 at 03:31:09PM +0200, Johan Hovold wrote:
> From: Mario Peter <mario.peter@xxxxxxxxxxxxxxxxxxxx>
> 
> On buses with need_parent_lock set (only USB), probe() must run with the
> parent device locked. A synchronous attach after device_add() gets this
> from the caller, e.g. usb_set_configuration() holds the udev lock while
> adding interfaces. __device_attach_async_helper() runs the probe from an
> async worker instead, where that lock isn't held, and only takes
> device_lock(dev).
> 
> With async probing enabled for the hub driver (e.g. driver_async_probe=*
> or driver_async_probe=hub), this races when a multi-TT hub is
> configured. For multi-TT hubs, hub_probe() calls
> usb_set_interface(hdev, 0, 1) to select the TT-per-port altsetting, and
> usb_set_interface() creates the endpoint devices of the interface a
> second time:
> 
>   usb_set_configuration()           async worker
>     [udev locked]
>     device_add(intf)
>       queues the probe  ----------> __device_attach_async_helper()
>                                       [intf locked, udev not locked]
>                                       hub_probe()
>                                         usb_set_interface(hdev, 0, 1)
>     create_intf_ep_devs(intf)             create_intf_ep_devs(intf)
> 
> create_intf_ep_devs() checks and sets intf->ep_devs_created without a
> lock of its own. If usb_set_configuration() has created ep_81 but not
> yet set the flag, usb_set_interface() skips removing the endpoint
> devices of the old altsetting and creates ep_81 again. With a
> synchronous probe, hub_probe() runs inside device_add() with the udev
> lock held, and usb_set_configuration() then finds ep_devs_created
> already set.
> 
> Single-TT hubs don't call usb_set_interface() from hub_probe() and
> don't show this warning, but their probe runs without the udev lock as
> well.
> 
> On an i.MX8MM board with an onboard USB2514 hub this hit 11 of 100
> boots:
> 
>   sysfs: cannot create duplicate filename '.../1-1/1-1:1.0/ep_81'
>   Call trace:
>    sysfs_warn_dup
>    usb_create_ep_devs
>    create_intf_ep_devs
>    usb_set_interface
>    hub_probe
>    usb_probe_interface
>    really_probe
>    __device_attach_async_helper
>    async_run_entry_fn
> 
> It also shows on v7.3-rc6 in QEMU, with dummy_hcd and a raw-gadget
> program emulating a multi-TT hub: re-authorizing the hub in a loop,
> while a SCHED_FIFO task preempts the writing task at random points for
> 3 ms, hit the warning in 29 of 6000 iterations.
> 
> Take the parent lock in __device_attach_async_helper() as well, like
> __driver_attach_async_helper() does, and move
> __device_driver_lock/unlock() up for that. With this the warning was
> gone in 300 boots on the board and in 6000 iterations in QEMU.
> 
> Fixes: 765230b5f084 ("driver-core: add asynchronous probing support for drivers")
> Assisted-by: LLM
> Signed-off-by: Mario Peter <mario.peter@xxxxxxxxxxxxxxxxxxxx>
> [ johan: switch to async_schedule_dev_nocall() to avoid deadlock if
>          async_schedule_dev() runs synchronously when parent lock is
> 	 held ]
> Cc: stable@xxxxxxxxxxxxxxx	# 4.2
> Cc: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> Signed-off-by: Johan Hovold <johan@xxxxxxxxxx>
> ---

...

> @@ -1118,8 +1150,13 @@ static int __device_attach(struct device *dev, bool allow_async)
>  	}
>  out_unlock:
>  	device_unlock(dev);
> -	if (async)
> -		async_schedule_dev(__device_attach_async_helper, dev);
> +	if (async) {
> +		if (!async_schedule_dev_nocall(__device_attach_async_helper, dev)) {
> +			put_device(dev);
> +			ret = 2;	/* retry synchronously */
> +		}
> +	}
> +
>  	return ret;
>  }
>  
> @@ -1146,48 +1183,20 @@ EXPORT_SYMBOL_GPL(device_attach);
>  void device_initial_probe(struct device *dev)
>  {
>  	struct subsys_private *sp = bus_to_subsys(dev->bus);
> +	int ret;
>  
>  	if (!sp)
>  		return;
>  
> -	if (sp->drivers_autoprobe)
> -		__device_attach(dev, true);
> +	if (sp->drivers_autoprobe) {
> +		ret = __device_attach(dev, true);
> +		if (ret == 2)
> +			__device_attach(dev, false);
> +	}
>  
>  	subsys_put(sp);
>  }

These two pieces do not seem to be related to the changes mentioned in 
the patch description.  Are they leftovers from some other patch?

Alan Stern