Re: [PATCH] lglock: make lg_lock_global() actually lock globally

Tejun Heo
2010-08-26T09:01:15Z

Hello,

On 08/25/2010 10:00 PM, Linus Torvalds wrote:
>> lg_lock_global() currently only acquires spinlocks for online CPUs, but
>> it's meant to lock all possible CPUs. At Nick's suggestion, change
>> for_each_online_cpu() to for_each_possible_cpu() to get the expected
>> behavior.
>
> Can you say what this actually matters for? Don't we do stop-machine
> for CPU hotplug anyway? And if we don't, shouldn't we? Exactly because
> otherwise "for_each_online_cpu()" is always racy (and that has nothing
> to do with the lglock).

We only do stop-machine for cpu downs not ups, so code running w/
preemption disabled is guaranteed that no cpu goes down while it's
running but not the other way around. There are two ways to achieve
synchronization against cpu up/down operations. One is explicitly
using get/put_online_cpus() and the other is via cpu notifiers with
proper synchronization.

So, yeah, given that there's no cpu notifier implemented, the use of
for_each_online_cpu for brlock seems fishy to me. It probably should
use for_each_possible_cpu().

Thanks.

--
tejun