On Wed, Dec 21, 2011 at 09:27:34AM +1100, Dave Chinner wrote: > Only thing that concerns me about this patch is the bitmap changing > between lock and unlock operations. i.e. > > CPU 1: lock all cpus in mask ... after having taken the spinlock > CPU 2: brings up new cpu, notifier adds CPU to bitmask ... which would also take the same spinlock > CPU 1: unlock all cpus in mask ... which is where that spinlock would've been released > And in this case the unlock tries to unlock a cpu that wasn't locked > to start with. It really seems to me that while a global lock is in > progress, the online bitmask cannot be allowed to change. Well, yes. See spin_lock() in br_write_lock() before the loop and spin_unlock() in br_write_unlock() after the loop... > Perhaps something can be passed between the lock and unlock > operations to be able to detect a changed mask between lock/unlock > operations (e.g. a generation number) and then handle that via a > slow path that unlocks only locks that are active in the online > bitmask? i.e. all the notifier does is bump the generation count, > and the slow path on the unlock handles everything else? ??? -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
