linux-kernel - Re: [PATCH 2/2] mm: page allocator: Do not drain per-cpu lists via IPI from page allocator context

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 15 Jan 2012 15:53:53 +0200
From:	Gilad Ben-Yossef <gilad@...yossef.com>
To:	Milton Miller <miltonm@....com>
Cc:	Mel Gorman <mgorman@...e.de>, linux-kernel@...r.kernel.org,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>
Subject: Re: [PATCH 2/2] mm: page allocator: Do not drain per-cpu lists via
 IPI from page allocator context

On Fri, Jan 13, 2012 at 10:58 PM, Milton Miller <miltonm@....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@...yossef.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@...yossef.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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/