[PATCH] Add missing kernel log when enabling NO_HZ_FULL is not possible

[PATCH] Add missing kernel log when enabling NO_HZ_FULL is not possible

[Ani Sinha]

Commit 4f49b90abb4aca ("sched-clock: Migrate to use new tick
dependency mask model") had also removed the kernel warning
message informing the user that it was not possible to turn
on NO_HZ_FULL. Adding back that log message here. It is
unhelpful when the kernel turns off NO_HZ_FULL silently
without informing anyone.

Signed-off-by: Ani Sinha <ani@anisinha.ca>
---
 kernel/time/tick-sched.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 828b091501ca..a82480c036e2 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -937,10 +937,18 @@ static void tick_nohz_full_update_tick(struct tick_sched *ts)
 	if (!ts->tick_stopped && ts->nohz_mode == NOHZ_MODE_INACTIVE)
 		return;
 
-	if (can_stop_full_tick(cpu, ts))
+	if (can_stop_full_tick(cpu, ts)) {
 		tick_nohz_stop_sched_tick(ts, cpu);
-	else if (ts->tick_stopped)
-		tick_nohz_restart_sched_tick(ts, ktime_get());
+	} else {
+		/*
+		 * Don't allow the user to think they can get
+		 * full NO_HZ with this machine.
+		 */
+		WARN_ONCE(tick_nohz_full_running,
+			  "NO_HZ_FULL will not work with current sched clock");
+		if (ts->tick_stopped)
+			tick_nohz_restart_sched_tick(ts, ktime_get());
+	}
 #endif
 }
 
-- 
2.25.1

Re: [PATCH] Add missing kernel log when enabling NO_HZ_FULL is not possible

[Thomas Gleixner]

Ani,

On Fri, Jun 11 2021 at 16:09, Ani Sinha wrote:
> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index 828b091501ca..a82480c036e2 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -937,10 +937,18 @@ static void tick_nohz_full_update_tick(struct tick_sched *ts)
>  	if (!ts->tick_stopped && ts->nohz_mode == NOHZ_MODE_INACTIVE)
>  		return;
>  
> -	if (can_stop_full_tick(cpu, ts))
> +	if (can_stop_full_tick(cpu, ts)) {
>  		tick_nohz_stop_sched_tick(ts, cpu);
> -	else if (ts->tick_stopped)
> -		tick_nohz_restart_sched_tick(ts, ktime_get());
> +	} else {
> +		/*
> +		 * Don't allow the user to think they can get
> +		 * full NO_HZ with this machine.
> +		 */
> +		WARN_ONCE(tick_nohz_full_running,
> +			  "NO_HZ_FULL will not work with current sched clock");

How is that warning useful and even remotely correct?

can_stop_full_tick() has 4 x 5 == 20 ways to return false and the
smaller portion of them is related to sched clock.

Thanks,

        tglx

Re: [PATCH] Add missing kernel log when enabling NO_HZ_FULL is not possible

[Ani Sinha]

On Sat, Jun 19, 2021 at 1:32 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> Ani,
>
> On Fri, Jun 11 2021 at 16:09, Ani Sinha wrote:
> > diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> > index 828b091501ca..a82480c036e2 100644
> > --- a/kernel/time/tick-sched.c
> > +++ b/kernel/time/tick-sched.c
> > @@ -937,10 +937,18 @@ static void tick_nohz_full_update_tick(struct tick_sched *ts)
> >       if (!ts->tick_stopped && ts->nohz_mode == NOHZ_MODE_INACTIVE)
> >               return;
> >
> > -     if (can_stop_full_tick(cpu, ts))
> > +     if (can_stop_full_tick(cpu, ts)) {
> >               tick_nohz_stop_sched_tick(ts, cpu);
> > -     else if (ts->tick_stopped)
> > -             tick_nohz_restart_sched_tick(ts, ktime_get());
> > +     } else {
> > +             /*
> > +              * Don't allow the user to think they can get
> > +              * full NO_HZ with this machine.
> > +              */
> > +             WARN_ONCE(tick_nohz_full_running,
> > +                       "NO_HZ_FULL will not work with current sched clock");
>
> How is that warning useful and even remotely correct?
>
> can_stop_full_tick() has 4 x 5 == 20 ways to return false and the
> smaller portion of them is related to sched clock.


Ok I admit I don't fully understand all the sched dep bits. There are
two ways we can solve this. Either we adjust the log message to
reflect that no hz cannot be enabled on this system in general or we
can specifically isolate the conditions that check for sched clock and
for those we add the warning message. Personally, I think we need some
kind of information on the kernel log when no hz enabling is not
possible. Silently disabling it can and did cause confusion that no hz
works fine ( particularly when the older kernel did produce the
warning). I am inclined towards keeping the log where it is but making
it more general and not specific to sched clock.

Ani