On Thu, Dec 15, 2016 at 12:32:08PM +0100, Takashi Iwai wrote:
On Wed, 14 Dec 2016 22:00:50 +0100, Imre Deak wrote:
Hi,
I got the trace below while trying to unload (unbind) snd_hda_intel, while its still loading the HDMI codec driver. IIUC what happens is:
Task1 Task2 Task3 modprobe snd_hda_intel schedule(azx_probe_work) unbind snd_hda_intel via sysfs device_release_driver() device_lock(snd_hda_intel) azx_remove() cancel_work_sync(azx_probe_work) azx_probe_work() request_module(snd-hda-codec-hdmi) hdmi_driver_init() __driver_attach() device_lock(snd_hda_intel)
Deadlock, since azx_probe_work() will never finish and the snd_hda_intel device lock will never get released.
This is indeed nasty. The deadlock happens when the driver core takes the parent's device lock.
static int __driver_attach(struct device *dev, void *data) { .... if (dev->parent) /* Needed for USB */ device_lock(dev->parent); device_lock(dev); if (!dev->driver) driver_probe_device(drv, dev);
I vaguely remember of some other issue due to the device_lock of the parent device. And, I guess a similar deadlock may happen not only with HD-audio driver but also in general with every driver using async probe.
Greg, any good way to avoid such a deadlock? Can we make the parent device lock conditional somehow?
Ick, messy. I don't want to make the parent lock conditional, as it's needed. Shouldn't the cancel_work_sync() prevent the request_module() from running? Seems like you need to serialize your probe_work somehow...
I don't see a way to handle this in the driver core, and no:
--- a/sound/hda/hdac_device.c +++ b/sound/hda/hdac_device.c @@ -44,7 +44,7 @@ int snd_hdac_device_init(struct hdac_device *codec, struct hdac_bus *bus,
dev = &codec->dev; device_initialize(dev);
- dev->parent = bus->dev;
- // dev->parent = bus->dev; dev->bus = &snd_hda_bus_type; dev->release = default_release; dev->groups = hdac_dev_attr_groups;
... but it's obviously ugly :)
That's not a good solution either :)
thanks,
greg k-h