On Tue, Dec 20, 2011 at 07:58:06PM +0000, Al Viro wrote: > On Wed, Dec 21, 2011 at 12:42:04AM +0530, Srivatsa S. Bhat wrote: > > register_hotcpu_notifier(...); > > grab spinlock > > for_each_online_cpu(N) > > add N to bitmap > > release spinlock > > > > because the latter code is not fully race-free (because we don't handle > > CPU_DOWN_PREPARE event in the callback and hence cpu_online_mask can get > > updated in-between). But it would still work since cpus going down don't > > really pose problems for us. > > Um? Sure, that loop can end up adding CPUs on their way down into the set. > And as soon as they get their CPU_DEAD, notifier will prune them out... Or > is there something I'm missing here? Anyway, the variant I have here > (untested) follows: 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 CPU 2: brings up new cpu, notifier adds CPU to bitmask CPU 1: unlock all cpus in mask 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. 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? Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx -- 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
