Re: [PATCH] VFS: br_write_lock locks on possible CPUs other than online CPUs

[Al Viro <viro@ZenIV.linux.org.uk>]

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?

???