On 06.12.2023 10:37, Johannes Berg wrote: > On Wed, 2023-12-06 at 09:46 +0100, Przemek Kitszel wrote: >> >> That sounds right too; one could argue if your fix is orthogonal to that >> or not. I would say that your fix makes core net code more robust >> against drivers from past millennia. :) >> igc folks are notified, no idea how much time it would take to propose >> a fix. > > Maybe it should be on whoever added runtime pm to ethtool ;-) > > Heiner, the igc driver was already doing this when you added > pm_runtime_get_sync() ops, was there a discussion at the time, or just > missed? > I think it went unnoticed at that time that igc is acquiring RTNL in runtime-resume. I'm just astonished that this pops up only now, considering that the change was done more than 2 yrs ago.
Note: In __dev_open() there's a similar scenario where the runtime-resume callback may be executed under RTNL. > I really don't know any of this ... > >>> 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? >> >> Nice to have such checks :) >> You need some &netdevice_tracker, perhaps one added into struct net >> or other place that would allow to track it at ethtool level. > > Yeah but that's dynamic? Seems weird to add something to allocations for > something released in the same function ... > >> "short term stack thing" does not relieve us from good coding practices, >> but perhaps "you just replaced __dev_get_by_name() call by >> dev_get_by_name()" to fix a bug would ;) - with transition to tracked >> alloc as a next series to be promised :) > > All I want is to know how ;) > but I guess I can try to find examples. > >> anyway, I'm fresh here, and would love to know what others think about > > Not me, but me too ;-) > > johannes _______________________________________________ Intel-wired-lan mailing list Intel-wired-lan@osuosl.org https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
