linux-kernel - Re: [PATCH] sched/core: Bypass arch_scale_freq_tick() on nohz

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20210830093103.GH4353@worktop.programming.kicks-ass.net>
Date:   Mon, 30 Aug 2021 11:31:03 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Nicolas Saenz Julienne <nsaenzju@...hat.com>
Cc:     linux-kernel@...r.kernel.org, mingo@...hat.com,
        vincent.guittot@...aro.org, dietmar.eggemann@....com,
        rostedt@...dmis.org, bsegall@...gle.com, mgorman@...e.de,
        bristot@...hat.com, juri.lelli@...hat.com, mtosatti@...hat.com,
        nilal@...hat.com, frederic@...nel.org
Subject: Re: [PATCH] sched/core: Bypass arch_scale_freq_tick() on nohz_full
 CPUs

On Fri, Aug 27, 2021 at 08:49:10PM +0200, Nicolas Saenz Julienne wrote:
> arch_scale_freq_tick() calculations generally track the increment of
> frequency performance counters in between scheduler ticks to provide
> estimations on how DVFS swayed frequency on that CPU. This information
> eventually allows for more precise scheduling decisions. It's all fine
> and good, but nohz_full CPUs are capable of disabling the tick
> indefinitely and potentially trigger overflows in
> arch_scale_freq_tick()'s calculations once it's eventually re-enabled.
> 
> This will happen for both users of this interface: x86 and arm64. And
> it's also relevant that the heuristic on what to do in case of
> operations overflowing varies depending on the implementation. It goes
> from fully disabling frequency invariance scaling on all CPUS, to
> ignoring this is a possibility.
> 
> It's arguable that nohz_full CPUs are unlikely to benefit much from this
> feature, since their aim is to allow for uninterrupted execution of a
> single task, effectively getting the scheduler out of the way. Also,
> DVFS itself is also unlikely be used on most nohz_full systems, given
> its effects on latency.
> 
> So get around this by not calling arch_scale_freq_tick() on nohz_full
> CPUs.
> 
> Note that tick_nohz_full_cpu() relies on a static branch, which avoids
> degrading performance on the rest of systems.
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenzju@...hat.com>
> ---
>  kernel/sched/core.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 2fd623b2270d..8c04ec0e073a 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -5016,7 +5016,13 @@ void scheduler_tick(void)
>  	unsigned long thermal_pressure;
>  	u64 resched_latency;
>  
> -	arch_scale_freq_tick();
> +	/*
> +	 * nohz_full CPUs are capable of disabling the scheduler tick
> +	 * indefinitely, potentially overflowing arch_scale_freq_tick()
> +	 * calculations once it's re-enabled.
> +	 */
> +	if (!tick_nohz_full_cpu(smp_processor_id()))
> +		arch_scale_freq_tick();
>  	sched_clock_tick();
>  
>  	rq_lock(rq, &rf);

Hurmph,.. I'm not too happy with this.. Fundamentally the whole
NOHZ_FULL state should be dynamic, it currently isn't but that's
arguably a bug of the current implementation.

As such the above doesn't really dtrt, since a CPU disabling NOHZ_FULL
will then still suffer all them overflows you mentioned.

Frederic, how should this be done right? Is there a place where upon
entering/exiting NOHZ_FULL we can do fixups?