Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Sender: kvalo=codeaurora.org@mg.codeaurora.org
DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org C1A31C433CA
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: Re: [PATCH v2] iwlwifi: don't call netif_napi_add() with rxq->lock
 held
 (was Re: Lockdep warning in iwl_pcie_rx_handle())
From:   Kalle Valo <kvalo@codeaurora.org>
In-Reply-To: <nycvar.YFH.7.76.2103021134060.12405@cbobk.fhfr.pm>
References: <nycvar.YFH.7.76.2103021134060.12405@cbobk.fhfr.pm>
To:     Jiri Kosina <jikos@kernel.org>
Cc:     Johannes Berg <johannes@sipsolutions.net>,
        Luca Coelho <luciano.coelho@intel.com>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org
User-Agent: pwcli/0.1.0-git (https://github.com/kvalo/pwcli/) Python/3.5.2
Message-Id: <20210303155940.C0819C433ED@smtp.codeaurora.org>
Date:   Wed,  3 Mar 2021 15:59:40 +0000 (UTC)
Precedence: bulk

Jiri Kosina <jikos@kernel.org> wrote:

> From: Jiri Kosina <jkosina@suse.cz>
> 
> We can't call netif_napi_add() with rxq-lock held, as there is a potential
> for deadlock as spotted by lockdep (see below). rxq->lock is not
> protecting anything over the netif_napi_add() codepath anyway, so let's
> drop it just before calling into NAPI.
> 
>  ========================================================
>  WARNING: possible irq lock inversion dependency detected
>  5.12.0-rc1-00002-gbada49429032 #5 Not tainted
>  --------------------------------------------------------
>  irq/136-iwlwifi/565 just changed the state of lock:
>  ffff89f28433b0b0 (&rxq->lock){+.-.}-{2:2}, at: iwl_pcie_rx_handle+0x7f/0x960 [iwlwifi]
>  but this lock took another, SOFTIRQ-unsafe lock in the past:
>   (napi_hash_lock){+.+.}-{2:2}
> 
>  and interrupts could create inverse lock ordering between them.
> 
>  other info that might help us debug this:
>   Possible interrupt unsafe locking scenario:
> 
>         CPU0                    CPU1
>         ----                    ----
>    lock(napi_hash_lock);
>                                 local_irq_disable();
>                                 lock(&rxq->lock);
>                                 lock(napi_hash_lock);
>    <Interrupt>
>      lock(&rxq->lock);
> 
>   *** DEADLOCK ***
> 
>  1 lock held by irq/136-iwlwifi/565:
>   #0: ffff89f2b1440170 (sync_cmd_lockdep_map){+.+.}-{0:0}, at: iwl_pcie_irq_handler+0x5/0xb30
> 
>  the shortest dependencies between 2nd lock and 1st lock:
>   -> (napi_hash_lock){+.+.}-{2:2} {
>      HARDIRQ-ON-W at:
>                        lock_acquire+0x277/0x3d0
>                        _raw_spin_lock+0x2c/0x40
>                        netif_napi_add+0x14b/0x270
>                        e1000_probe+0x2fe/0xee0 [e1000e]
>                        local_pci_probe+0x42/0x90
>                        pci_device_probe+0x10b/0x1c0
>                        really_probe+0xef/0x4b0
>                        driver_probe_device+0xde/0x150
>                        device_driver_attach+0x4f/0x60
>                        __driver_attach+0x9c/0x140
>                        bus_for_each_dev+0x79/0xc0
>                        bus_add_driver+0x18d/0x220
>                        driver_register+0x5b/0xf0
>                        do_one_initcall+0x5b/0x300
>                        do_init_module+0x5b/0x21c
>                        load_module+0x1dae/0x22c0
>                        __do_sys_finit_module+0xad/0x110
>                        do_syscall_64+0x33/0x80
>                        entry_SYSCALL_64_after_hwframe+0x44/0xae
>      SOFTIRQ-ON-W at:
>                        lock_acquire+0x277/0x3d0
>                        _raw_spin_lock+0x2c/0x40
>                        netif_napi_add+0x14b/0x270
>                        e1000_probe+0x2fe/0xee0 [e1000e]
>                        local_pci_probe+0x42/0x90
>                        pci_device_probe+0x10b/0x1c0
>                        really_probe+0xef/0x4b0
>                        driver_probe_device+0xde/0x150
>                        device_driver_attach+0x4f/0x60
>                        __driver_attach+0x9c/0x140
>                        bus_for_each_dev+0x79/0xc0
>                        bus_add_driver+0x18d/0x220
>                        driver_register+0x5b/0xf0
>                        do_one_initcall+0x5b/0x300
>                        do_init_module+0x5b/0x21c
>                        load_module+0x1dae/0x22c0
>                        __do_sys_finit_module+0xad/0x110
>                        do_syscall_64+0x33/0x80
>                        entry_SYSCALL_64_after_hwframe+0x44/0xae
>      INITIAL USE at:
>                       lock_acquire+0x277/0x3d0
>                       _raw_spin_lock+0x2c/0x40
>                       netif_napi_add+0x14b/0x270
>                       e1000_probe+0x2fe/0xee0 [e1000e]
>                       local_pci_probe+0x42/0x90
>                       pci_device_probe+0x10b/0x1c0
>                       really_probe+0xef/0x4b0
>                       driver_probe_device+0xde/0x150
>                       device_driver_attach+0x4f/0x60
>                       __driver_attach+0x9c/0x140
>                       bus_for_each_dev+0x79/0xc0
>                       bus_add_driver+0x18d/0x220
>                       driver_register+0x5b/0xf0
>                       do_one_initcall+0x5b/0x300
>                       do_init_module+0x5b/0x21c
>                       load_module+0x1dae/0x22c0
>                       __do_sys_finit_module+0xad/0x110
>                       do_syscall_64+0x33/0x80
>                       entry_SYSCALL_64_after_hwframe+0x44/0xae
>    }
>    ... key      at: [<ffffffffae84ef38>] napi_hash_lock+0x18/0x40
>    ... acquired at:
>     _raw_spin_lock+0x2c/0x40
>     netif_napi_add+0x14b/0x270
>     _iwl_pcie_rx_init+0x1f4/0x710 [iwlwifi]
>     iwl_pcie_rx_init+0x1b/0x3b0 [iwlwifi]
>     iwl_trans_pcie_start_fw+0x2ac/0x6a0 [iwlwifi]
>     iwl_mvm_load_ucode_wait_alive+0x116/0x460 [iwlmvm]
>     iwl_run_init_mvm_ucode+0xa4/0x3a0 [iwlmvm]
>     iwl_op_mode_mvm_start+0x9ed/0xbf0 [iwlmvm]
>     _iwl_op_mode_start.isra.4+0x42/0x80 [iwlwifi]
>     iwl_opmode_register+0x71/0xe0 [iwlwifi]
>     iwl_mvm_init+0x34/0x1000 [iwlmvm]
>     do_one_initcall+0x5b/0x300
>     do_init_module+0x5b/0x21c
>     load_module+0x1dae/0x22c0
>     __do_sys_finit_module+0xad/0x110
>     do_syscall_64+0x33/0x80
>     entry_SYSCALL_64_after_hwframe+0x44/0xae
> 
> [ ... lockdep output trimmed .... ]
> 
> Fixes: 25edc8f259c7106 ("iwlwifi: pcie: properly implement NAPI")
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> Acked-by: Luca Coelho <luciano.coelho@intel.com>

Patch applied to wireless-drivers.git, thanks.

295d4cd82b01 iwlwifi: don't call netif_napi_add() with rxq->lock held (was Re: Lockdep warning in iwl_pcie_rx_handle())

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/nycvar.YFH.7.76.2103021134060.12405@cbobk.fhfr.pm/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches