netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net v1 1/1] net: usb: asix: forbid runtime PM to avoid PM/MDIO + RTNL deadlock
@ 2025-09-17  9:54 Oleksij Rempel
  2025-09-17 10:10 ` Oliver Neukum
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Oleksij Rempel @ 2025-09-17  9:54 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: Oleksij Rempel, Hubert Wiśniewski, Marek Szyprowski, stable,
	kernel, linux-kernel, netdev, Lukas Wunner, Russell King, Xu Yang,
	linux-usb

Forbid USB runtime PM (autosuspend) for AX88772* in bind.

usbnet enables runtime PM by default in probe, so disabling it via the
usb_driver flag is ineffective. For AX88772B, autosuspend shows no
measurable power saving in my tests (no link partner, admin up/down).
The ~0.453 W -> ~0.248 W reduction on 6.1 comes from phylib powering
the PHY off on admin-down, not from USB autosuspend.

With autosuspend active, resume paths may require calling phylink/phylib
(caller must hold RTNL) and doing MDIO I/O. Taking RTNL from a USB PM
resume can deadlock (RTNL may already be held), and MDIO can attempt a
runtime-wake while the USB PM lock is held. Given the lack of benefit
and poor test coverage (autosuspend is usually disabled by default in
distros), forbid runtime PM here to avoid these hazards.

This affects only AX88772* devices (per-interface in bind). System
sleep/resume is unchanged.

Fixes: 4a2c7217cd5a ("net: usb: asix: ax88772: manage PHY PM from MAC")
Reported-by: Hubert Wiśniewski <hubert.wisniewski.25632@gmail.com>
Closes: https://lore.kernel.org/all/20220622141638.GE930160@montezuma.acc.umu.se
Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Closes: https://lore.kernel.org/all/b5ea8296-f981-445d-a09a-2f389d7f6fdd@samsung.com
Cc: stable@vger.kernel.org
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
Link to the measurement results:
https://lore.kernel.org/all/aMkPMa650kfKfmF4@pengutronix.de/
---
 drivers/net/usb/asix_devices.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index 792ddda1ad49..0d341d7e6154 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -625,6 +625,22 @@ static void ax88772_suspend(struct usbnet *dev)
 		   asix_read_medium_status(dev, 1));
 }
 
+/*
+ * Notes on PM callbacks and locking context:
+ *
+ * - asix_suspend()/asix_resume() are invoked for both runtime PM and
+ *   system-wide suspend/resume. For struct usb_driver the ->resume()
+ *   callback does not receive pm_message_t, so the resume type cannot
+ *   be distinguished here.
+ *
+ * - The MAC driver must hold RTNL when calling phylink interfaces such as
+ *   phylink_suspend()/resume(). Those calls will also perform MDIO I/O.
+ *
+ * - Taking RTNL and doing MDIO from a runtime-PM resume callback (while
+ *   the USB PM lock is held) is fragile. Since autosuspend brings no
+ *   measurable power saving for this device with current driver version, it is
+ *   disabled below.
+ */
 static int asix_suspend(struct usb_interface *intf, pm_message_t message)
 {
 	struct usbnet *dev = usb_get_intfdata(intf);
@@ -919,6 +935,16 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
 	if (ret)
 		goto initphy_err;
 
+	/* Disable USB runtime PM (autosuspend) for this interface.
+	 * Rationale:
+	 * - No measurable power saving from autosuspend for this device.
+	 * - phylink/phylib calls require caller-held RTNL and do MDIO I/O,
+	 *   which is unsafe from USB PM resume paths (possible RTNL already
+	 *   held, USB PM lock held).
+	 * System suspend/resume is unaffected.
+	 */
+	pm_runtime_forbid(&intf->dev);
+
 	return 0;
 
 initphy_err:
@@ -948,6 +974,10 @@ static void ax88772_unbind(struct usbnet *dev, struct usb_interface *intf)
 	phylink_destroy(priv->phylink);
 	ax88772_mdio_unregister(priv);
 	asix_rx_fixup_common_free(dev->driver_priv);
+	/* Re-allow runtime PM on disconnect for tidiness. The interface
+	 * goes away anyway, but this balances forbid for debug sanity.
+	 */
+	pm_runtime_allow(&intf->dev);
 }
 
 static void ax88178_unbind(struct usbnet *dev, struct usb_interface *intf)
@@ -1600,6 +1630,10 @@ static struct usb_driver asix_driver = {
 	.resume =	asix_resume,
 	.reset_resume =	asix_resume,
 	.disconnect =	usbnet_disconnect,
+	/* usbnet will force supports_autosuspend=1; we explicitly forbid RPM
+	 * per-interface in bind to keep autosuspend disabled for this driver
+	 * by using pm_runtime_forbid().
+	 */
 	.supports_autosuspend = 1,
 	.disable_hub_initiated_lpm = 1,
 };
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH net v1 1/1] net: usb: asix: forbid runtime PM to avoid PM/MDIO + RTNL deadlock
  2025-09-17  9:54 [PATCH net v1 1/1] net: usb: asix: forbid runtime PM to avoid PM/MDIO + RTNL deadlock Oleksij Rempel
@ 2025-09-17 10:10 ` Oliver Neukum
  2025-09-17 11:52   ` Oleksij Rempel
  2025-09-17 11:26 ` Hubert Wiśniewski
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Oliver Neukum @ 2025-09-17 10:10 UTC (permalink / raw)
  To: Oleksij Rempel, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni
  Cc: Hubert Wiśniewski, Marek Szyprowski, stable, kernel,
	linux-kernel, netdev, Lukas Wunner, Russell King, Xu Yang,
	linux-usb

