Skip to content
This repository was archived by the owner on Jan 10, 2023. It is now read-only.

Commit 4b2f0eb

Browse files
figielgregkh
authored andcommitted
brcmfmac: convert dev_init_lock mutex to completion
[ Upstream commit a9fd0953fa4a62887306be28641b4b0809f3b2fd ] 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 59ec3ad commit 4b2f0eb

1 file changed

Lines changed: 8 additions & 9 deletions

File tree

  • drivers/net/wireless/broadcom/brcm80211/brcmfmac

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

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ struct brcmf_usbdev_info {
160160

161161
struct usb_device *usbdev;
162162
struct device *dev;
163-
struct mutex dev_init_lock;
163+
struct completion dev_init_done;
164164

165165
int ctl_in_pipe, ctl_out_pipe;
166166
struct urb *ctl_urb; /* URB for control endpoint */
@@ -1195,11 +1195,11 @@ static void brcmf_usb_probe_phase2(struct device *dev, int ret,
11951195
if (ret)
11961196
goto error;
11971197

1198-
mutex_unlock(&devinfo->dev_init_lock);
1198+
complete(&devinfo->dev_init_done);
11991199
return;
12001200
error:
12011201
brcmf_dbg(TRACE, "failed: dev=%s, err=%d\n", dev_name(dev), ret);
1202-
mutex_unlock(&devinfo->dev_init_lock);
1202+
complete(&devinfo->dev_init_done);
12031203
device_release_driver(dev);
12041204
}
12051205

@@ -1267,7 +1267,7 @@ static int brcmf_usb_probe_cb(struct brcmf_usbdev_info *devinfo)
12671267
if (ret)
12681268
goto fail;
12691269
/* we are done */
1270-
mutex_unlock(&devinfo->dev_init_lock);
1270+
complete(&devinfo->dev_init_done);
12711271
return 0;
12721272
}
12731273
bus->chip = bus_pub->devid;
@@ -1327,11 +1327,10 @@ brcmf_usb_probe(struct usb_interface *intf, const struct usb_device_id *id)
13271327

13281328
devinfo->usbdev = usb;
13291329
devinfo->dev = &usb->dev;
1330-
/* Take an init lock, to protect for disconnect while still loading.
1330+
/* Init completion, to protect for disconnect while still loading.
13311331
* Necessary because of the asynchronous firmware load construction
13321332
*/
1333-
mutex_init(&devinfo->dev_init_lock);
1334-
mutex_lock(&devinfo->dev_init_lock);
1333+
init_completion(&devinfo->dev_init_done);
13351334

13361335
usb_set_intfdata(intf, devinfo);
13371336

@@ -1409,7 +1408,7 @@ brcmf_usb_probe(struct usb_interface *intf, const struct usb_device_id *id)
14091408
return 0;
14101409

14111410
fail:
1412-
mutex_unlock(&devinfo->dev_init_lock);
1411+
complete(&devinfo->dev_init_done);
14131412
kfree(devinfo);
14141413
usb_set_intfdata(intf, NULL);
14151414
return ret;
@@ -1424,7 +1423,7 @@ brcmf_usb_disconnect(struct usb_interface *intf)
14241423
devinfo = (struct brcmf_usbdev_info *)usb_get_intfdata(intf);
14251424

14261425
if (devinfo) {
1427-
mutex_lock(&devinfo->dev_init_lock);
1426+
wait_for_completion(&devinfo->dev_init_done);
14281427
/* Make sure that devinfo still exists. Firmware probe routines
14291428
* may have released the device and cleared the intfdata.
14301429
*/

0 commit comments

Comments
 (0)