From: Jakub Kicinski <kuba@kernel.org>
To: Jacob Keller <jacob.e.keller@intel.com>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Re: iAVF circular lock dependency due to netdev_lock
Date: Wed, 5 Feb 2025 17:23:25 -0800 [thread overview]
Message-ID: <20250205172325.5f7c8969@kernel.org> (raw)
In-Reply-To: <81562543-5ea1-4994-9503-90b5ff19b094@intel.com>
On Wed, 5 Feb 2025 15:20:07 -0800 Jacob Keller wrote:
> This happens because the driver takes netdev_lock prior to acquiring its
> own adapter->crit_lock, but then it calls register_netdevice under the
> crit_lock. Since commit 5fda3f35349b ("net: make netdev_lock() protect
> netdev->reg_state"), the register_netdevice() function now acquires
> netdev_lock as part of its flow.
>
> I can fix this by refactoring iavf to only take netdev_lock after
> acquiring its own crit_lock.. but that smells funny. It seems like a
> future change could require to take netdev_lock before calling into the
> driver routines somehow, making that ordering problematic.
>
> I'm not sure how else to fix this... I briefly considered just removing
> crit_lock and relying solely on netdev_lock for synchronization, but
> that doesn't work because of the register_netdevice() taking the lock.
>
> I guess I could do some funky stuff with unlocking but that seems ugly
> as well...
>
> I'm not sure what we should do to fix this.
Not sure either, the locking in this driver is quite odd. Do you know
why it's registering the netdev from a workqueue, and what the entry
points to the driver are?
Normally before the netdev is registered it can't get called, so all
the locking is moot. But IDK if we need to protect from some FW
interactions, maybe?
next prev parent reply other threads:[~2025-02-06 1:23 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-05 23:20 iAVF circular lock dependency due to netdev_lock Jacob Keller
2025-02-06 1:23 ` Jakub Kicinski [this message]
2025-02-06 2:27 ` Jacob Keller
2025-02-06 4:32 ` Jakub Kicinski
2025-02-06 17:18 ` Jacob Keller
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=20250205172325.5f7c8969@kernel.org \
--to=kuba@kernel.org \
--cc=jacob.e.keller@intel.com \
--cc=netdev@vger.kernel.org \
/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;
as well as URLs for NNTP newsgroup(s).