On Thu, 29 Oct 2020 18:45:29 +0100 Pavel Machek <pavel@xxxxxx> wrote: > Hi! > > > since you are the original author of netdev LED trigger, I guess this > > question should go to you. Why are spinlocks used as locks in the > > netdev trigger code? Is this for performance? Would it be a drastic > > performance hit to use mutexes? > > Triggers can be called from interrupt context, IIRC, and many simple > LEDs can be operated from interrupt context, too. > > Thus you need spinlocks... > > Best regards, > Pavel Pavel, the set_baseline_state function in netdev trigger is guarded by a spinlock only when reading/writing device_name attribute and in netdev notify callback. netdev_trig_notify can for example put the device away on NETDEV_UNREGISTER event, and if someone was reading/writing the device_name at the same time netdev_trig_notify is manipulating netdevice pointer, it could break. So my guess for the lock is that it is used because of this. It is possible that netdev_trig_notify can be called from interrupt context, I will have to look into this. Anyway for the trigger_offload() method to be able to communicate with PHYs I need the set_baseline_state function not to be called from within spinlock. Otherwise the drivers implementing this method would get too complicated. Would it be allright if on netdev event (link up, link down, netdev changename, netdev unregister) the set_baseline_state was called from a work, instead of directly from the spinlock? I will send a patch proposing and explaining this. Marek
