linux-kernel - Re: [PATCH] cpuidle: Always stop scheduler tick on adaptive-tick CPUs

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Wed, 17 Jul 2019 09:55:08 +0200
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Frederic Weisbecker <frederic@...nel.org>
Cc:     "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Linux PM <linux-pm@...r.kernel.org>,
        Thomas Lindroth <thomas.lindroth@...il.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH] cpuidle: Always stop scheduler tick on adaptive-tick CPUs

On Tue, Jul 16, 2019 at 11:40 PM Frederic Weisbecker
<frederic@...nel.org> wrote:
>
> On Tue, Jul 16, 2019 at 05:25:10PM +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> >
> > Running the scheduler tick on idle adaptive-tick CPUs is not useful
>
> Judging by the below change, you mean full dynticks, right?

Right.

> > and it may also be not expected by users (as reported by Thomas), so
> > add a check to cpuidle_idle_call() to always stop the tick on them
> > regardless of the idle duration predicted by the governor.
> >
> > Fixes: 554c8aa8ecad ("sched: idle: Select idle state before stopping the tick")
> > Reported-by: Thomas Lindroth <thomas.lindroth@...il.com>
> > Tested-by: Thomas Lindroth <thomas.lindroth@...il.com>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> > ---
> >  kernel/sched/idle.c |    3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > Index: linux-pm/kernel/sched/idle.c
> > ===================================================================
> > --- linux-pm.orig/kernel/sched/idle.c
> > +++ linux-pm/kernel/sched/idle.c
> > @@ -191,7 +191,8 @@ static void cpuidle_idle_call(void)
> >                */
> >               next_state = cpuidle_select(drv, dev, &stop_tick);
> >
> > -             if (stop_tick || tick_nohz_tick_stopped())
> > +             if (stop_tick || tick_nohz_tick_stopped() ||
> > +                 !housekeeping_cpu(dev->cpu, HK_FLAG_TICK))
>
> But tick_nohz_tick_stopped() also works on full dynticks CPUs. If the
> tick isn't stopped on a full dynticks CPU by the time we reach this path,
> it means that the conditions for the tick to be stopped are not met anyway
> (eg: more than one task and sched tick is needed, perf event requires the tick,
> posix CPU timer, etc...)

First of all, according to Thomas, the patch does make a difference,
so evidently on his system(s) the full dynticks CPUs enter the idle
loop with running tick.

This means that, indeed, the conditions for the tick to be stopped
have not been met up to that point, but if the (full dynticks) CPU
becomes idle, that's because it has been made idle on purpose
(presumably by a user-space "orchestrator" or the sysadmin), so the
kernel can assume that it will remain idle indefinitely.  That, in
turn, is when the tick would be stopped on it regardless of everything
else (even if it wasn't a full dynticks CPU).

I guess I should add the above to the changelog.

> Or am I missing something else?

Well, if full dynticks CPUs are expected to always enter the idle loop
with stopped tick, then something appears to be amiss, but I'm not
sure if that expectation is entirely realistic.

Cheers!