On 06/23, Peter Zijlstra wrote:
>
> On Tue, Jun 23, 2015 at 12:21:52AM +0200, Oleg Nesterov wrote:
>
> > Suppose that stop_two_cpus(cpu1 => 0, cpu2 => 1) races with stop_machine().
> >
> > - stop_machine takes the lock on CPU 0, adds the work
> > and drops the lock
> >
> > - cpu_stop_queue_work() queues both works
>
> cpu_stop_queue_work() only ever queues _1_ work.
>
> > - stop_machine takes the lock on CPU 1, etc
> >
> > In this case both CPU 0 and 1 will run multi_cpu_stop() but they will
> > use different multi_stop_data's, so they will wait for each other
> > forever?
>
> So what you're saying is:
>
> queue_stop_cpus_work() stop_two_cpus()
>
> cpu_stop_queue_work(0,..);
> spin_lock(0);
> spin_lock(1);
>
> __cpu_stop_queue_work(0,..);
> __cpu_stop_queue_work(1,..);
>
> spin_unlock(1);
> spin_unlock(0);
> cpu_stop_queue_work(1,..);
Yes, sorry for confusion.
> We can of course slap a percpu-rwsem in, but I wonder if there's
> anything smarter we can do here.
I am wondering too if we can make this multi_cpu_stop() more clever.
Or at least add some deadlock detection...
Until then you can probably just uglify queue_stop_cpus_work() and
avoid the race,
static void queue_stop_cpus_work(const struct cpumask *cpumask,
cpu_stop_fn_t fn, void *arg,
struct cpu_stop_done *done)
{
struct cpu_stopper *stopper;
struct cpu_stop_work *work;
unsigned long flags;
unsigned int cpu;
local_irq_save(flags);
for_each_cpu(cpu, cpumask) {
stopper = &per_cpu(cpu_stopper, cpu);
spin_lock(&stopper->lock);
work = &per_cpu(stop_cpus_work, cpu);
work->fn = fn;
work->arg = arg;
work->done = done;
}
for_each_cpu(cpu, cpumask)
__cpu_stop_queue_work(cpu, &per_cpu(stop_cpus_work,
cpu));
for_each_cpu(cpu, cpumask) {
stopper = &per_cpu(cpu_stopper, cpu);
spin_unlock(&stopper->lock);
}
local_irq_restore(flags);
}
ignoring lockdep problems.
It would be nice to remove stop_cpus_mutex, it actually protects
stop_cpus_work... Then probably stop_two_cpus() can just use
stop_cpus(). We could simply make stop_cpus_mutex per-cpu too,
but this doesn't look nice.
Oleg.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
