Re: [PATCH v5 1/8] smp: Run functions concurrently in smp_call_function_many_cond()

From: Nadav Amit <namit@vmware.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Andy Lutomirski <luto@kernel.org>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Rik van Riel <riel@surriel.com>,
	Josh Poimboeuf <jpoimboe@redhat.com>
Subject: Re: [PATCH v5 1/8] smp: Run functions concurrently in smp_call_function_many_cond()
Date: Tue, 16 Feb 2021 18:53:09 +0000	[thread overview]
Message-ID: <457D8FBC-8F64-48E9-B9E2-1A316DB0C2B6@vmware.com> (raw)
In-Reply-To: <YCvztEk6sqiCxXZV@hirez.programming.kicks-ass.net>

> On Feb 16, 2021, at 8:32 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> 
> On Tue, Feb 09, 2021 at 02:16:46PM -0800, Nadav Amit wrote:
>> From: Nadav Amit <namit@vmware.com>
>> 
>> Currently, on_each_cpu() and similar functions do not exploit the
>> potential of concurrency: the function is first executed remotely and
>> only then it is executed locally. Functions such as TLB flush can take
>> considerable time, so this provides an opportunity for performance
>> optimization.
>> 
>> To do so, modify smp_call_function_many_cond(), to allows the callers to
>> provide a function that should be executed (remotely/locally), and run
>> them concurrently. Keep other smp_call_function_many() semantic as it is
>> today for backward compatibility: the called function is not executed in
>> this case locally.
>> 
>> smp_call_function_many_cond() does not use the optimized version for a
>> single remote target that smp_call_function_single() implements. For
>> synchronous function call, smp_call_function_single() keeps a
>> call_single_data (which is used for synchronization) on the stack.
>> Interestingly, it seems that not using this optimization provides
>> greater performance improvements (greater speedup with a single remote
>> target than with multiple ones). Presumably, holding data structures
>> that are intended for synchronization on the stack can introduce
>> overheads due to TLB misses and false-sharing when the stack is used for
>> other purposes.
>> 
>> Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Rik van Riel <riel@surriel.com>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Andy Lutomirski <luto@kernel.org>
>> Cc: Josh Poimboeuf <jpoimboe@redhat.com>
>> Signed-off-by: Nadav Amit <namit@vmware.com>
> 
> Kernel-CI is giving me a regression that's most likely this patch:
> 
>  https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fkernelci.org%2Ftest%2Fcase%2Fid%2F602bdd621c979f83faaddcc6%2F&amp;data=04%7C01%7Cnamit%40vmware.com%7C7dc93f3b74d8488de06f08d8d2988b0a%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637490899907612612%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=PFs0ydMLh6xVfAQzAxSNd108YjxKMopNwxqsm82lEog%3D&amp;reserved=0
> 
> I'm not sure I can explain it yet. It did get me looking at
> on_each_cpu() and it appears that wants to be converted too, something
> like the below perhaps.

Looks like a good cleanup, but I cannot say I understand the problem and how
it would solve it. Err...