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

From: Tejun Heo
Date: Thu Aug 26 2010 - 05:01:18 EST


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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/