On 06/22, Peter Zijlstra wrote:
>
> By having stop_two_cpus() acquire two cpu_stopper::locks we gain full
> order against the global stop_machine which takes each of these locks
> in order.
Yes, but stop_machine() locks/unlocs cpu_stopper->lock sequentially, it
never holds more than 1 ->lock, so
> +static void cpu_stop_queue_work(unsigned int cpu, struct cpu_stop_work *work)
> +{
> + struct cpu_stopper *stopper = &per_cpu(cpu_stopper, cpu);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&stopper->lock, flags);
> + __cpu_stop_queue_work(cpu, work);
> spin_unlock_irqrestore(&stopper->lock, flags);
> }
...
> int stop_two_cpus(unsigned int cpu1, unsigned int cpu2, cpu_stop_fn_t fn,
> void *arg)
> {
> - struct cpu_stop_done done;
> + struct cpu_stopper *stopper1, *stopper2;
> struct cpu_stop_work work1, work2;
> struct multi_stop_data msdata;
> + struct cpu_stop_done done;
> + unsigned long flags;
> +
> + if (cpu2 < cpu1)
> + swap(cpu1, cpu2);
...
> + stopper1 = per_cpu_ptr(&cpu_stopper, cpu1);
> + stopper2 = per_cpu_ptr(&cpu_stopper, cpu2);
> +
> + spin_lock_irqsave(&stopper1->lock, flags);
> + spin_lock(&stopper2->lock);
> +
> + __cpu_stop_queue_work(cpu1, &work1);
> + __cpu_stop_queue_work(cpu2, &work2);
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
- 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?
Oleg.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at http://www.tux.org/lkml/
