netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alan Stern <stern@rowland.harvard.edu>
To: Lukas Wunner <lukas@wunner.de>
Cc: Steve Glendinning <steve.glendinning@shawell.net>,
	UNGLinuxDriver@microchip.com, Oliver Neukum <oneukum@suse.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	netdev@vger.kernel.org, linux-usb@vger.kernel.org,
	Andre Edich <andre.edich@microchip.com>,
	Oleksij Rempel <o.rempel@pengutronix.de>,
	Martyn Welch <martyn.welch@collabora.com>,
	Gabriel Hojda <ghojda@yo2urs.ro>,
	Christoph Fritz <chf.fritz@googlemail.com>,
	Lino Sanfilippo <LinoSanfilippo@gmx.de>,
	Philipp Rosenberger <p.rosenberger@kunbus.com>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Russell King <linux@armlinux.org.uk>
Subject: Re: [PATCH net] usbnet: smsc95xx: Fix deadlock on runtime resume
Date: Wed, 27 Apr 2022 10:00:08 -0400	[thread overview]
Message-ID: <YmlMaE53+EhRz5it@rowland.harvard.edu> (raw)
In-Reply-To: <6710d8c18ff54139cdc538763ba544187c5a0cee.1651041411.git.lukas@wunner.de>

On Wed, Apr 27, 2022 at 08:41:49AM +0200, Lukas Wunner wrote:
> Commit 05b35e7eb9a1 ("smsc95xx: add phylib support") amended
> smsc95xx_resume() to call phy_init_hw().  That function waits for the
> device to runtime resume even though it is placed in the runtime resume
> path, causing a deadlock.
> 
> The problem is that phy_init_hw() calls down to smsc95xx_mdiobus_read(),
> which never uses the _nopm variant of usbnet_read_cmd().  Amend it to
> autosense that it's called from the runtime resume/suspend path and use
> the _nopm variant if so.

...

> diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
> index 4ef61f6b85df..82b8feaa5162 100644
> --- a/drivers/net/usb/smsc95xx.c
> +++ b/drivers/net/usb/smsc95xx.c
> @@ -285,11 +285,21 @@ static void smsc95xx_mdio_write_nopm(struct usbnet *dev, int idx, int regval)
>  	__smsc95xx_mdio_write(dev, pdata->phydev->mdio.addr, idx, regval, 1);
>  }
>  
> +static bool smsc95xx_in_pm(struct usbnet *dev)
> +{
> +#ifdef CONFIG_PM
> +	return dev->udev->dev.power.runtime_status == RPM_RESUMING ||
> +	       dev->udev->dev.power.runtime_status == RPM_SUSPENDING;
> +#else
> +	return false;
> +#endif
> +}

This does not do what you want.  You want to know if this function is 
being called in the resume pathway, but all it really tells you is 
whether the function is being called while a resume is in progress (and 
it doesn't even do that very precisely because the code does not use the 
runtime-pm spinlock).  The resume could be running in a different 
thread, in which case you most definitely _would_ want to want for it to 
complete.

Alan Stern

  reply	other threads:[~2022-04-27 14:00 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-27  6:41 [PATCH net] usbnet: smsc95xx: Fix deadlock on runtime resume Lukas Wunner
2022-04-27 14:00 ` Alan Stern [this message]
2022-04-27 15:10   ` Lukas Wunner
2022-04-27 15:24 ` Andrew Lunn
2022-04-27 15:38   ` Lukas Wunner
2022-04-27 18:19     ` Alan Stern
2022-04-27 15:45   ` 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=YmlMaE53+EhRz5it@rowland.harvard.edu \
    --to=stern@rowland.harvard.edu \
    --cc=LinoSanfilippo@gmx.de \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=andre.edich@microchip.com \
    --cc=andrew@lunn.ch \
    --cc=chf.fritz@googlemail.com \
    --cc=davem@davemloft.net \
    --cc=ghojda@yo2urs.ro \
    --cc=hkallweit1@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=lukas@wunner.de \
    --cc=martyn.welch@collabora.com \
    --cc=netdev@vger.kernel.org \
    --cc=o.rempel@pengutronix.de \
    --cc=oneukum@suse.com \
    --cc=p.rosenberger@kunbus.com \
    --cc=pabeni@redhat.com \
    --cc=steve.glendinning@shawell.net \
    /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 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).