Skip to content

Commit d696d38

Browse files
figielgregkh
authored andcommitted
brcmfmac: convert dev_init_lock mutex to completion
[ Upstream commit a9fd095 ] Leaving dev_init_lock mutex locked in probe causes BUG and a WARNING when kernel is compiled with CONFIG_PROVE_LOCKING. Convert mutex to completion which silences those warnings and improves code readability. Fix below errors when connecting the USB WiFi dongle: brcmfmac: brcmf_fw_alloc_request: using brcm/brcmfmac43143 for chip BCM43143/2 BUG: workqueue leaked lock or atomic: kworker/0:2/0x00000000/434 last function: hub_event 1 lock held by kworker/0:2/434: #0: 18d5dcdf (&devinfo->dev_init_lock){+.+.}, at: brcmf_usb_probe+0x78/0x550 [brcmfmac] CPU: 0 PID: 434 Comm: kworker/0:2 Not tainted 4.19.23-00084-g454a789-dirty #123 Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree) Workqueue: usb_hub_wq hub_event [<8011237c>] (unwind_backtrace) from [<8010d74c>] (show_stack+0x10/0x14) [<8010d74c>] (show_stack) from [<809c4324>] (dump_stack+0xa8/0xd4) [<809c4324>] (dump_stack) from [<8014195c>] (process_one_work+0x710/0x808) [<8014195c>] (process_one_work) from [<80141a80>] (worker_thread+0x2c/0x564) [<80141a80>] (worker_thread) from [<80147bcc>] (kthread+0x13c/0x16c) [<80147bcc>] (kthread) from [<801010b4>] (ret_from_fork+0x14/0x20) Exception stack(0xed1d9fb0 to 0xed1d9ff8) 9fa0: 00000000 00000000 00000000 00000000 9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000 ====================================================== WARNING: possible circular locking dependency detected 4.19.23-00084-g454a789-dirty #123 Not tainted ------------------------------------------------------ kworker/0:2/434 is trying to acquire lock: e29cf799 ((wq_completion)"events"){+.+.}, at: process_one_work+0x174/0x808 but task is already holding lock: 18d5dcdf (&devinfo->dev_init_lock){+.+.}, at: brcmf_usb_probe+0x78/0x550 [brcmfmac] which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (&devinfo->dev_init_lock){+.+.}: mutex_lock_nested+0x1c/0x24 brcmf_usb_probe+0x78/0x550 [brcmfmac] usb_probe_interface+0xc0/0x1bc really_probe+0x228/0x2c0 __driver_attach+0xe4/0xe8 bus_for_each_dev+0x68/0xb4 bus_add_driver+0x19c/0x214 driver_register+0x78/0x110 usb_register_driver+0x84/0x148 process_one_work+0x228/0x808 worker_thread+0x2c/0x564 kthread+0x13c/0x16c ret_from_fork+0x14/0x20 (null) -> #1 (brcmf_driver_work){+.+.}: worker_thread+0x2c/0x564 kthread+0x13c/0x16c ret_from_fork+0x14/0x20 (null) -> #0 ((wq_completion)"events"){+.+.}: process_one_work+0x1b8/0x808 worker_thread+0x2c/0x564 kthread+0x13c/0x16c ret_from_fork+0x14/0x20 (null) other info that might help us debug this: Chain exists of: (wq_completion)"events" --> brcmf_driver_work --> &devinfo->dev_init_lock Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(&devinfo->dev_init_lock); lock(brcmf_driver_work); lock(&devinfo->dev_init_lock); lock((wq_completion)"events"); *** DEADLOCK *** 1 lock held by kworker/0:2/434: #0: 18d5dcdf (&devinfo->dev_init_lock){+.+.}, at: brcmf_usb_probe+0x78/0x550 [brcmfmac] stack backtrace: CPU: 0 PID: 434 Comm: kworker/0:2 Not tainted 4.19.23-00084-g454a789-dirty #123 Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree) Workqueue: events request_firmware_work_func [<8011237c>] (unwind_backtrace) from [<8010d74c>] (show_stack+0x10/0x14) [<8010d74c>] (show_stack) from [<809c4324>] (dump_stack+0xa8/0xd4) [<809c4324>] (dump_stack) from [<80172838>] (print_circular_bug+0x210/0x330) [<80172838>] (print_circular_bug) from [<80175940>] (__lock_acquire+0x160c/0x1a30) [<80175940>] (__lock_acquire) from [<8017671c>] (lock_acquire+0xe0/0x268) [<8017671c>] (lock_acquire) from [<80141404>] (process_one_work+0x1b8/0x808) [<80141404>] (process_one_work) from [<80141a80>] (worker_thread+0x2c/0x564) [<80141a80>] (worker_thread) from [<80147bcc>] (kthread+0x13c/0x16c) [<80147bcc>] (kthread) from [<801010b4>] (ret_from_fork+0x14/0x20) Exception stack(0xed1d9fb0 to 0xed1d9ff8) 9fa0: 00000000 00000000 00000000 00000000 9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000 Signed-off-by: Piotr Figiel <p.figiel@camlintechnologies.com> Signed-off-by: Kalle Valo <kvalo@codeaurora.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent 66fb581 commit d696d38

