Messages in this thread Patch in this message | | | Date | Sun, 15 Jan 2012 15:53:53 +0200 | Subject | Re: [PATCH 2/2] mm: page allocator: Do not drain per-cpu lists via IPI from page allocator context | From | Gilad Ben-Yossef <> |
| |
On Fri, Jan 13, 2012 at 10:58 PM, Milton Miller <miltonm@bga.com> wrote: > On Thu, 12 Jan 2012 about 14:14:57 -0500 (EST), Gilad Ben-Yossef wrote: >> I also think there is still some problems with IPIs somewhere that >> cause some corruption when a lot of IPIs are sent and that >> the patch simply lowered the very big number of IPIs that are sent >> via the direct reclaim code path so the problem >> is hidden, not solved by this patch. >> >> I've seen something related when trying to test the IPI reduction >> patches. Interesting enough it was not related to CPU hotplug at all - > >> When a lot of IPIs are being sent, I sometime got an assert from low >> level platform code that I'm trying to send IPIs with an empty mask >> although the mask was NOT empty. I didn't manage to debug it then but >> I did manage to recreate it quite easily. > > That is a known scenario and expected race, the check must be removed > from the platform code.
OK.
> >> >> I will see if I can recreate it with recent master and report. >> > > The code in kernel/smp checks the mask, and looks for opportunities to > convert smp_call_function_many to call_function_single. But it now > tolerates callers passing a mask that other cpus are changing. So > while it tries to make sure the mask has > 1 cpu, its not guaranteed.
Yes, that much I figured out already, even counted on this behavior.
> But that is not what causes the assert to fire.
Yes, my test case involves on_each_cpu with hotplug disabled.
> > There are a few opportunities for the architecture code to detect an > empty mask. The first case is a cpu started processing the list due > to an interrupt generated by a cpu other than the requester between > the time the requester put its work element in the list and when it > executed the architecture code to send the interrupt. A second > case is a work element is deleted by a third cpu as its ref count > goes to zero and then that element gets reused by the owning cpu, > so we process part of the request list twice. >
OK, that makes sense.
> The solution is to tell the architecture code its not an error for > the mask to be empty.
I believe this patch is in order then:
diff --git a/arch/x86/kernel/apic/ipi.c b/arch/x86/kernel/apic/ipi.c index cce91bf..00b68a3 100644 --- a/arch/x86/kernel/apic/ipi.c +++ b/arch/x86/kernel/apic/ipi.c @@ -106,7 +106,10 @@ void default_send_IPI_mask_logical(const struct cpumask *cpumask, int vector) unsigned long mask = cpumask_bits(cpumask)[0]; unsigned long flags;
- if (WARN_ONCE(!mask, "empty IPI mask")) + if (!mask) + /* The target CPUs must have already processed the + * work items due to a concurrent IPI + */ return;
local_irq_save(flags); Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com
> Here is a walk though of how smp_call_function_many works. <SNIP> > the interrupt. Eventually we wrap back to the head of the list and > return from the interrupt.
Would you mind if I pasted the above into Documentation/ipi.txt with your name on it?
It could save the next potential bloke trying to get his head around IPIs a lot of head scratching...
Also, it means I have no idea about the original problem with hotplug :-)
Gilad
-- Gilad Ben-Yossef Chief Coffee Drinker gilad@benyossef.com Israel Cell: +972-52-8260388 US Cell: +1-973-8260388 http://benyossef.com
"Unfortunately, cache misses are an equal opportunity pain provider." -- Mike Galbraith, LKML -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |