On 12/4/23 20:07, Johannes Berg wrote:
From: Johannes Berg <johannes.b...@intel.com>

As reported by Marc MERLIN in [1], at least one driver (igc)

perhaps Reported-by tag? (I know this is RFC as of now)

wants/needs to acquire the RTNL inside suspend/resume ops,
which can be called from here in ethtool if runtime PM is
enabled.

[1] https://lore.kernel.org/r/20231202221402.ga11...@merlins.org

Allow this by doing runtime PM transitions without the RTNL
held. For the ioctl to have the same operations order, this
required reworking the code to separately check validity and
do the operation. For the netlink code, this now has to do
the runtime_pm_put a bit later.

Signed-off-by: Johannes Berg <johannes.b...@intel.com>
---
  net/ethtool/ioctl.c   | 71 ++++++++++++++++++++++++++-----------------
  net/ethtool/netlink.c | 32 ++++++++-----------
  2 files changed, 56 insertions(+), 47 deletions(-)

Thank you for the patch,

I like the idea of split into validate + do for dev_ethtool(),
what minimizes unneeded PM touching. Moving pm_runtime_get_sync() out of RTNL is also a great improvement per se. Also from the pure coding perspective I see no obvious flaws in the patch. I think that igc code
was just accidental to the issue, in a way that it was not deliberate to
hold RTNL for extended periods. With your patch fixing the bug, there is
no point with waiting IMO, so

Reviewed-by: Przemek Kitszel <przemyslaw.kits...@intel.com>
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Reply via email to