Hi,

On 17.09.25 11:54, Oleksij Rempel wrote:

> With autosuspend active, resume paths may require calling phylink/phylib
> (caller must hold RTNL) and doing MDIO I/O. Taking RTNL from a USB PM
> resume can deadlock (RTNL may already be held), and MDIO can attempt a
> runtime-wake while the USB PM lock is held. Given the lack of benefit
> and poor test coverage (autosuspend is usually disabled by default in
> distros), forbid runtime PM here to avoid these hazards.

This reasoning depends on netif_running() returning false during system resume.
Is that guaranteed?

	Regards
		Oliver


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net v1 1/1] net: usb: asix: forbid runtime PM to avoid PM/MDIO + RTNL deadlock
  2025-09-17  9:54 [PATCH net v1 1/1] net: usb: asix: forbid runtime PM to avoid PM/MDIO + RTNL deadlock Oleksij Rempel
  2025-09-17 10:10 ` Oliver Neukum
@ 2025-09-17 11:26 ` Hubert Wiśniewski
  2025-09-17 13:54 ` Alan Stern
  2025-09-17 18:58 ` Lukas Wunner
  3 siblings, 0 replies; 10+ messages in thread
From: Hubert Wiśniewski @ 2025-09-17 11:26 UTC (permalink / raw)
  To: Oleksij Rempel, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni
  Cc: Marek Szyprowski, stable, kernel, linux-kernel, netdev,
	Lukas Wunner, Russell King, Xu Yang, linux-usb

