nmi: Provide the option to issue an NMI back trace to every cpu but current
A message that is not in the archive (sent off-list or not yet received): [email protected]
A message that is not in the archive (sent off-list or not yet received): [email protected]
A message that is not in the archive (sent off-list or not yet received): [email protected]
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH v2 1/4 nmi: Provide the option to issue an NMI back trace to every cpu but current
Quoted 12 lines · Don Zickus
Quoted 10 lines · Oleg Nesterov
Quoted 5 lines · Oleg Nesterov
Looking at https://lkml.org/lkml/2014/4/4/469... It seems that 2/4 can be simplified, you can simply remove
smp_processor_id()frombacktrace_maskif !include_self and useapic->send_IPI_mask(backtrace_mask). But this is minor, I won't insist.And in fact, I do not understand why
arch_trigger_all_cpu_backtrace()doesn't disable preemption. OK, probably we can simply ignore the race with cpu hotplug.
Quoted 7 lines
Quoted 4 lines
But it seems that your patch makes the things worse. Lets look at, say,
numachip_send_IPI_mask_allbutself(). The usage ofsmp_processor_id()is obviously racy but perhaps we do not care again. But we do not want a warning fromdebug_smp_processor_id().Good point. I forgot that going from all cpus down to allbutself, preemption now matters.
I am not sure it actually matters wrt "show other CPU's traces". If the preemption is possible then the caller can be preempted even before it sends ipi.
OTOH I think it does matter anyway, even without your patch, otherwise the usage of cpu_online_mask is racy and we can hit the "Wait for up to 10 seconds" case.
Btw...
/* Wait for up to 10 seconds for all CPUs to do the backtrace */
for (i = 0; i < 10 * 1000; i++) {
if (cpumask_empty(to_cpumask(backtrace_mask)))
break;
mdelay(1);
}OK, but perhaps we should clear backtrace_mask if we return due to timeout.
Quoted 2 lines
does disabling preemption help in the cpu
hotplug case?
Yes. But I'd suggest to change your patch to use get_cpu() instead of preempt_disable/smp_processor_id.
And I think it would be better to not discuss this off-list, I added lkml.
Oleg.
Quoted 18 lines · Oleg Nesterov
Quoted 17 lines · Don Zickus
Quoted 14 lines · Oleg Nesterov
Quoted 5 lines · Oleg Nesterov
Looking at https://lkml.org/lkml/2014/4/4/469... It seems that 2/4 can be simplified, you can simply remove
smp_processor_id()frombacktrace_maskif !include_self and useapic->send_IPI_mask(backtrace_mask). But this is minor, I won't insist.And in fact, I do not understand why
arch_trigger_all_cpu_backtrace()doesn't disable preemption. OK, probably we can simply ignore the race with cpu hotplug.But it seems that your patch makes the things worse. Lets look at, say,
numachip_send_IPI_mask_allbutself(). The usage ofsmp_processor_id()is obviously racy but perhaps we do not care again. But we do not want a warning fromdebug_smp_processor_id().
Quoted 8 lines
Quoted 2 lines
Good point. I forgot that going from all cpus down to allbutself, preemption now matters.
I am not sure it actually matters wrt "show other CPU's traces". If the preemption is possible then the caller can be preempted even before it sends ipi.
OTOH I think it does matter anyway, even without your patch, otherwise the usage of
cpu_online_maskis racy and we can hit the "Wait for up to 10 seconds" case.
Hmm, I understand what you are saying now.
Quoted 11 lines
Btw...
/* Wait for up to 10 seconds for all CPUs to do the backtrace */ for (i = 0; i < 10 * 1000; i++) { if (cpumask_empty(to_cpumask(backtrace_mask))) break; mdelay(1); }OK, but perhaps we should clear
backtrace_maskif we return due to timeout.
I can look at that.
Quoted 6 lines
Quoted 3 lines
does disabling preemption help in the cpu
hotplug case?Yes. But I'd suggest to change your patch to use
get_cpu()instead of preempt_disable/smp_processor_id.
ok. Originally I was thinking of the remote hotplug cpu case, which pre-emption won't block. But forgot about the local cpu hotplug case.
Quoted 5 lines
And I think it would be better to not discuss this off-list, I added lkml.
Oleg.
Thanks!
Cheers,
Don