On 31.07.19 15:14, Michal Hocko wrote: > On Wed 31-07-19 15:02:49, David Hildenbrand wrote: >> On 31.07.19 14:53, Michal Hocko wrote: >>> On Wed 31-07-19 14:32:01, David Hildenbrand wrote: >>>> Let's document why we take the lock here. If we're going to overhaul >>>> memory hotplug locking, we'll have to touch many places - this comment >>>> will help to clairfy why it was added here. >>> >>> And how exactly is "lock for consistency" comment going to help the poor >>> soul touching that code? How do people know that it is safe to remove it? >>> I am not going to repeat my arguments how/why I hate "locking for >>> consistency" (or fun or whatever but a real synchronization reasons) >>> but if you want to help then just explicitly state what should done to >>> remove this lock. >>> >> >> I know that you have a different opinion here. To remove the lock, >> add_memory() locking has to be changed *completely* to the point where >> we can drop the lock from the documentation of the function (*whoever >> knows what we have to exactly change* - and I don't have time to do that >> *right now*). > > Not really. To remove a lock in this particular path it would be > sufficient to add > /* > * Although __add_memory used down the road is documented to > * require lock_device_hotplug, it is not necessary here because > * this is an early code when userspace or any other code path > * cannot trigger hotplug operations. > */ Okay, let me phrase it like this: Are you 100% (!) sure that we don't need the lock here. I am not - I only know what I documented back then and what I found out - could be that we are forgetting something else the lock protects. As I already said, I am fine with adding such a comment instead. But I am not convinced that the absence of the lock is 100% safe. (I am 99.99% sure ;) ). -- Thanks, David / dhildenb