On Wed Sep 17, 2025 at 11:54 AM CEST, Oleksij Rempel wrote:
> Forbid USB runtime PM (autosuspend) for AX88772* in bind.
>
> usbnet enables runtime PM by default in probe, so disabling it via the
> usb_driver flag is ineffective. For AX88772B, autosuspend shows no
> measurable power saving in my tests (no link partner, admin up/down).
> The ~0.453 W -> ~0.248 W reduction on 6.1 comes from phylib powering
> the PHY off on admin-down, not from USB autosuspend.
>
> With autosuspend active, resume paths may require calling phylink/phylib
> (caller must hold RTNL) and doing MDIO I/O. Taking RTNL from a USB PM
> resume can deadlock (RTNL may already be held), and MDIO can attempt a
> runtime-wake while the USB PM lock is held. Given the lack of benefit
> and poor test coverage (autosuspend is usually disabled by default in
> distros), forbid runtime PM here to avoid these hazards.
>
> This affects only AX88772* devices (per-interface in bind). System
> sleep/resume is unchanged.
>
> Fixes: 4a2c7217cd5a ("net: usb: asix: ax88772: manage PHY PM from MAC")
> Reported-by: Hubert Wiśniewski <hubert.wisniewski.25632@gmail.com>
> Closes: https://lore.kernel.org/all/20220622141638.GE930160@montezuma.acc.umu.se
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Closes: https://lore.kernel.org/all/b5ea8296-f981-445d-a09a-2f389d7f6fdd@samsung.com
> Cc: stable@vger.kernel.org
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
> Link to the measurement results:
> https://lore.kernel.org/all/aMkPMa650kfKfmF4@pengutronix.de/
> ---
>  drivers/net/usb/asix_devices.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>
> diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
> index 792ddda1ad49..0d341d7e6154 100644
> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -625,6 +625,22 @@ static void ax88772_suspend(struct usbnet *dev)
>  		   asix_read_medium_status(dev, 1));
>  }
>  
> +/*
> + * Notes on PM callbacks and locking context:
> + *
> + * - asix_suspend()/asix_resume() are invoked for both runtime PM and
> + *   system-wide suspend/resume. For struct usb_driver the ->resume()
> + *   callback does not receive pm_message_t, so the resume type cannot
> + *   be distinguished here.
> + *
> + * - The MAC driver must hold RTNL when calling phylink interfaces such as
> + *   phylink_suspend()/resume(). Those calls will also perform MDIO I/O.
> + *
> + * - Taking RTNL and doing MDIO from a runtime-PM resume callback (while
> + *   the USB PM lock is held) is fragile. Since autosuspend brings no
> + *   measurable power saving for this device with current driver version, it is
> + *   disabled below.
> + */
>  static int asix_suspend(struct usb_interface *intf, pm_message_t message)
>  {
>  	struct usbnet *dev = usb_get_intfdata(intf);
> @@ -919,6 +935,16 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
>  	if (ret)
>  		goto initphy_err;
>  
> +	/* Disable USB runtime PM (autosuspend) for this interface.
> +	 * Rationale:
> +	 * - No measurable power saving from autosuspend for this device.
> +	 * - phylink/phylib calls require caller-held RTNL and do MDIO I/O,
> +	 *   which is unsafe from USB PM resume paths (possible RTNL already
> +	 *   held, USB PM lock held).
> +	 * System suspend/resume is unaffected.
> +	 */
> +	pm_runtime_forbid(&intf->dev);
> +
>  	return 0;
>  
>  initphy_err:
> @@ -948,6 +974,10 @@ static void ax88772_unbind(struct usbnet *dev, struct usb_interface *intf)
>  	phylink_destroy(priv->phylink);
>  	ax88772_mdio_unregister(priv);
>  	asix_rx_fixup_common_free(dev->driver_priv);
> +	/* Re-allow runtime PM on disconnect for tidiness. The interface
> +	 * goes away anyway, but this balances forbid for debug sanity.
> +	 */
> +	pm_runtime_allow(&intf->dev);
>  }
>  
>  static void ax88178_unbind(struct usbnet *dev, struct usb_interface *intf)
> @@ -1600,6 +1630,10 @@ static struct usb_driver asix_driver = {
>  	.resume =	asix_resume,
>  	.reset_resume =	asix_resume,
>  	.disconnect =	usbnet_disconnect,
> +	/* usbnet will force supports_autosuspend=1; we explicitly forbid RPM
> +	 * per-interface in bind to keep autosuspend disabled for this driver
> +	 * by using pm_runtime_forbid().
> +	 */
>  	.supports_autosuspend = 1,
>  	.disable_hub_initiated_lpm = 1,
>  };

Well, this fixes the issue for me. No suspend/resume -- no deadlock -- no
problem. Thanks.

