On Tue, 2023-12-05 at 06:19 +0100, Przemek Kitszel wrote: > 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)
I guess. > > 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. Well Jakub was arguing igc shouldn't be taking rtnl in suspend/resume, maybe, but dunno. > With your patch fixing the bug, there is > no point with waiting IMO, so > > Reviewed-by: Przemek Kitszel <przemyslaw.kits...@intel.com> Well, according to the checks, the patch really should use netdev_get_by_name() and netdev_put()? But I don't know how to do that on short-term stack thing ... maybe it doesn't have to? johannes _______________________________________________ Intel-wired-lan mailing list Intel-wired-lan@osuosl.org https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
