Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <14cceb7c-f5f7-ee8e-18aa-4884ef212eb0@intel.com>
Date:   Fri, 11 Mar 2022 17:35:44 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Firefox/91.0 Thunderbird/91.7.0
Subject: Re: [PATCH] x86/cpu: use smp_call_function_many() in
 arch_freq_prepare_all()
Content-Language: en-US
To:     Eric Dumazet <eric.dumazet@gmail.com>,
        "Paul E . McKenney" <paulmck@kernel.org>
CC:     linux-kernel <linux-kernel@vger.kernel.org>,
        Eric Dumazet <edumazet@google.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        "H. Peter Anvin" <hpa@zytor.com>, <x86@kernel.org>
References: <20220311011715.2440601-1-eric.dumazet@gmail.com>
From:   "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Organization: Intel Technology Poland Sp. z o. o., KRS 101882, ul. Slowackiego
 173, 80-298 Gdansk
In-Reply-To: <20220311011715.2440601-1-eric.dumazet@gmail.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?L1RGUnpmTjF4UWhwL2NBajRhOS9BU1NoeE5OWlIwdFkvY0diYVlrek40dXpt?=
 =?utf-8?B?TXlXWUU2QWxiQktrMVA2Mm5HTkorL0pyOGsvc3E0V3BsdnpOZTZ1amw2cUt6?=
 =?utf-8?B?clFydS9zRlIrMUo2cTAvKzNDVS96QVN3dC9tZEsraWlFVzhJdDl1Z1FhT1RT?=
 =?utf-8?B?YzFIcjJ3NWRvZnJyWTkrT1NxMW5TdCt3MjVRaEJjTE9EdEkwNi9PWk1sUzRs?=
 =?utf-8?B?RXJPUElpd0srbHJLb2syMmFDV3FGclAvR3d0WnlmVjROb2dCcG5Cenh3a2lY?=
 =?utf-8?B?U0JqQm1CSW0wN1kwVUlnRUZoeUdIUTBia0hWSjlOY2kxM2Q5bFIzalhLVUJG?=
 =?utf-8?B?NGhaSC9vRWluV2J2dGYraVFyaXZQeDN1enh6RGpBNVlIYkFvR3BXK0RpL1gr?=
 =?utf-8?B?SHBKaEs4UVVYTUxLWWxvZ0J6REYvOGxSRFhEQnBDeXU0MmdvcGtaZ3MwaXpF?=
 =?utf-8?B?YnZ1L24zS0s3TTNJRzd3akw5KzJQNHl5eUl4dkxTQ0E0djIwOXFFSndoUzhS?=
 =?utf-8?B?RUg3S2NQL1VZTldpRUl1Q2swME5JT3BiRkZ5MnNuMllQaWcrc3V1SExoRFVr?=
 =?utf-8?B?RnQyT3FjK05SRUN2N2dPY0lOaERsUTIva2NwaWpVS3RqeFBWZEFUK2NlbkZU?=
 =?utf-8?B?dHRtYzZnMkhpS1hDNWxQNTRpMlJUMFQ5UzhTbkFJNm9zMUE1ZzJvZjVUZXFs?=
 =?utf-8?B?UG1IdXorU0RwVExkTjR6Z3dsVFljR3dJSUdBcHFlYUoybGMrU0ZEcDBrTGRh?=
 =?utf-8?B?b3I2eGUzbG0yT1B6VXh1QjBDMisyeHk0UmJoemR1N0F5Mkg3a1Y0VWw4TjZZ?=
 =?utf-8?B?eHNldC9JUm5GMHFieGpYdE1tMnk0dThtdFlxLzgrOWNUY2tUMjAwYVlJdGJU?=
 =?utf-8?B?Z2htS21BVk4rVjVTeTRnQ3FibDM0VFVnNTdWSXdBbzNKS284T0U1SjY2dU1h?=
 =?utf-8?B?K2cxMmNXS083OEhUTnBDRXE2cEgvSkt5dC9TWG5xRFYxNlV4TzZOVCt5VFE2?=
 =?utf-8?B?clpSRTArNWhsaC9rd1Jza0hMQUxIY1Y2S1psVncrVTE0VjdmUzVhWk9TbTdW?=
 =?utf-8?B?K3FxYWFZNUFlUEx6dWJwaWdxMVJBNVRLekJudXlTNy9IVG5aUEFyL0FPQnRH?=
 =?utf-8?B?TVE3SldFbVN2elEybUp3RWhMTzZTOTViVUZBd3FVUSt0OVZtUlRJZmlxTnBG?=
 =?utf-8?B?Yjc1ZWlRZVNrOHhDNlpvZk5CN2h5NE9vMzM5THk2eUNkZk5iakczUmJWWFBn?=
 =?utf-8?B?dEJUbDZMdEJOTlJhbTlGVUpLT1FGbXJ0YjFuVGhQQ3paTCs0Zm8wN2l2azVM?=
 =?utf-8?B?cGFna2ZmUjhLMFRxa1UwQnJKY3NwNFMwcUZKR2ZtOUFZQXMrbHppQmM2WHBO?=
 =?utf-8?B?YUFzbm1BVVFTVmVxZ0JJRTF4L2tqejBkdGlNdU4vMVcybnVkNjZndEl2MGpR?=
 =?utf-8?B?S2ZSZ3IyZFNuQm9TRkVrTEVKdHE3VkdCckdxNnRiQjlBWVNyTzBZL21XU3dC?=
 =?utf-8?B?VnVhQVp1NnhzdCs0N2ZWUHN0K1NUeDFPc0U1TVprVUtOcTBJRVVjZFNKNExk?=
 =?utf-8?B?VUhYWmlMNTlRenBCVDI1MjZDVC9NV0t2LzJhSGMvNHo0VzFxOGRqQ3FQQWNu?=
 =?utf-8?B?dXdxVEVOMzVuZVM2OFRoWEIxd0VrSmJkT1pKYUg1ampxNVlseTNOdDFZc1JQ?=
 =?utf-8?B?eEoremRYL3UxRnJrdXZSd3N1UjBUaXJIR3MrRk1wTi8vbklzTG82NlUzdlZO?=
 =?utf-8?B?dDJRKzUvbktNRlUxeWR3Z3VMcTJDbkFLU2Z2MlpwRFBXT2FaOVVHZWdZbDkz?=
 =?utf-8?B?MUxkWlBOQ09veWhnRllrODVrSWVrWU5icTliQjRoR1ZGZFpOYU9JbUQxaFFB?=
 =?utf-8?B?Vyt2a0I1RnpOSjZ6MktjQ2drV2hKNWpITE9peXpUcjE0bTlzMzNERUx6d0lS?=
 =?utf-8?B?K1QzUFhNVHpXdHdNTG40NDM4K2xrOGJUWmhWOGZYOEo0dnVHTHhNOGFISVR0?=
 =?utf-8?B?S0cvOU9rOXJ3PT0=?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 274c89bf-2093-4038-a3d8-08da037d3492
