Re: git: cb1f5d11366d - main - Reduce minimum idle hardclock rate from 2Hz to 1Hz.

On Mon, Jan 10, 2022 at 12:26:01AM +0000, Alexander Motin wrote:
> The branch main has been updated by mav:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=cb1f5d11366dc9b803f2755d83fe02599830882a
> 
> commit cb1f5d11366dc9b803f2755d83fe02599830882a
> Author:     Alexander Motin <mav@FreeBSD.org>
> AuthorDate: 2022-01-10 00:17:43 +0000
> Commit:     Alexander Motin <mav@FreeBSD.org>
> CommitDate: 2022-01-10 00:25:56 +0000
> 
>     Reduce minimum idle hardclock rate from 2Hz to 1Hz.
>     
>     On idle 80-thread system it allows to improve package-level idle state
>     residency and so power consumption by several percent.
Are you sure that this is safe?

Our timecounters are tailored to wrap at 1sec, at least the most precise/
important TSC-low is.  If idle system interrupts with 1Hz frequency,
it probably comes dangerously close to situations where tc_windup() could
be missed.

>     
>     MFC after:      2 weeks
> ---
>  sys/kern/kern_clocksource.c | 5 +++--
>  sys/kern/kern_timeout.c     | 5 ++---
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/sys/kern/kern_clocksource.c b/sys/kern/kern_clocksource.c
> index dfc9081ba9f6..48e06ee082fc 100644
> --- a/sys/kern/kern_clocksource.c
> +++ b/sys/kern/kern_clocksource.c
> @@ -239,13 +239,14 @@ getnextcpuevent(int idle)
>  	/* Handle hardclock() events, skipping some if CPU is idle. */
>  	event = state->nexthard;
>  	if (idle) {
> -		hardfreq = (u_int)hz / 2;
> -		if (tc_min_ticktock_freq > 2
> +		if (tc_min_ticktock_freq > 1
>  #ifdef SMP
>  		    && curcpu == CPU_FIRST()
>  #endif
>  		    )
>  			hardfreq = hz / tc_min_ticktock_freq;
> +		else
> +			hardfreq = hz;
>  		if (hardfreq > 1)
>  			event += tick_sbt * (hardfreq - 1);
>  	}
> diff --git a/sys/kern/kern_timeout.c b/sys/kern/kern_timeout.c
> index 91882ddb5fba..d0fb19661fa4 100644
> --- a/sys/kern/kern_timeout.c
> +++ b/sys/kern/kern_timeout.c
> @@ -434,8 +434,7 @@ callout_process(sbintime_t now)
>  	struct callout_cpu *cc;
>  	struct callout_list *sc;
>  	struct thread *td;
> -	sbintime_t first, last, max, tmp_max;
> -	uint32_t lookahead;
> +	sbintime_t first, last, lookahead, max, tmp_max;
>  	u_int firstb, lastb, nowb;
>  #ifdef CALLOUT_PROFILING
>  	int depth_dir = 0, mpcalls_dir = 0, lockcalls_dir = 0;
> @@ -455,7 +454,7 @@ callout_process(sbintime_t now)
>  	else if (nowb - firstb == 1)
>  		lookahead = (SBT_1S / 8);
>  	else
> -		lookahead = (SBT_1S / 2);
> +		lookahead = SBT_1S;
>  	first = last = now;
>  	first += (lookahead / 2);
>  	last += lookahead;