All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kalle Valo <kvalo@codeaurora.org>
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
Subject: Re: [PATCH v2] iwlwifi: don't call netif_napi_add() with rxq->lock held (was Re: Lockdep warning in iwl_pcie_rx_handle())
Date: Wed,  3 Mar 2021 15:59:40 +0000 (UTC)	[thread overview]
Message-ID: <20210303155940.C0819C433ED@smtp.codeaurora.org> (raw)
In-Reply-To: <nycvar.YFH.7.76.2103021134060.12405@cbobk.fhfr.pm>

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


  parent reply	other threads:[~2021-03-03 21:52 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-01 20:44 Lockdep warning in iwl_pcie_rx_handle() Jiri Kosina
2021-03-01 21:09 ` Jiri Kosina
2021-03-01 21:12 ` Johannes Berg
2021-03-02  9:27   ` [PATCH] iwlwifi: don't call netif_napi_add() with rxq->lock held (was Re: Lockdep warning in iwl_pcie_rx_handle()) Jiri Kosina
2021-03-02 10:34     ` [PATCH v2] " Jiri Kosina
2021-03-02 10:49       ` Coelho, Luciano
2021-03-02 12:17         ` Kalle Valo
2021-03-02 14:55           ` Jiri Kosina
2021-03-03 21:39           ` Sedat Dilek
2021-03-03 15:59       ` Kalle Valo [this message]
     [not found]       ` <20210303155941.25521C43463@smtp.codeaurora.org>
2021-03-03 18:01         ` Jiri Kosina
2021-03-03 18:33           ` Kalle Valo
2021-03-03 19:19             ` Jiri Kosina
2021-03-02 10:48     ` [PATCH] " Coelho, Luciano

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=20210303155940.C0819C433ED@smtp.codeaurora.org \
    --to=kvalo@codeaurora.org \
    --cc=hkallweit1@gmail.com \
    --cc=jikos@kernel.org \
    --cc=johannes@sipsolutions.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=luciano.coelho@intel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.