From: Jacob Keller <jacob.e.keller@intel•com>
To: "Marek Marczykowski-Górecki" <marmarek@invisiblethingslab•com>
Cc: Tony Nguyen <anthony.l.nguyen@intel•com>, <davem@davemloft•net>,
<kuba@kernel•org>, <pabeni@redhat•com>, <edumazet@google•com>,
<netdev@vger•kernel.org>, Lukas Wunner <lukas@wunner•de>,
<sasha.neftin@intel•com>, Roman Lozko <lozko.roma@gmail•com>,
Kurt Kanzenbach <kurt@linutronix•de>,
Heiner Kallweit <hkallweit1@gmail•com>,
Simon Horman <horms@kernel•org>,
Naama Meir <naamax.meir@linux•intel.com>
Subject: Re: [PATCH net] igc: Fix LED-related deadlock on driver unbind
Date: Mon, 22 Apr 2024 16:46:28 -0700 [thread overview]
Message-ID: <a356d2a0-e573-4e31-bae3-2a361476f937@intel.com> (raw)
In-Reply-To: <Zib0veVgvgTg7Mq6@mail-itl>
On 4/22/2024 4:37 PM, Marek Marczykowski-Górecki wrote:
> On Mon, Apr 22, 2024 at 04:32:01PM -0700, Jacob Keller wrote:
>> On 4/22/2024 1:45 PM, Tony Nguyen wrote:
>>> From: Lukas Wunner <lukas@wunner•de>
>>>
>>> Roman reports a deadlock on unplug of a Thunderbolt docking station
>>> containing an Intel I225 Ethernet adapter.
>>>
>>> The root cause is that led_classdev's for LEDs on the adapter are
>>> registered such that they're device-managed by the netdev. That
>>> results in recursive acquisition of the rtnl_lock() mutex on unplug:
>>>
>>> When the driver calls unregister_netdev(), it acquires rtnl_lock(),
>>> then frees the device-managed resources. Upon unregistering the LEDs,
>>> netdev_trig_deactivate() invokes unregister_netdevice_notifier(),
>>> which tries to acquire rtnl_lock() again.
>>>
>>> Avoid by using non-device-managed LED registration.
>>
>> Could we instead switch to using devm with the PCI device struct instead
>> of the netdev struct? That would make it still get automatically cleaned
>> up, but by cleaning it up only when the PCIe device goes away, which
>> should be after rtnl_lock() is released..
>
> Wouldn't that effectively leak memory if driver is unbound from the
> device and then bound back (and possibly repeated multiple times)?
>
My understanding of devm is that when you unload the driver it calls the
devm teardowns so you only leak until driver remove.
In the netdev case, you're releasing during unregister_netdev() instead
of at the end of the .remove() callback of the PCI driver.
To me, using devm from the PCI device should be equivalent to managing
it manually within the igc_remove() function.
I could be mis-understanding how devm works, or the order and flow for
how and when igc allocates these?
next prev parent reply other threads:[~2024-04-22 23:46 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-22 20:45 [PATCH net] igc: Fix LED-related deadlock on driver unbind Tony Nguyen
2024-04-22 23:32 ` Jacob Keller
2024-04-22 23:37 ` Marek Marczykowski-Górecki
2024-04-22 23:46 ` Jacob Keller [this message]
2024-04-23 8:08 ` Lukas Wunner
2024-04-23 7:53 ` Lukas Wunner
2024-04-25 3:40 ` patchwork-bot+netdevbpf
-- strict thread matches above, loose matches on Subject: below --
2024-04-15 13:48 Lukas Wunner
2024-04-16 13:51 ` Simon Horman
2024-04-16 14:06 ` Kurt Kanzenbach
2024-04-16 20:55 ` Lukas Wunner
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=a356d2a0-e573-4e31-bae3-2a361476f937@intel.com \
--to=jacob.e.keller@intel$(echo .)com \
--cc=anthony.l.nguyen@intel$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=edumazet@google$(echo .)com \
--cc=hkallweit1@gmail$(echo .)com \
--cc=horms@kernel$(echo .)org \
--cc=kuba@kernel$(echo .)org \
--cc=kurt@linutronix$(echo .)de \
--cc=lozko.roma@gmail$(echo .)com \
--cc=lukas@wunner$(echo .)de \
--cc=marmarek@invisiblethingslab$(echo .)com \
--cc=naamax.meir@linux$(echo .)intel.com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=pabeni@redhat$(echo .)com \
--cc=sasha.neftin@intel$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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