e6ecd2215f2c737ffa452bab2baa6b7412420eab
[openwrt/openwrt.git] /
1 From a9fd0953fa4a62887306be28641b4b0809f3b2fd Mon Sep 17 00:00:00 2001
2 From: Piotr Figiel <p.figiel@camlintechnologies.com>
3 Date: Wed, 13 Mar 2019 09:52:42 +0000
4 Subject: [PATCH] brcmfmac: convert dev_init_lock mutex to completion
5
6 Leaving dev_init_lock mutex locked in probe causes BUG and a WARNING when
7 kernel is compiled with CONFIG_PROVE_LOCKING. Convert mutex to completion
8 which silences those warnings and improves code readability.
9
10 Fix below errors when connecting the USB WiFi dongle:
11
12 brcmfmac: brcmf_fw_alloc_request: using brcm/brcmfmac43143 for chip BCM43143/2
13 BUG: workqueue leaked lock or atomic: kworker/0:2/0x00000000/434
14 last function: hub_event
15 1 lock held by kworker/0:2/434:
16 #0: 18d5dcdf (&devinfo->dev_init_lock){+.+.}, at: brcmf_usb_probe+0x78/0x550 [brcmfmac]
17 CPU: 0 PID: 434 Comm: kworker/0:2 Not tainted 4.19.23-00084-g454a789-dirty #123
18 Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
19 Workqueue: usb_hub_wq hub_event
20 [<8011237c>] (unwind_backtrace) from [<8010d74c>] (show_stack+0x10/0x14)
21 [<8010d74c>] (show_stack) from [<809c4324>] (dump_stack+0xa8/0xd4)
22 [<809c4324>] (dump_stack) from [<8014195c>] (process_one_work+0x710/0x808)
23 [<8014195c>] (process_one_work) from [<80141a80>] (worker_thread+0x2c/0x564)
24 [<80141a80>] (worker_thread) from [<80147bcc>] (kthread+0x13c/0x16c)
25 [<80147bcc>] (kthread) from [<801010b4>] (ret_from_fork+0x14/0x20)
26 Exception stack(0xed1d9fb0 to 0xed1d9ff8)
27 9fa0: 00000000 00000000 00000000 00000000
28 9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
29 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000
30
31 ======================================================
32 WARNING: possible circular locking dependency detected
33 4.19.23-00084-g454a789-dirty #123 Not tainted
34 ------------------------------------------------------
35 kworker/0:2/434 is trying to acquire lock:
36 e29cf799 ((wq_completion)"events"){+.+.}, at: process_one_work+0x174/0x808
37
38 but task is already holding lock:
39 18d5dcdf (&devinfo->dev_init_lock){+.+.}, at: brcmf_usb_probe+0x78/0x550 [brcmfmac]
40
41 which lock already depends on the new lock.
42
43 the existing dependency chain (in reverse order) is:
44
45 -> #2 (&devinfo->dev_init_lock){+.+.}:
46 mutex_lock_nested+0x1c/0x24
47 brcmf_usb_probe+0x78/0x550 [brcmfmac]
48 usb_probe_interface+0xc0/0x1bc
49 really_probe+0x228/0x2c0
50 __driver_attach+0xe4/0xe8
51 bus_for_each_dev+0x68/0xb4
52 bus_add_driver+0x19c/0x214
53 driver_register+0x78/0x110
54 usb_register_driver+0x84/0x148
55 process_one_work+0x228/0x808
56 worker_thread+0x2c/0x564
57 kthread+0x13c/0x16c
58 ret_from_fork+0x14/0x20
59 (null)
60
61 -> #1 (brcmf_driver_work){+.+.}:
62 worker_thread+0x2c/0x564
63 kthread+0x13c/0x16c
64 ret_from_fork+0x14/0x20
65 (null)
66
67 -> #0 ((wq_completion)"events"){+.+.}:
68 process_one_work+0x1b8/0x808
69 worker_thread+0x2c/0x564
70 kthread+0x13c/0x16c
71 ret_from_fork+0x14/0x20
72 (null)
73
74 other info that might help us debug this:
75
76 Chain exists of:
77 (wq_completion)"events" --> brcmf_driver_work --> &devinfo->dev_init_lock
78
79 Possible unsafe locking scenario:
80
81 CPU0 CPU1
82 ---- ----
83 lock(&devinfo->dev_init_lock);
84 lock(brcmf_driver_work);
85 lock(&devinfo->dev_init_lock);
86 lock((wq_completion)"events");
87
88 *** DEADLOCK ***
89
90 1 lock held by kworker/0:2/434:
91 #0: 18d5dcdf (&devinfo->dev_init_lock){+.+.}, at: brcmf_usb_probe+0x78/0x550 [brcmfmac]
92
93 stack backtrace:
94 CPU: 0 PID: 434 Comm: kworker/0:2 Not tainted 4.19.23-00084-g454a789-dirty #123
95 Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
96 Workqueue: events request_firmware_work_func
97 [<8011237c>] (unwind_backtrace) from [<8010d74c>] (show_stack+0x10/0x14)
98 [<8010d74c>] (show_stack) from [<809c4324>] (dump_stack+0xa8/0xd4)
99 [<809c4324>] (dump_stack) from [<80172838>] (print_circular_bug+0x210/0x330)
100 [<80172838>] (print_circular_bug) from [<80175940>] (__lock_acquire+0x160c/0x1a30)
101 [<80175940>] (__lock_acquire) from [<8017671c>] (lock_acquire+0xe0/0x268)
102 [<8017671c>] (lock_acquire) from [<80141404>] (process_one_work+0x1b8/0x808)
103 [<80141404>] (process_one_work) from [<80141a80>] (worker_thread+0x2c/0x564)
104 [<80141a80>] (worker_thread) from [<80147bcc>] (kthread+0x13c/0x16c)
105 [<80147bcc>] (kthread) from [<801010b4>] (ret_from_fork+0x14/0x20)
106 Exception stack(0xed1d9fb0 to 0xed1d9ff8)
107 9fa0: 00000000 00000000 00000000 00000000
108 9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
109 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000
110
111 Signed-off-by: Piotr Figiel <p.figiel@camlintechnologies.com>
112 Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
113 ---
114 .../wireless/broadcom/brcm80211/brcmfmac/usb.c | 17 ++++++++---------
115 1 file changed, 8 insertions(+), 9 deletions(-)
116
117 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
118 +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
119 @@ -160,7 +160,7 @@ struct brcmf_usbdev_info {
120
121 struct usb_device *usbdev;
122 struct device *dev;
123 - struct mutex dev_init_lock;
124 + struct completion dev_init_done;
125
126 int ctl_in_pipe, ctl_out_pipe;
127 struct urb *ctl_urb; /* URB for control endpoint */
128 @@ -1194,11 +1194,11 @@ static void brcmf_usb_probe_phase2(struc
129 if (ret)
130 goto error;
131
132 - mutex_unlock(&devinfo->dev_init_lock);
133 + complete(&devinfo->dev_init_done);
134 return;
135 error:
136 brcmf_dbg(TRACE, "failed: dev=%s, err=%d\n", dev_name(dev), ret);
137 - mutex_unlock(&devinfo->dev_init_lock);
138 + complete(&devinfo->dev_init_done);
139 device_release_driver(dev);
140 }
141
142 @@ -1266,7 +1266,7 @@ static int brcmf_usb_probe_cb(struct brc
143 if (ret)
144 goto fail;
145 /* we are done */
146 - mutex_unlock(&devinfo->dev_init_lock);
147 + complete(&devinfo->dev_init_done);
148 return 0;
149 }
150 bus->chip = bus_pub->devid;
151 @@ -1326,11 +1326,10 @@ brcmf_usb_probe(struct usb_interface *in
152
153 devinfo->usbdev = usb;
154 devinfo->dev = &usb->dev;
155 - /* Take an init lock, to protect for disconnect while still loading.
156 + /* Init completion, to protect for disconnect while still loading.
157 * Necessary because of the asynchronous firmware load construction
158 */
159 - mutex_init(&devinfo->dev_init_lock);
160 - mutex_lock(&devinfo->dev_init_lock);
161 + init_completion(&devinfo->dev_init_done);
162
163 usb_set_intfdata(intf, devinfo);
164
165 @@ -1408,7 +1407,7 @@ brcmf_usb_probe(struct usb_interface *in
166 return 0;
167
168 fail:
169 - mutex_unlock(&devinfo->dev_init_lock);
170 + complete(&devinfo->dev_init_done);
171 kfree(devinfo);
172 usb_set_intfdata(intf, NULL);
173 return ret;
174 @@ -1423,7 +1422,7 @@ brcmf_usb_disconnect(struct usb_interfac
175 devinfo = (struct brcmf_usbdev_info *)usb_get_intfdata(intf);
176
177 if (devinfo) {
178 - mutex_lock(&devinfo->dev_init_lock);
179 + wait_for_completion(&devinfo->dev_init_done);
180 /* Make sure that devinfo still exists. Firmware probe routines
181 * may have released the device and cleared the intfdata.
182 */