File tree

1 file changed

+8
-9
lines changed
  • drivers/net/wireless/broadcom/brcm80211/brcmfmac

1 file changed

+8
-9
lines changed

drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c

+8-9
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ struct brcmf_usbdev_info {
157157

158158
struct usb_device *usbdev;
159159
struct device *dev;
160-
struct mutex dev_init_lock;
160+
struct completion dev_init_done;
161161

162162
int ctl_in_pipe, ctl_out_pipe;
163163
struct urb *ctl_urb; /* URB for control endpoint */
@@ -1189,11 +1189,11 @@ static void brcmf_usb_probe_phase2(struct device *dev, int ret,
11891189
if (ret)
11901190
goto error;
11911191

1192-
mutex_unlock(&devinfo->dev_init_lock);
1192+
complete(&devinfo->dev_init_done);
11931193
return;
11941194
error:
11951195
brcmf_dbg(TRACE, "failed: dev=%s, err=%d\n", dev_name(dev), ret);
1196-
mutex_unlock(&devinfo->dev_init_lock);
1196+
complete(&devinfo->dev_init_done);
11971197
device_release_driver(dev);
11981198
}
11991199

@@ -1239,7 +1239,7 @@ static int brcmf_usb_probe_cb(struct brcmf_usbdev_info *devinfo)
12391239
if (ret)
12401240
goto fail;
12411241
/* we are done */
1242-
mutex_unlock(&devinfo->dev_init_lock);
1242+
complete(&devinfo->dev_init_done);
12431243
return 0;
12441244
}
12451245
bus->chip = bus_pub->devid;
@@ -1300,11 +1300,10 @@ brcmf_usb_probe(struct usb_interface *intf, const struct usb_device_id *id)
13001300

13011301
devinfo->usbdev = usb;
13021302
devinfo->dev = &usb->dev;
1303-
/* Take an init lock, to protect for disconnect while still loading.
1303+
/* Init completion, to protect for disconnect while still loading.
13041304
* Necessary because of the asynchronous firmware load construction
13051305
*/
1306-
mutex_init(&devinfo->dev_init_lock);
1307-
mutex_lock(&devinfo->dev_init_lock);
1306+
init_completion(&devinfo->dev_init_done);
13081307

13091308
usb_set_intfdata(intf, devinfo);
13101309

@@ -1382,7 +1381,7 @@ brcmf_usb_probe(struct usb_interface *intf, const struct usb_device_id *id)
13821381
return 0;
13831382

13841383
fail:
1385-
mutex_unlock(&devinfo->dev_init_lock);
1384+
complete(&devinfo->dev_init_done);
13861385
kfree(devinfo);
13871386
usb_set_intfdata(intf, NULL);
13881387
return ret;
@@ -1397,7 +1396,7 @@ brcmf_usb_disconnect(struct usb_interface *intf)
13971396
devinfo = (struct brcmf_usbdev_info *)usb_get_intfdata(intf);
13981397

13991398
if (devinfo) {
1400-
mutex_lock(&devinfo->dev_init_lock);
1399+
wait_for_completion(&devinfo->dev_init_done);
14011400
/* Make sure that devinfo still exists. Firmware probe routines
14021401
* may have released the device and cleared the intfdata.
14031402
*/

0 commit comments

Comments
 (0)