Tested-by: Hubert Wiśniewski <hubert.wisniewski.25632@gmail.com>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net v1 1/1] net: usb: asix: forbid runtime PM to avoid PM/MDIO + RTNL deadlock
  2025-09-17 10:10 ` Oliver Neukum
@ 2025-09-17 11:52   ` Oleksij Rempel
  2025-09-17 12:25     ` Oliver Neukum
  0 siblings, 1 reply; 10+ messages in thread
From: Oleksij Rempel @ 2025-09-17 11:52 UTC (permalink / raw)
  To: Oliver Neukum
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Hubert Wiśniewski, Marek Szyprowski, stable,
	kernel, linux-kernel, netdev, Lukas Wunner, Russell King, Xu Yang,
	linux-usb

Hi Oliver,

On Wed, Sep 17, 2025 at 12:10:48PM +0200, Oliver Neukum wrote:
> Hi,
> 
> On 17.09.25 11:54, Oleksij Rempel wrote:
> 
> > With autosuspend active, resume paths may require calling phylink/phylib
> > (caller must hold RTNL) and doing MDIO I/O. Taking RTNL from a USB PM
> > resume can deadlock (RTNL may already be held), and MDIO can attempt a
> > runtime-wake while the USB PM lock is held. Given the lack of benefit
> > and poor test coverage (autosuspend is usually disabled by default in
> > distros), forbid runtime PM here to avoid these hazards.
> 
> This reasoning depends on netif_running() returning false during system resume.
> Is that guaranteed?

You’re right - there is no guarantee that netif_running() is false
during system resume. This change does not rely on that. If my wording
suggested otherwise, I’ll reword the commit message to make it explicit.

1) Runtime PM (autosuspend/autoresume)

Typical chain when user does ip link set dev <if> up while autosuspended:
rtnl_newlink (RTNL held)
  -> __dev_open -> usbnet_open
     -> usb_autopm_get_interface -> __pm_runtime_resume
        -> usb_resume_interface -> asix_resume

Here resume happens synchronously under RTNL (and with USB PM locking). If the
driver then calls phylink/phylib from resume (caller must hold RTNL; MDIO I/O),
we can deadlock or hit PM-lock vs MDIO wake issues.

Patch effect:
I forbid runtime PM per-interface in ax88772_bind(). This removes the
synchronous autoresume path.

2) System suspend/resume

Typical chain:
... dpm_run_callback (workqueue)
 -> usb_resume_interface -> asix_resume

This is not under RTNL, and no pm_runtime locking is involved. The patch does
not change this path and makes no assumption about netif_running() here.

If helpful, I can rework the commit message.

Best Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net v1 1/1] net: usb: asix: forbid runtime PM to avoid PM/MDIO + RTNL deadlock
  2025-09-17 11:52   ` Oleksij Rempel
@ 2025-09-17 12:25     ` Oliver Neukum
  0 siblings, 0 replies; 10+ messages in thread
From: Oliver Neukum @ 2025-09-17 12:25 UTC (permalink / raw)
  To: Oleksij Rempel, Oliver Neukum
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Hubert Wiśniewski, Marek Szyprowski, stable,
	kernel, linux-kernel, netdev, Lukas Wunner, Russell King, Xu Yang,
	linux-usb



On 17.09.25 13:52, Oleksij Rempel wrote:
> Hi Oliver,
> 
> On Wed, Sep 17, 2025 at 12:10:48PM +0200, Oliver Neukum wrote:
>> Hi,
>>
>> On 17.09.25 11:54, Oleksij Rempel wrote:
>>
>>> With autosuspend active, resume paths may require calling phylink/phylib
>>> (caller must hold RTNL) and doing MDIO I/O. Taking RTNL from a USB PM

This very strongly suggested that the conditional call is the issue.

>>> resume can deadlock (RTNL may already be held), and MDIO can attempt a
>>> runtime-wake while the USB PM lock is held. Given the lack of benefit
>>> and poor test coverage (autosuspend is usually disabled by default in
>>> distros), forbid runtime PM here to avoid these hazards.
>>
>> This reasoning depends on netif_running() returning false during system resume.
>> Is that guaranteed?
> 
> You’re right - there is no guarantee that netif_running() is false
> during system resume. This change does not rely on that. If my wording
> suggested otherwise, I’ll reword the commit message to make it explicit.
> 
> 1) Runtime PM (autosuspend/autoresume)
> 
> Typical chain when user does ip link set dev <if> up while autosuspended:
> rtnl_newlink (RTNL held)
>    -> __dev_open -> usbnet_open
>       -> usb_autopm_get_interface -> __pm_runtime_resume
>          -> usb_resume_interface -> asix_resume
> 
> Here resume happens synchronously under RTNL (and with USB PM locking). If the
> driver then calls phylink/phylib from resume (caller must hold RTNL; MDIO I/O),
> we can deadlock or hit PM-lock vs MDIO wake issues.
> 
> Patch effect:
> I forbid runtime PM per-interface in ax88772_bind(). This removes the
> synchronous autoresume path.
> 
> 2) System suspend/resume
> 
> Typical chain:
> ... dpm_run_callback (workqueue)
>   -> usb_resume_interface -> asix_resume
> 
> This is not under RTNL, and no pm_runtime locking is involved. The patch does
> not change this path and makes no assumption about netif_running() here.
> 
> If helpful, I can rework the commit message.

It would maybe good to include a wording like:

With runtime PM, the driver is forced to resume its device while
holding RTNL, if it happens to be suspended. The methods needed
to resume the device take RTNL themselves. Thus runtime PM will deadlock.


	Regards
		Oliver



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net v1 1/1] net: usb: asix: forbid runtime PM to avoid PM/MDIO + RTNL deadlock
  2025-09-17  9:54 [PATCH net v1 1/1] net: usb: asix: forbid runtime PM to avoid PM/MDIO + RTNL deadlock Oleksij Rempel
  2025-09-17 10:10 ` Oliver Neukum
  2025-09-17 11:26 ` Hubert Wiśniewski
@ 2025-09-17 13:54 ` Alan Stern
  2025-09-17 14:31   ` Hubert Wiśniewski
  2025-09-17 18:58 ` Lukas Wunner
  3 siblings, 1 reply; 10+ messages in thread
