2021-02-16 18:55:10

by Nadav Amit

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

> On Feb 16, 2021, at 8:32 AM, Peter Zijlstra <[email protected]> wrote:
>
> On Tue, Feb 09, 2021 at 02:16:46PM -0800, Nadav Amit wrote:
>> From: Nadav Amit <[email protected]>
>>
>> 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 <[email protected]>
>> Cc: Peter Zijlstra <[email protected]>
>> Cc: Rik van Riel <[email protected]>
>> Cc: Thomas Gleixner <[email protected]>
>> Cc: Andy Lutomirski <[email protected]>
>> Cc: Josh Poimboeuf <[email protected]>
>> Signed-off-by: Nadav Amit <[email protected]>
>
> 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...


2021-02-16 19:02:42

by Peter Zijlstra

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

On Tue, Feb 16, 2021 at 06:53:09PM +0000, Nadav Amit wrote:
> > On Feb 16, 2021, at 8:32 AM, Peter Zijlstra <[email protected]> wrote:

> > 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...

Yeah, me neither. Bit of a mystery so far.

2021-02-16 19:06:34

by Nadav Amit

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

> On Feb 16, 2021, at 10:59 AM, Peter Zijlstra <[email protected]> wrote:
>
> On Tue, Feb 16, 2021 at 06:53:09PM +0000, Nadav Amit wrote:
>>> On Feb 16, 2021, at 8:32 AM, Peter Zijlstra <[email protected]> wrote:
>
>>> 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...
>
> Yeah, me neither. Bit of a mystery so far.

I’ll try to see whether I can figure out about it. Perhaps there is
somewhere an assumption of ordering between the local and remote function
invocations.

Regardless, would you want me to have on_each_cpu() as inline or to keep it
in smp.c?

2021-02-17 01:13:55

by Nadav Amit

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

> On Feb 16, 2021, at 10:59 AM, Peter Zijlstra <[email protected]> wrote:
>
> On Tue, Feb 16, 2021 at 06:53:09PM +0000, Nadav Amit wrote:
>>> On Feb 16, 2021, at 8:32 AM, Peter Zijlstra <[email protected]> wrote:
>
>>> 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...
>
> Yeah, me neither. Bit of a mystery so far.

This stall seems to be real. Intuitively I presumed preemption was
mistakenly enabled, but it does not seem so.

Any chance you can build the kernel with “CONFIG_CSD_LOCK_WAIT_DEBUG=Y” and
rerun it? Perhaps that output will tell us more.

2021-02-18 15:14:02

by Peter Zijlstra

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

On Wed, Feb 17, 2021 at 01:02:41AM +0000, Nadav Amit wrote:
> > On Feb 16, 2021, at 10:59 AM, Peter Zijlstra <[email protected]> wrote:
> >
> > On Tue, Feb 16, 2021 at 06:53:09PM +0000, Nadav Amit wrote:
> >>> On Feb 16, 2021, at 8:32 AM, Peter Zijlstra <[email protected]> wrote:
> >
> >>> 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...
> >
> > Yeah, me neither. Bit of a mystery so far.
>
> This stall seems to be real. Intuitively I presumed preemption was
> mistakenly enabled, but it does not seem so.
>
> Any chance you can build the kernel with “CONFIG_CSD_LOCK_WAIT_DEBUG=Y” and
> rerun it? Perhaps that output will tell us more.

Sadly not my system. It's a KernelCI box.