All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: "Nikola Z. Ivanov" <zlatistiv@gmail.com>,
	Dmitry Bezrukov <dbezrukov@marvell.com>,
	Igor Russkikh <irusskikh@marvell.com>
Cc: andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
	pabeni@redhat.com, enelsonmoore@gmail.com, kees@kernel.org,
	oneukum@suse.com, n.zhandarovich@fintech.ru,
	linux-usb@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	syzbot+48dc1e8dfc92faf1124c@syzkaller.appspotmail.com
Subject: Re: [PATCH net] net: usb: aqc111: Do not perform PM inside runtime suspend callback
Date: Fri, 6 Mar 2026 16:43:51 -0800	[thread overview]
Message-ID: <20260306164351.503e8540@kernel.org> (raw)
In-Reply-To: <20260304155734.110734-1-zlatistiv@gmail.com>

On Wed,  4 Mar 2026 17:57:34 +0200 Nikola Z. Ivanov wrote:
> syzbot reports "task hung in rpm_resume"
> 
> This is caused by aqc111_suspend calling
> the PM variant of its write_cmd routine.
> 
> The simplified call trace looks like this:
> 
> rpm_suspend()
>   usb_suspend_both() - here udev->dev.power.runtime_status == RPM_SUSPENDING
>     aqc111_suspend() - called for the usb device interface
>       aqc111_write32_cmd()
>         usb_autopm_get_interface()
>           pm_runtime_resume_and_get()
>             rpm_resume() - here we call rpm_resume() on our parent
>               rpm_resume() - Here we wait for a status change that will never happen.
> 
> At this point we block another task which holds
> rtnl_lock and locks up the whole networking stack.
> 
> Fix this by replacing the write_cmd calls with their _nopm variants
> in the case where we are inside a runtime suspend call.
> 
> Reported-by: syzbot+48dc1e8dfc92faf1124c@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=48dc1e8dfc92faf1124c
> Fixes: e58ba4544c77 ("net: usb: aqc111: Add support for wake on LAN by MAGIC packet")
> Signed-off-by: Nikola Z. Ivanov <zlatistiv@gmail.com>
> ---
> This patch is untested!
> I do not have access to a real device to test it,
> testing on real hardware would be appreciated,
> if anyone has a device laying around.
> 
> I have found no reason for the PM variants to be
> used in the ->suspend callback when it comes
> to the device driver.
> 
> The PM docs suggest that PM should not be done
> during runtime suspend, but I cannot find a
> definitive answer for system suspend, hence the
> conditional if(PMSG_IS_AUTO(message))

Dmitiry, Igor, could you possibly shed some light?
Can we simply switch to the _nopm() helpers instead?

> diff --git a/drivers/net/usb/aqc111.c b/drivers/net/usb/aqc111.c
> index cbffa9ae1bb6..2f0d66c7ade0 100644
> --- a/drivers/net/usb/aqc111.c
> +++ b/drivers/net/usb/aqc111.c
> @@ -1395,14 +1395,27 @@ static int aqc111_suspend(struct usb_interface *intf, pm_message_t message)
>  		aqc111_write16_cmd_nopm(dev, AQ_ACCESS_MAC,
>  					SFR_MEDIUM_STATUS_MODE, 2, &reg16);
>  
> -		aqc111_write_cmd(dev, AQ_WOL_CFG, 0, 0,
> -				 WOL_CFG_SIZE, &wol_cfg);
> -		aqc111_write32_cmd(dev, AQ_PHY_OPS, 0, 0,
> -				   &aqc111_data->phy_cfg);
> +		if (PMSG_IS_AUTO(message)) {
> +			aqc111_write_cmd_nopm(dev, AQ_WOL_CFG, 0, 0,
> +					      WOL_CFG_SIZE, &wol_cfg);
> +			aqc111_write32_cmd_nopm(dev, AQ_PHY_OPS, 0, 0,
> +						&aqc111_data->phy_cfg);
> +		} else {
> +			aqc111_write_cmd(dev, AQ_WOL_CFG, 0, 0,
> +					 WOL_CFG_SIZE, &wol_cfg);
> +			aqc111_write32_cmd(dev, AQ_PHY_OPS, 0, 0,
> +					   &aqc111_data->phy_cfg);
> +		}
>  	} else {
>  		aqc111_data->phy_cfg |= AQ_LOW_POWER;
> -		aqc111_write32_cmd(dev, AQ_PHY_OPS, 0, 0,
> -				   &aqc111_data->phy_cfg);
> +
> +		if (PMSG_IS_AUTO(message)) {
> +			aqc111_write32_cmd_nopm(dev, AQ_PHY_OPS, 0, 0,
> +						&aqc111_data->phy_cfg);
> +		} else {
> +			aqc111_write32_cmd(dev, AQ_PHY_OPS, 0, 0,
> +					   &aqc111_data->phy_cfg);
> +		}
>  
>  		/* Disable RX path */
>  		aqc111_read16_cmd_nopm(dev, AQ_ACCESS_MAC,


  reply	other threads:[~2026-03-07  0:43 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-04 15:57 [PATCH net] net: usb: aqc111: Do not perform PM inside runtime suspend callback Nikola Z. Ivanov
2026-03-07  0:43 ` Jakub Kicinski [this message]
2026-03-11  0:20   ` Jakub Kicinski

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=20260306164351.503e8540@kernel.org \
    --to=kuba@kernel.org \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=dbezrukov@marvell.com \
    --cc=edumazet@google.com \
    --cc=enelsonmoore@gmail.com \
    --cc=irusskikh@marvell.com \
    --cc=kees@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=n.zhandarovich@fintech.ru \
    --cc=netdev@vger.kernel.org \
    --cc=oneukum@suse.com \
    --cc=pabeni@redhat.com \
    --cc=syzbot+48dc1e8dfc92faf1124c@syzkaller.appspotmail.com \
    --cc=zlatistiv@gmail.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.