Messages in this thread | | | Date | Tue, 22 Apr 2014 16:44:55 -0400 | From | Tejun Heo <> | Subject | Re: [RFC PATCH v5 2/2] Use kernfs_break_active_protection() for device online store callbacks |
| |
Hello,
On Tue, Apr 22, 2014 at 11:34:39AM +0800, Li Zhong wrote: > > Is this assumption true? If so, can we add lockdep assertions in > > places to verify and enforce this? If not, aren't we just feeling > > good when the reality is broken? > > It seems not true ... I think there are devices that don't have the > online/offline concept, we just need to add it, remove it, like ethernet > cards. > > Maybe we could change the comments above, like: > /* We assume device_hotplug_lock must be acquired before > * removing devices, which have online/offline sysfs knob, > * and some locks are needed to serialize the online/offline > * callbacks and device removing. ... > ? > > And we could add lockdep assertions in cpu and memory related code? e.g. > remove_memory(), unregister_cpu() > > Currently, remove_memory() has comments for the function: > > * NOTE: The caller must call lock_device_hotplug() to serialize hotplug > * and online/offline operations before this call, as required by > * try_offline_node(). > */ > > maybe it could be removed with the lockdep assertion.
I'm confused about the overall locking scheme. What's the role of device_hotplug_lock? Is that solely to prevent the sysfs deadlock issue? Or does it serve other synchronization purposes depending on the specific subsystem? If the former, the lock no longer needs to exist. The only thing necessary would be synchronization between device_del() deleting the sysfs file and the unbreak helper invoking device-specific callback. If the latter, we probably should change that. Sharing hotplug lock across multiple subsystems through driver core sounds like a pretty bad idea.
Thanks.
-- tejun
| |