X-MS-Exchange-CrossTenant-AuthSource: MW5PR11MB5810.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Mar 2022 16:35:51.6666
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: TnEo9M0kdhy1abx5n5WVjkNhBYSciwfVzNGoGtmVQU1wxo055F2rJ1FYvGZQkfFYt28tNHP/2x7ZRuOn0rLBoyi+hSs14JKoQvXq/xEdb/8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM8PR11MB5736
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
	DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,RDNS_NONE,SPF_HELO_NONE,
	T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
	lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 3/11/2022 2:17 AM, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Opening /proc/cpuinfo can have a big latency on hosts with many cpus,
> mostly because it is essentially doing:
>
>     for_each_online_cpu(cpu)
>      smp_call_function_single(cpu, aperfmperf_snapshot_khz, ...)
>
> smp_call_function_single() is reusing a common csd, meaning that
> each invocation needs to wait for completion of the prior one.
>
> Paul recent patches have lowered number of cpus receiving the IPI,
> but there are still cases where the latency of the above loop can
> reach 10 ms, then an extra msleep(10) is performed, for a total of 20ms.
>
> Using smp_call_function_many() allows for full parallelism,
> and latency is down to ~80 usec, on a host with 256 cpus.

This looks reasonable to me.

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

or if you want me to pick it up, please resend the patch with a CC to 
linux-pm@vger.kernel.org.

> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Paul E. McKenney <paulmck@kernel.org>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: <x86@kernel.org>
> ---
>   arch/x86/kernel/cpu/aperfmperf.c | 32 +++++++++++++++++++++++---------
>   1 file changed, 23 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/aperfmperf.c b/arch/x86/kernel/cpu/aperfmperf.c
> index 22911deacb6e441ad60ddb57190ef3772afb3cf0..a305310ceb44784a0ad9be7c196061d98fa1adbc 100644
> --- a/arch/x86/kernel/cpu/aperfmperf.c
> +++ b/arch/x86/kernel/cpu/aperfmperf.c
> @@ -67,7 +67,8 @@ static void aperfmperf_snapshot_khz(void *dummy)
>   	atomic_set_release(&s->scfpending, 0);
>   }
>   
> -static bool aperfmperf_snapshot_cpu(int cpu, ktime_t now, bool wait)
> +static bool aperfmperf_snapshot_cpu(int cpu, ktime_t now, bool wait,
> +				    struct cpumask *mask)
>   {
>   	s64 time_delta = ktime_ms_delta(now, per_cpu(samples.time, cpu));
>   	struct aperfmperf_sample *s = per_cpu_ptr(&samples, cpu);
> @@ -76,9 +77,13 @@ static bool aperfmperf_snapshot_cpu(int cpu, ktime_t now, bool wait)
>   	if (time_delta < APERFMPERF_CACHE_THRESHOLD_MS)
>   		return true;
>   
> -	if (!atomic_xchg(&s->scfpending, 1) || wait)
> -		smp_call_function_single(cpu, aperfmperf_snapshot_khz, NULL, wait);
> -
> +	if (!atomic_xchg(&s->scfpending, 1) || wait) {
> +		if (mask)
> +			__cpumask_set_cpu(cpu, mask);
> +		else
> +			smp_call_function_single(cpu, aperfmperf_snapshot_khz,
> +						 NULL, wait);
> +	}
>   	/* Return false if the previous iteration was too long ago. */
>   	return time_delta <= APERFMPERF_STALE_THRESHOLD_MS;
>   }
> @@ -97,13 +102,14 @@ unsigned int aperfmperf_get_khz(int cpu)
>   	if (rcu_is_idle_cpu(cpu))
>   		return 0; /* Idle CPUs are completely uninteresting. */
>   
> -	aperfmperf_snapshot_cpu(cpu, ktime_get(), true);
> +	aperfmperf_snapshot_cpu(cpu, ktime_get(), true, NULL);
>   	return per_cpu(samples.khz, cpu);
>   }
>   
>   void arch_freq_prepare_all(void)
>   {
>   	ktime_t now = ktime_get();
> +	cpumask_var_t mask;
>   	bool wait = false;
>   	int cpu;
>   
> @@ -113,17 +119,25 @@ void arch_freq_prepare_all(void)
>   	if (!boot_cpu_has(X86_FEATURE_APERFMPERF))
>   		return;
>   
> +	if (!zalloc_cpumask_var(&mask, GFP_KERNEL))
> +		return;
> +
> +	cpus_read_lock();
>   	for_each_online_cpu(cpu) {
>   		if (!housekeeping_cpu(cpu, HK_FLAG_MISC))
>   			continue;
>   		if (rcu_is_idle_cpu(cpu))
>   			continue; /* Idle CPUs are completely uninteresting. */
> -		if (!aperfmperf_snapshot_cpu(cpu, now, false))
> +		if (!aperfmperf_snapshot_cpu(cpu, now, false, mask))
>   			wait = true;
>   	}
>   
> -	if (wait)
> -		msleep(APERFMPERF_REFRESH_DELAY_MS);
> +	preempt_disable();
> +	smp_call_function_many(mask, aperfmperf_snapshot_khz, NULL, wait);
> +	preempt_enable();
> +	cpus_read_unlock();
> +
> +	free_cpumask_var(mask);
>   }
>   
>   unsigned int arch_freq_get_on_cpu(int cpu)
> @@ -139,7 +153,7 @@ unsigned int arch_freq_get_on_cpu(int cpu)
>   	if (!housekeeping_cpu(cpu, HK_FLAG_MISC))
>   		return 0;
>   
> -	if (aperfmperf_snapshot_cpu(cpu, ktime_get(), true))
> +	if (aperfmperf_snapshot_cpu(cpu, ktime_get(), true, NULL))
>   		return per_cpu(samples.khz, cpu);
>   
>   	msleep(APERFMPERF_REFRESH_DELAY_MS);