Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Message-ID: <37e46985b60839190f65c764da7c2eb2877d53b9.camel@redhat.com>
Subject: Re: [PATCH] timers/nohz: Update nohz load even if tick already
 stopped
From:   Scott Wood <swood@redhat.com>
To:     Thomas Gleixner <tglx@linutronix.de>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Frederic Weisbecker <frederic@kernel.org>,
        Ingo Molnar <mingo@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>
Date:   Fri, 08 Nov 2019 02:16:03 -0600
In-Reply-To: <alpine.DEB.2.21.1911051048190.17054@nanos.tec.linutronix.de>
References: <20191028150716.22890-1-frederic@kernel.org>
           <20191029100506.GJ4114@hirez.programming.kicks-ass.net>
           <52d963553deda810113accd8d69b6dffdb37144f.camel@redhat.com>
           <20191030133130.GY4097@hirez.programming.kicks-ass.net>
          <813ed21938aa47b15f35f8834ffd98ad4dd27771.camel@redhat.com>
          <alpine.DEB.2.21.1911042315390.17054@nanos.tec.linutronix.de>
          <alpine.DEB.2.21.1911050042250.17054@nanos.tec.linutronix.de>
         <7b782bc880a29eb7d37f2c2aff73c43e7f7d032f.camel@redhat.com>
         <alpine.DEB.2.21.1911051048190.17054@nanos.tec.linutronix.de>
Organization: Red Hat
User-Agent: Evolution 3.30.5 (3.30.5-1.fc29)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, 2019-11-05 at 10:53 +0100, Thomas Gleixner wrote:
> On Tue, 5 Nov 2019, Scott Wood wrote:
> > On Tue, 2019-11-05 at 00:43 +0100, Thomas Gleixner wrote:
> > > As Peter pointed out to me privately we should rather go and analyze
> > > the
> > > real thing instead of just applying duct tape.
> > >=20
> > > /me drops the patch again.
> >=20
> > The warning is due to kernel/sched/idle.c not updating curr-
> > >se.exec_start.
> >=20
> > While debugging I noticed an issue with a particular load pattern.  The
> > CPU
> > goes non-nohz for a brief time at an interval very close to twice=20
> > tick_period.  When the tick is started, the timer expiration is more
> > than
> > tick_period in the past, so hrtimer_forward() tries to catch up by
> > adding
> > 2*tick_period to the expiration.  Then the tick is stopped before that
> > new
> > expiration, and when the tick is woken up the expiry is again advanced
> > by
> > 2*tick_period with the timer never actually
> > running.  sched_tick_remote()
> > does fire every second, but there are streaks of several seconds where
> > it
> > keeps catching the CPU in a non-nohz state, so neither the normal nor
> > remote
> > ticks are calling calc_load_nohz_remote().
> >=20
> > Is there a reason to not just remove the hrtimer_forward() from
> > tick_nohz_restart(), letting the timer fire if it's in the past, which
> > will
> > take care of doing hrtimer_forward()?
>=20
> Well, no. tick_nohz_restart() can be invoked in a situation where the
> timer
> is armed for something in the far future (or completelt disabled) due to
> previously entering an estimated long idle (or user space execution on
> NOHZ_FULL) period.
>=20
> That means if the timer is not canceled, realigned to the current tick an=
d
> forwarded to the next due tick, the tick will not fire on time causing
> another sort of trouble.

That might be true of the expiry on entering tick_nohz_restart(), but it
shouldn't be true of ts->last_tick which the expiry is set to before callin=
g
hrtimer_forward() -- and if it were, hrtimer_forward() is a no-op when the
expiry is in the future.

BTW, the name "last_tick" seems misleading as it's actually the saved
expiry, not the last time the tick ran.

-Scott