From: Kalle Valo <kvalo@codeaurora.org>
To: Piotr Figiel <p.figiel@camlintechnologies.com>
Cc: "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
"arend.vanspriel@broadcom.com" <arend.vanspriel@broadcom.com>,
"franky.lin@broadcom.com" <franky.lin@broadcom.com>,
"hante.meuleman@broadcom.com" <hante.meuleman@broadcom.com>,
"chi-hsien.lin@cypress.com" <chi-hsien.lin@cypress.com>,
"wright.feng@cypress.com" <wright.feng@cypress.com>,
"brcm80211-dev-list@cypress.com" <brcm80211-dev-list@cypress.com>,
"Krzysztof Drobiński" <k.drobinski@camlintechnologies.com>,
"Pawel Lenkow" <p.lenkow@camlintechnologies.com>,
"Lech Perczak" <l.perczak@camlintechnologies.com>,
"Piotr Figiel" <p.figiel@camlintechnologies.com>
Subject: Re: [PATCH] brcmfmac: convert dev_init_lock mutex to completion
Date: Thu, 4 Apr 2019 10:13:00 +0000 (UTC) [thread overview]
Message-ID: <20190404101300.9A2E661A26@smtp.codeaurora.org> (raw)
In-Reply-To: <1552470749-3625-1-git-send-email-p.figiel@camlintechnologies.com>
Piotr Figiel <p.figiel@camlintechnologies.com> wrote:
> 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>
Patch applied to wireless-drivers-next.git, thanks.
a9fd0953fa4a brcmfmac: convert dev_init_lock mutex to completion
--
https://patchwork.kernel.org/patch/10850843/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
prev parent reply other threads:[~2019-04-04 10:13 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-13 9:52 [PATCH] brcmfmac: convert dev_init_lock mutex to completion Piotr Figiel
2019-04-04 10:13 ` Kalle Valo [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190404101300.9A2E661A26@smtp.codeaurora.org \
--to=kvalo@codeaurora.org \
--cc=arend.vanspriel@broadcom.com \
--cc=brcm80211-dev-list@cypress.com \
--cc=chi-hsien.lin@cypress.com \
--cc=franky.lin@broadcom.com \
--cc=hante.meuleman@broadcom.com \
--cc=k.drobinski@camlintechnologies.com \
--cc=l.perczak@camlintechnologies.com \
--cc=linux-wireless@vger.kernel.org \
--cc=p.figiel@camlintechnologies.com \
--cc=p.lenkow@camlintechnologies.com \
--cc=wright.feng@cypress.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox