DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 4E4AF61779
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: Re: [PATCH] brcmfmac: convert dev_init_lock mutex to completion
From:   Kalle Valo <kvalo@codeaurora.org>
In-Reply-To: <1552470749-3625-1-git-send-email-p.figiel@camlintechnologies.com>
References: <1552470749-3625-1-git-send-email-p.figiel@camlintechnologies.com>
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>,
        =?iso-8859-2?q?Krzysztof_Drobi=F1ski?= 
        <k.drobinski@camlintechnologies.com>,
        Pawel Lenkow <p.lenkow@camlintechnologies.com>,
        Lech Perczak <l.perczak@camlintechnologies.com>,
        Piotr Figiel <p.figiel@camlintechnologies.com>
User-Agent: pwcli/0.0.0-git (https://github.com/kvalo/pwcli/) Python/2.7.12
Message-Id: <20190404101300.9A2E661A26@smtp.codeaurora.org>
Date:   Thu,  4 Apr 2019 10:13:00 +0000 (UTC)
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

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