From: Lukas Wunner <lukas@wunner•de>
To: Heiner Kallweit <hkallweit1@gmail•com>
Cc: Paolo Abeni <pabeni@redhat•com>,
Eric Dumazet <edumazet@google•com>,
Jakub Kicinski <kuba@kernel•org>,
David Miller <davem@davemloft•net>,
"netdev@vger•kernel.org" <netdev@vger•kernel.org>
Subject: Re: [PATCH net] r8169: fix LED-related deadlock on module removal
Date: Mon, 15 Apr 2024 10:49:11 +0200 [thread overview]
Message-ID: <ZhzqB9_xvEKSkMB7@wunner.de> (raw)
In-Reply-To: <6b6b07f5-250c-415e-bdc4-bd08ac69b24d@gmail.com>
On Mon, Apr 15, 2024 at 08:44:35AM +0200, Heiner Kallweit wrote:
> Binding devm_led_classdev_register() to the netdev is problematic
> because on module removal we get a RTNL-related deadlock.
More precisely the issue is triggered on driver unbind.
Module unload as well as device unplug imply driver unbinding.
> The original change was introduced with 6.8, 6.9 added support for
> LEDs on RTL8125. Therefore the first version of the fix applied on
> 6.9-rc only. This is the modified version for 6.8.
I guess the recipient of this patch should have been the stable
maintainers then, not netdev maintainers.
> --- a/drivers/net/ethernet/realtek/r8169.h
> +++ b/drivers/net/ethernet/realtek/r8169.h
> @@ -72,6 +72,7 @@ enum mac_version {
> };
>
> struct rtl8169_private;
> +struct r8169_led_classdev;
Normally these forward declarations are not needed if you're just
referencing the struct name in a pointer. Usage of the struct name
in a pointer implies a forward declaration.
> +struct r8169_led_classdev *rtl8168_init_leds(struct net_device *ndev)
> {
> - /* bind resource mgmt to netdev */
> - struct device *dev = &ndev->dev;
> struct r8169_led_classdev *leds;
> int i;
>
> - leds = devm_kcalloc(dev, RTL8168_NUM_LEDS, sizeof(*leds), GFP_KERNEL);
> + leds = kcalloc(RTL8168_NUM_LEDS + 1, sizeof(*leds), GFP_KERNEL);
> if (!leds)
> - return;
> + return NULL;
>
> for (i = 0; i < RTL8168_NUM_LEDS; i++)
> rtl8168_setup_ldev(leds + i, ndev, i);
> +
> + return leds;
> +}
If registration of some LEDs fails, you seem to continue driver binding.
So the leds allocation may stick around even if it's not used at all.
Not a big deal, but not super pretty either.
Thanks,
Lukas
next prev parent reply other threads:[~2024-04-15 8:49 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-15 6:44 [PATCH net] r8169: fix LED-related deadlock on module removal Heiner Kallweit
2024-04-15 8:49 ` Lukas Wunner [this message]
2024-04-15 11:54 ` Heiner Kallweit
2024-04-16 23:41 ` Jakub Kicinski
2024-04-17 7:26 ` Lukas Wunner
-- strict thread matches above, loose matches on Subject: below --
2024-04-15 11:57 Heiner Kallweit
2024-04-17 2:34 ` Jakub Kicinski
2024-04-17 6:02 ` Heiner Kallweit
2024-04-17 7:04 ` Greg KH
2024-04-17 7:16 ` Heiner Kallweit
2024-04-17 7:43 ` Greg KH
2024-04-17 22:33 ` Lukas Wunner
2024-04-18 9:55 ` Greg KH
2024-04-18 14:33 ` Heiner Kallweit
2024-04-18 14:44 ` Greg KH
2024-04-17 13:45 ` Jakub Kicinski
2024-04-05 20:29 Heiner Kallweit
2024-04-05 20:50 ` Andrew Lunn
2024-04-05 20:59 ` 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=ZhzqB9_xvEKSkMB7@wunner.de \
--to=lukas@wunner$(echo .)de \
--cc=davem@davemloft$(echo .)net \
--cc=edumazet@google$(echo .)com \
--cc=hkallweit1@gmail$(echo .)com \
--cc=kuba@kernel$(echo .)org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=pabeni@redhat$(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