From: Alan Stern @ 2025-09-17 13:54 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Hubert Wiśniewski, Marek Szyprowski, stable,
	kernel, linux-kernel, netdev, Lukas Wunner, Russell King, Xu Yang,
	linux-usb

On Wed, Sep 17, 2025 at 11:54:57AM +0200, Oleksij Rempel wrote:
> Forbid USB runtime PM (autosuspend) for AX88772* in bind.
> 
> usbnet enables runtime PM by default in probe, so disabling it via the
> usb_driver flag is ineffective. For AX88772B, autosuspend shows no
> measurable power saving in my tests (no link partner, admin up/down).
> The ~0.453 W -> ~0.248 W reduction on 6.1 comes from phylib powering
> the PHY off on admin-down, not from USB autosuspend.
> 
> With autosuspend active, resume paths may require calling phylink/phylib
> (caller must hold RTNL) and doing MDIO I/O. Taking RTNL from a USB PM
> resume can deadlock (RTNL may already be held), and MDIO can attempt a
> runtime-wake while the USB PM lock is held. Given the lack of benefit
> and poor test coverage (autosuspend is usually disabled by default in
> distros), forbid runtime PM here to avoid these hazards.
> 
> This affects only AX88772* devices (per-interface in bind). System
> sleep/resume is unchanged.

> @@ -919,6 +935,16 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
>  	if (ret)
>  		goto initphy_err;
>  
> +	/* Disable USB runtime PM (autosuspend) for this interface.
> +	 * Rationale:
> +	 * - No measurable power saving from autosuspend for this device.
> +	 * - phylink/phylib calls require caller-held RTNL and do MDIO I/O,
> +	 *   which is unsafe from USB PM resume paths (possible RTNL already
> +	 *   held, USB PM lock held).
> +	 * System suspend/resume is unaffected.
> +	 */
> +	pm_runtime_forbid(&intf->dev);

Are you aware that the action of pm_runtime_forbid() can be reversed by 
the user (by writing "auto" to the .../power/control sysfs file)?

To prevent the user from re-enabling runtime PM, you should call 
pm_runtime_get_noresume() (and then of course pm_runtime_put() or 
equivalent while unbinding).

