| Message ID | 1404120720-14649-1-git-send-email-michal.kazior@tieto.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
Michal Kazior <michal.kazior@tieto.com> writes: > If firmware probing worker failed it called > device_release_driver() which synchronously called > remove() pci callback. The callback in turn waited > for the worker that called it to finish resulting > in a deadlock. > > Waiting for a completion instead of a worker, like > some other drivers do, doesn't seem like the best > idea either: > > Syscall Worker > > probe_fw() > rmmod > dev_lock() > pci->remove() > wait_for_completion() > complete_all() > device_release_driver() > dev_lock() > [sleep] > free(ar) > dev_unlock() > [resume] > > There's no guarantee that Worker upon resuming can > still access any data/code of the module. > > Leaving device bound to a driver is not as harmful > as deadlocking so remove the call to > device_release_driver() while a proper solution is > figured out. > > Signed-off-by: Michal Kazior <michal.kazior@tieto.com> Thanks, applied.
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c index 82017f5..b70e443 100644 --- a/drivers/net/wireless/ath/ath10k/core.c +++ b/drivers/net/wireless/ath/ath10k/core.c @@ -984,7 +984,9 @@ err_unregister_mac: err_release_fw: ath10k_core_free_firmware_files(ar); err: - device_release_driver(ar->dev); + /* TODO: It's probably a good idea to release device from the driver + * but calling device_release_driver() here will cause a deadlock. + */ return; }
If firmware probing worker failed it called device_release_driver() which synchronously called remove() pci callback. The callback in turn waited for the worker that called it to finish resulting in a deadlock. Waiting for a completion instead of a worker, like some other drivers do, doesn't seem like the best idea either: Syscall Worker probe_fw() rmmod dev_lock() pci->remove() wait_for_completion() complete_all() device_release_driver() dev_lock() [sleep] free(ar) dev_unlock() [resume] There's no guarantee that Worker upon resuming can still access any data/code of the module. Leaving device bound to a driver is not as harmful as deadlocking so remove the call to device_release_driver() while a proper solution is figured out. Signed-off-by: Michal Kazior <michal.kazior@tieto.com> --- drivers/net/wireless/ath/ath10k/core.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)