Re: BUG_ON(!cpus_equal(cpumask, tmp));

Andrew Morton
2004-03-31T01:36:38Z

"Martin J. Bligh" <mbligh@aracnet.com> wrote:
>
> --On Tuesday, March 30, 2004 17:11:04 -0800 Andrew Morton <akpm@osdl.org> wrote:
>
> > "Martin J. Bligh" <mbligh@aracnet.com> wrote:
> >>
> >> I made a similar patch, but I don't see how we can really fix it without
> >> providing locking on cpu_online_map.
> >
> > Are we missing something here?
> >
> > Why does, for example, smp_send_reschedule() not have the same problem?
> > Because we've gone around and correctly removed all references to the CPU
> > from the scheduler data structures before offlining it.
> >
> > But we're not doing that in the mm code, right? Should we not be taking
> > mmlist_lock and running around knocking this CPU out of everyone's
> > cpu_vm_mask before offlining it?
>
> I think we're assuming that we don't have to because the problem is fixed
> by the "cpus_and(tmp, cpumask, cpu_online_map)" in flush_tlb_others so we
> don't have to. Except it's racy, and doesn't work.

And it's a kludge, to work around dangling references to a CPU which has
gone away.

> It would seem to me that your suggestion would fix it. But isn't locking
> cpu_online_map both simpler and (most importantly) more generic? I can't
> imagine that we don't use this elsewhere ... suppose for instance we took
> a timer interrupt, causing a scheduler rebalance, and moved a process to
> an offline CPU at that point? Isn't any user of smp_call_function also racy?

If we have to add any fastpath locking to cope with CPU removal or reboot
then it's time to make CONFIG_HOTPLUG_CPU dependent upon CONFIG_BROKEN.

yes, cpu_online_map should be viewed as a reference to the going-away CPU
for smp_call_function purposes. However the CPU takedown code appears to
do the right thing: it removes the cpu from cpu_online_map first, then does
the stop_machine() thing which should ensure that all other CPUs have
completed any cross-CPU call which they were doing, yes?