netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
Cc: Heiner Kallweit <hkallweit1@gmail.com>,
	Johannes Berg <johannes@sipsolutions.net>,
	netdev@vger.kernel.org, Johannes Berg <johannes.berg@intel.com>,
	Marc MERLIN <marc@merlins.org>,
	Przemek Kitszel <przemyslaw.kitszel@intel.com>
Subject: Re: [PATCH net v3] net: ethtool: do runtime PM outside RTNL
Date: Fri, 5 Jan 2024 19:02:18 -0800	[thread overview]
Message-ID: <20240105190218.5b69b9f5@kernel.org> (raw)
In-Reply-To: <ZZguXLO3DAX/2Y0/@linux.intel.com>

On Fri, 5 Jan 2024 17:29:16 +0100 Stanislaw Gruszka wrote:
> > Removing the rpm calls from the core is just going to lead to a
> > whack-a-mole of bugs in the drivers themselves.
> >
> > IOW I look at the RPM calls in the core as a canary for people
> > doing the wrong thing :(  
> 
> Hmm, this one I don't understand, what other bugs could pop up
> after reverting bd869245a3dcc and others that added rpm calls
> for the net core?

IDK what igc powers down, but if there's any ndo or ethtool
callback which needs to access a register that requires power 
to be resumed - it will deadlock on rtnl exactly the same.

Looking at igc_ethtool I see:

static int igc_ethtool_begin(struct net_device *netdev)
{
	struct igc_adapter *adapter = netdev_priv(netdev);

	pm_runtime_get_sync(&adapter->pdev->dev);
	return 0;
}

static void igc_ethtool_complete(struct net_device *netdev)
{
	struct igc_adapter *adapter = netdev_priv(netdev);

	pm_runtime_put(&adapter->pdev->dev);
}

so unless we think that returning -ENODEV from all ethtool calls
when cable is not plugged in is okay - removing the PM resume
from the core doesn't buy us much :(

  reply	other threads:[~2024-01-06  3:02 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-06 10:39 [PATCH net v3] net: ethtool: do runtime PM outside RTNL Johannes Berg
2023-12-06 16:44 ` Jakub Kicinski
2023-12-06 16:46   ` Johannes Berg
2023-12-06 21:39     ` Marc MERLIN
2023-12-07 10:16     ` Przemek Kitszel
2023-12-07 17:40       ` Jakub Kicinski
2023-12-11  4:52         ` Marc MERLIN
2023-12-15 13:42           ` Heiner Kallweit
2023-12-15 17:46             ` Marc MERLIN
2023-12-24 16:30               ` Marc MERLIN
2023-12-24 23:12                 ` Heiner Kallweit
2023-12-25  8:03                   ` [Intel-wired-lan] " Sasha Neftin
2023-12-25 11:21                     ` Marc MERLIN
2024-01-03 10:30   ` Stanislaw Gruszka
2024-01-03 11:24     ` Heiner Kallweit
2024-01-03 12:15       ` Stanislaw Gruszka
2024-01-03 23:34     ` Jakub Kicinski
2024-01-04  8:25       ` Stanislaw Gruszka
2024-01-04  9:05         ` Heiner Kallweit
2024-01-04 16:16           ` Jakub Kicinski
2024-01-05 11:53             ` Stanislaw Gruszka
2024-01-05 15:30               ` Jakub Kicinski
2024-01-05 16:29                 ` Stanislaw Gruszka
2024-01-06  3:02                   ` Jakub Kicinski [this message]
2024-01-08 11:18                     ` Stanislaw Gruszka
2024-01-05 10:34           ` Stanislaw Gruszka

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=20240105190218.5b69b9f5@kernel.org \
    --to=kuba@kernel.org \
    --cc=hkallweit1@gmail.com \
    --cc=johannes.berg@intel.com \
    --cc=johannes@sipsolutions.net \
    --cc=marc@merlins.org \
    --cc=netdev@vger.kernel.org \
    --cc=przemyslaw.kitszel@intel.com \
    --cc=stanislaw.gruszka@linux.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 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).