All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Horman <horms@kernel.org>
To: Lukas Wunner <lukas@wunner.de>
Cc: Andrew Lunn <andrew@lunn.ch>,
	Sasha Neftin <sasha.neftin@intel.com>,
	Roman Lozko <lozko.roma@gmail.com>,
	intel-wired-lan@lists.osuosl.org,
	Kurt Kanzenbach <kurt@linutronix.de>,
	Eric Dumazet <edumazet@google.com>,
	Tony Nguyen <anthony.l.nguyen@intel.com>,
	netdev@vger.kernel.org, Jakub Kicinski <kuba@kernel.org>,
	Paolo Abeni <pabeni@redhat.com>,
	"David S. Miller" <davem@davemloft.net>,
	Heiner Kallweit <hkallweit1@gmail.com>
Subject: Re: [Intel-wired-lan] [PATCH net] igc: Fix LED-related deadlock on driver unbind
Date: Tue, 16 Apr 2024 14:51:29 +0100	[thread overview]
Message-ID: <20240416135129.GM2320920@kernel.org> (raw)
In-Reply-To: <2f1be6b1cf2b3346929b0049f2ac7d7d79acb5c9.1713188539.git.lukas@wunner.de>

On Mon, Apr 15, 2024 at 03:48:48PM +0200, Lukas Wunner wrote:
> 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.
> 
> Stack trace for posterity:
> 
>   schedule+0x6e/0xf0
>   schedule_preempt_disabled+0x15/0x20
>   __mutex_lock+0x2a0/0x750
>   unregister_netdevice_notifier+0x40/0x150
>   netdev_trig_deactivate+0x1f/0x60 [ledtrig_netdev]
>   led_trigger_set+0x102/0x330
>   led_classdev_unregister+0x4b/0x110
>   release_nodes+0x3d/0xb0
>   devres_release_all+0x8b/0xc0
>   device_del+0x34f/0x3c0
>   unregister_netdevice_many_notify+0x80b/0xaf0
>   unregister_netdev+0x7c/0xd0
>   igc_remove+0xd8/0x1e0 [igc]
>   pci_device_remove+0x3f/0xb0
> 
> Fixes: ea578703b03d ("igc: Add support for LEDs on i225/i226")
> Reported-by: Roman Lozko <lozko.roma@gmail.com>
> Closes: https://lore.kernel.org/r/CAEhC_B=ksywxCG_+aQqXUrGEgKq+4mqnSV8EBHOKbC3-Obj9+Q@mail.gmail.com/
> Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> Cc: Heiner Kallweit <hkallweit1@gmail.com>

I am aware that Kurt has submitted what appears to be the same patch [1,2],
which I'm inclined to put down to miscommunication (email based workflows
are like that sometimes).

FWIIW, it is my understanding is that the patch originated from
Lukas[3], and thus it seems most appropriate to take his submission.

As for the patch itself, I agree that it addresses the problem at hand.
For the record, I have not tested it.

Reviewed-by: Simon Horman <horms@kernel.org>

[1] [PATCH iwl-net] igc: Fix deadlock on module removal
    https://lore.kernel.org/netdev/20240411-igc_led_deadlock-v1-1-0da98a3c68c5@linutronix.de/
[2] [PATCH iwl-net v2] igc: Fix deadlock on module removal
    https://lore.kernel.org/netdev/20240411-igc_led_deadlock-v2-1-b758c0c88b2b@linutronix.de/
[3] Re: Deadlock in pciehp on dock disconnect
    https://lore.kernel.org/all/ZhBN9p1yOyciXkzw@wunner.de/


WARNING: multiple messages have this Message-ID (diff)
From: Simon Horman <horms@kernel.org>
To: Lukas Wunner <lukas@wunner.de>
Cc: "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Jesse Brandeburg <jesse.brandeburg@intel.com>,
	Tony Nguyen <anthony.l.nguyen@intel.com>,
	intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
	Roman Lozko <lozko.roma@gmail.com>,
	Kurt Kanzenbach <kurt@linutronix.de>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Sasha Neftin <sasha.neftin@intel.com>