Alan Stern

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net v1 1/1] net: usb: asix: forbid runtime PM to avoid PM/MDIO + RTNL deadlock
  2025-09-17 13:54 ` Alan Stern
@ 2025-09-17 14:31   ` Hubert Wiśniewski
  2025-09-17 14:57     ` Alan Stern
  0 siblings, 1 reply; 10+ messages in thread
From: Hubert Wiśniewski @ 2025-09-17 14:31 UTC (permalink / raw)
  To: Alan Stern, Oleksij Rempel
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Marek Szyprowski, stable, kernel, linux-kernel,
	netdev, Lukas Wunner, Russell King, Xu Yang, linux-usb

On Wed Sep 17, 2025 at 3:54 PM CEST, Alan Stern wrote:
> On Wed, Sep 17, 2025 at 11:54:57AM +0200, Oleksij Rempel wrote:
>> Forbid USB runtime PM (autosuspend) for AX88772* in bind.
>> 
>> usbnet enables runtime PM by default in probe, so disabling it via the
>> usb_driver flag is ineffective. For AX88772B, autosuspend shows no
>> measurable power saving in my tests (no link partner, admin up/down).
>> The ~0.453 W -> ~0.248 W reduction on 6.1 comes from phylib powering
>> the PHY off on admin-down, not from USB autosuspend.
>> 
>> With autosuspend active, resume paths may require calling phylink/phylib
>> (caller must hold RTNL) and doing MDIO I/O. Taking RTNL from a USB PM
>> resume can deadlock (RTNL may already be held), and MDIO can attempt a
>> runtime-wake while the USB PM lock is held. Given the lack of benefit
>> and poor test coverage (autosuspend is usually disabled by default in
>> distros), forbid runtime PM here to avoid these hazards.
>> 
>> This affects only AX88772* devices (per-interface in bind). System
>> sleep/resume is unchanged.
>
>> @@ -919,6 +935,16 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
>>  	if (ret)
>>  		goto initphy_err;
>>  
>> +	/* Disable USB runtime PM (autosuspend) for this interface.
>> +	 * Rationale:
>> +	 * - No measurable power saving from autosuspend for this device.
>> +	 * - phylink/phylib calls require caller-held RTNL and do MDIO I/O,
>> +	 *   which is unsafe from USB PM resume paths (possible RTNL already
>> +	 *   held, USB PM lock held).
>> +	 * System suspend/resume is unaffected.
>> +	 */
>> +	pm_runtime_forbid(&intf->dev);
>
> Are you aware that the action of pm_runtime_forbid() can be reversed by 
> the user (by writing "auto" to the .../power/control sysfs file)?

I have tested this. With this patch, it seems that writing "auto" to
power/control has no effect -- power/runtime_status remains "active" and
the device does not get suspended. But maybe there is a way to force the
suspension anyway?

> To prevent the user from re-enabling runtime PM, you should call 
> pm_runtime_get_noresume() (and then of course pm_runtime_put() or 
> equivalent while unbinding).
>
> Alan Stern


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net v1 1/1] net: usb: asix: forbid runtime PM to avoid PM/MDIO + RTNL deadlock
  2025-09-17 14:31   ` Hubert Wiśniewski
@ 2025-09-17 14:57     ` Alan Stern
  0 siblings, 0 replies; 10+ messages in thread
From: Alan Stern @ 2025-09-17 14:57 UTC (permalink / raw)
  To: Hubert Wiśniewski
  Cc: Oleksij Rempel, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Marek Szyprowski, stable, kernel,
	linux-kernel, netdev, Lukas Wunner, Russell King, Xu Yang,
	linux-usb

On Wed, Sep 17, 2025 at 04:31:40PM +0200, Hubert Wiśniewski wrote:
> On Wed Sep 17, 2025 at 3:54 PM CEST, Alan Stern wrote:
> > Are you aware that the action of pm_runtime_forbid() can be reversed by 
> > the user (by writing "auto" to the .../power/control sysfs file)?
> 
> I have tested this. With this patch, it seems that writing "auto" to
> power/control has no effect -- power/runtime_status remains "active" and
> the device does not get suspended. But maybe there is a way to force the
> suspension anyway?

I don't know exactly what's going on in your particular case.  However, 
if you read the source code for control_store() in 
drivers/base/power/sysfs.c, you'll see that writing "auto" to the 
attribute file causes the function to call pm_runtime_allow().

If you turn on CONFIG_PM_ADVANCED_DEBUG there will be extra files in the 
.../power/ directory, showing some of the other runtime PM values.  
Perhaps they will help you to figure out what's happening.

Alan Stern

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net v1 1/1] net: usb: asix: forbid runtime PM to avoid PM/MDIO + RTNL deadlock
  2025-09-17  9:54 [PATCH net v1 1/1] net: usb: asix: forbid runtime PM to avoid PM/MDIO + RTNL deadlock Oleksij Rempel
                   ` (2 preceding siblings ...)
  2025-09-17 13:54 ` Alan Stern
@ 2025-09-17 18:58 ` Lukas Wunner
  2025-09-22  7:11   ` Oleksij Rempel
  3 siblings, 1 reply; 10+ messages in thread
From: Lukas Wunner @ 2025-09-17 18:58 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Hubert Wi??niewski, Marek Szyprowski, stable, kernel,
	linux-kernel, netdev, Russell King, Xu Yang, linux-usb

On Wed, Sep 17, 2025 at 11:54:57AM +0200, Oleksij Rempel wrote:
> Forbid USB runtime PM (autosuspend) for AX88772* in bind.
[...]
> With autosuspend active, resume paths may require calling phylink/phylib
> (caller must hold RTNL) and doing MDIO I/O. Taking RTNL from a USB PM
> resume can deadlock (RTNL may already be held), and MDIO can attempt a
> runtime-wake while the USB PM lock is held.

FWIW, for smsc95xx.c, the MDIO deadlock issue was resolved by commit
7b960c967f2a ("usbnet: smsc95xx: Fix deadlock on runtime resume").
I would assume that something similar would be possible for
asix_devices.c as well.

Thanks,

Lukas

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net v1 1/1] net: usb: asix: forbid runtime PM to avoid PM/MDIO + RTNL deadlock
  2025-09-17 18:58 ` Lukas Wunner
