Re: [PATCH][3.10] nohz: Fix lockup on restart from wrong error code

[Frederic Weisbecker]

2013/5/22, Steven Rostedt <rostedt@xxxxxxxxxxx>:
> On Tue, 2013-05-21 at 22:14 +0200, Frederic Weisbecker wrote:
>> 2013/5/21 Steven Rostedt <rostedt@xxxxxxxxxxx>:
>> > commit a382bf934449 "nohz: Assign timekeeping duty to a CPU outside the
>> > full dynticks range" added a cpu notifier callback that would prevent
>> > the time keeping CPU from going offline if the have_nohz_full_mask was
>> > set.
>> >
>> > This also prevents the CPU from going offline on system reboot.
>> >
>> > Worse yet, the return code was -EINVAL, but the notifier does not
>> > recognize error codes, and it must be wrapped by a
>> > notifier_from_errno()
>> > function. This means that even though the CPU would fail to go down,
>> > the
>> > notifier would think it succeeded, and the cpu down process would
>> > continue.
>> >
>> > This caused two different problems. One, the migration thread after
>> > moving tasks from the CPU would park itself and then a task, namely the
>> > reboot task, could migrate onto that CPU. Then the reboot task spins
>> > waiting for the cpu to go idle. But because the reboot task happens to
>> > be spinning on the cpu its waiting for, the system hangs.
>>
>> Can that happen if that CPU is the boot CPU? Note this is the only
>> possible timekeeper with the upstream code.
>
> Yep it can happen in upstream (that's all I'm using). In
> tick_broadcast_setup_oneshot(), it sets the tick_do_timer_cpu to the
> current CPU, which can be something other than the boot CPU. Now that
> CPU wont be able to be hot plugged.

Ah indeed it can happen on broadcast timer initialization. A secondary
CPU then steal the duty from the boot CPU. Hmm this reminds me of this
patch:  https://patchwork.kernel.org/patch/2302951/
I thought it deserved some attention due its code simplification but
it could also solve the issue.

>>
>> >
>> > The other error that happened was that the sched_domain re-setup would
>> > get confused, and in get_group() the cpu = cpumask_first() would
>> > process
>> > a mask that had nothing set, and return cpu > nr_cpu_ids. Later it
>> > would
>> > reference the per_cpu sg with this CPU and get a bogus pointer and
>> > crash.
>>
>> Ouch, when are we doing this domain re-setup? I remember we
>> repartition the domains after cpu down/up but I don't understand how
>> that can interfere with this issue.
>
> I haven't looked hard enough yet, but this problem only appeared when
> this bug triggered. By telling the system a CPU is offline, but still
> having tasks schedule to it, causes all sorts of weird side effects. I
> haven't figured out in detail how this affected the sched domains, but I
> don't get the sched domain corruption after fixing this bug.

Weird. but the CPU refuses to offline so how could it see itself
online? Anyway if that happen again I'll have a look when I'm fully
back next week.
--
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/