Subject: Re: [PATCH net] igc: Fix LED-related deadlock on driver unbind
Date: Tue, 16 Apr 2024 14:51:29 +0100	[thread overview]
Message-ID: <20240416135129.GM2320920@kernel.org> (raw)
In-Reply-To: <2f1be6b1cf2b3346929b0049f2ac7d7d79acb5c9.1713188539.git.lukas@wunner.de>

On Mon, Apr 15, 2024 at 03:48:48PM +0200, Lukas Wunner wrote:
> 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.
> 
> Stack trace for posterity:
> 
>   schedule+0x6e/0xf0
>   schedule_preempt_disabled+0x15/0x20
>   __mutex_lock+0x2a0/0x750
>   unregister_netdevice_notifier+0x40/0x150
>   netdev_trig_deactivate+0x1f/0x60 [ledtrig_netdev]
>   led_trigger_set+0x102/0x330
>   led_classdev_unregister+0x4b/0x110
>   release_nodes+0x3d/0xb0
>   devres_release_all+0x8b/0xc0
>   device_del+0x34f/0x3c0
>   unregister_netdevice_many_notify+0x80b/0xaf0
>   unregister_netdev+0x7c/0xd0
>   igc_remove+0xd8/0x1e0 [igc]
>   pci_device_remove+0x3f/0xb0
> 
> Fixes: ea578703b03d ("igc: Add support for LEDs on i225/i226")
> Reported-by: Roman Lozko <lozko.roma@gmail.com>
> Closes: https://lore.kernel.org/r/CAEhC_B=ksywxCG_+aQqXUrGEgKq+4mqnSV8EBHOKbC3-Obj9+Q@mail.gmail.com/
> Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> Cc: Heiner Kallweit <hkallweit1@gmail.com>

I am aware that Kurt has submitted what appears to be the same patch [1,2],
which I'm inclined to put down to miscommunication (email based workflows
are like that sometimes).

FWIIW, it is my understanding is that the patch originated from
Lukas[3], and thus it seems most appropriate to take his submission.

As for the patch itself, I agree that it addresses the problem at hand.
For the record, I have not tested it.

Reviewed-by: Simon Horman <horms@kernel.org>

[1] [PATCH iwl-net] igc: Fix deadlock on module removal
    https://lore.kernel.org/netdev/20240411-igc_led_deadlock-v1-1-0da98a3c68c5@linutronix.de/
[2] [PATCH iwl-net v2] igc: Fix deadlock on module removal
    https://lore.kernel.org/netdev/20240411-igc_led_deadlock-v2-1-b758c0c88b2b@linutronix.de/
[3] Re: Deadlock in pciehp on dock disconnect
    https://lore.kernel.org/all/ZhBN9p1yOyciXkzw@wunner.de/


  reply	other threads:[~2024-04-16 13:51 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-15 13:48 [Intel-wired-lan] [PATCH net] igc: Fix LED-related deadlock on driver unbind Lukas Wunner
2024-04-15 13:48 ` Lukas Wunner
2024-04-16 13:51 ` Simon Horman [this message]
2024-04-16 13:51   ` Simon Horman
2024-04-16 14:06 ` [Intel-wired-lan] " Kurt Kanzenbach
2024-04-16 14:06   ` Kurt Kanzenbach
2024-04-16 20:55   ` [Intel-wired-lan] " Lukas Wunner
2024-04-16 20:55     ` Lukas Wunner
2024-04-18  9:13 ` [Intel-wired-lan] " naamax.meir
2024-04-18  9:13   ` naamax.meir

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=20240416135129.GM2320920@kernel.org \
    --to=horms@kernel.org \
    --cc=andrew@lunn.ch \
    --cc=anthony.l.nguyen@intel.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=hkallweit1@gmail.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=kuba@kernel.org \
    --cc=kurt@linutronix.de \
    --cc=lozko.roma@gmail.com \
    --cc=lukas@wunner.de \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sasha.neftin@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.