@ 2025-09-22  7:11   ` Oleksij Rempel
  0 siblings, 0 replies; 10+ messages in thread
From: Oleksij Rempel @ 2025-09-22  7:11 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Hubert Wi??niewski, Marek Szyprowski, stable, kernel,
	linux-kernel, netdev, Russell King, Xu Yang, linux-usb

Hi Lukas,

On Wed, Sep 17, 2025 at 08:58:17PM +0200, Lukas Wunner wrote:
> On Wed, Sep 17, 2025 at 11:54:57AM +0200, Oleksij Rempel wrote:
> > Forbid USB runtime PM (autosuspend) for AX88772* in bind.
> [...]
> > With autosuspend active, resume paths may require calling phylink/phylib
> > (caller must hold RTNL) and doing MDIO I/O. Taking RTNL from a USB PM
> > resume can deadlock (RTNL may already be held), and MDIO can attempt a
> > runtime-wake while the USB PM lock is held.
> 
> FWIW, for smsc95xx.c, the MDIO deadlock issue was resolved by commit
> 7b960c967f2a ("usbnet: smsc95xx: Fix deadlock on runtime resume").
> I would assume that something similar would be possible for
> asix_devices.c as well.

thanks for the recommendation.

Right now I’m juggling two goals:
- Stable: provide a simple and robust fix with minimal risk.
- net-next: design a clean and reliable solution that keeps autosuspend
  working.

For -stable, keeping autosuspend disabled per AX88772* seems to be the
most straightforward and low-risk way to avoid the problematic
autoresume path. Autosuspend isn’t on by default in most distros anyway,
so the behavioral impact is minimal.

If we keep autosuspend enabled, the driver has to be careful about
multiple contexts:
- Runtime PM callbacks: asix_{suspend,resume,reset_resume} running under
the USB PM lock.

- System sleep/resume: asix_resume() via dpm_run_callback() workqueues (no
pm_runtime involved).

- ndo_open() path (RTNL held): usbnet_open() -> usb_autopm_get_interface()
  -> synchronous autoresume into asix_resume().

- ethtool / netlink control paths: often under RTNL, may trigger
  autoresume and/or MDIO via phylib.

- phylink/MAC ops: may touch MDIO; caller is expected to hold RTNL.

- status URB / NAPI: atomic/BH context (no sleeping, no RTNL).

If maintainers prefer attempting the smsc95xx-style change right away, I
can draft it; my preference for -stable is still the minimal forbid to
limit churn.

Best Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-09-22  7:11 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-17  9:54 [PATCH net v1 1/1] net: usb: asix: forbid runtime PM to avoid PM/MDIO + RTNL deadlock Oleksij Rempel
2025-09-17 10:10 ` Oliver Neukum
2025-09-17 11:52   ` Oleksij Rempel
2025-09-17 12:25     ` Oliver Neukum
2025-09-17 11:26 ` Hubert Wiśniewski
2025-09-17 13:54 ` Alan Stern
2025-09-17 14:31   ` Hubert Wiśniewski
2025-09-17 14:57     ` Alan Stern
2025-09-17 18:58 ` Lukas Wunner
2025-09-22  7:11   ` Oleksij Rempel

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).