[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <52d963553deda810113accd8d69b6dffdb37144f.camel@redhat.com>
Date: Wed, 30 Oct 2019 03:48:26 -0500
From: Scott Wood <swood@...hat.com>
To: Peter Zijlstra <peterz@...radead.org>,
Frederic Weisbecker <frederic@...nel.org>
Cc: Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...nel.org>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] timers/nohz: Update nohz load even if tick already
stopped
On Tue, 2019-10-29 at 11:05 +0100, Peter Zijlstra wrote:
> On Mon, Oct 28, 2019 at 04:07:16PM +0100, Frederic Weisbecker wrote:
> > From: Scott Wood <swood@...hat.com>
> >
> > The way loadavg is tracked during nohz only pays attention to the load
> > upon entering nohz. This can be particularly noticeable if nohz is
> > entered while non-idle, and then the cpu goes idle and stays that way
> > for
> > a long time. We've had reports of a loadavg near 150 on a mostly idle
> > system.
> >
> > Calling calc_load_nohz_start() regardless of whether the tick is already
> > stopped addresses the issue when going idle. Tracking load changes when
> > not going idle (e.g. multiple SCHED_FIFO tasks coming and going) is not
> > addressed by this patch.
>
> Hurph, is that phenomena you describe NOHZ or NOHZ_FULL? Because that
> second thing you talk about, multiple SCHED_FIFO tasks running without a
> tick is definitely NOHZ_FULL.
>
> I'm thinking all of this is NOHZ_FULL because IIRC we always start the
> tick when there is a runnable task. So your example of going idle in
> NOHZ already cannot happen for regular NOHZ.
Yes, NOHZ_FULL (sorry for not stating that clearly).
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index eb42b71faab9..209e50d48f80 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -3666,6 +3666,8 @@ static void sched_tick_remote(struct work_struct
> *work)
> * having one too much is no big deal because the scheduler tick
> updates
> * statistics and checks timeslices in a time-independent way,
> regardless
> * of when exactly it is running.
> + *
> + * XXX should we be checking tick_nohz_tick_stopped_cpu() under rq-
> >lock ?
> */
> if (idle_cpu(cpu) || !tick_nohz_tick_stopped_cpu(cpu))
> goto out_requeue;
> @@ -3686,6 +3688,7 @@ static void sched_tick_remote(struct work_struct
> *work)
> curr->sched_class->task_tick(rq, curr, 0);
>
> out_unlock:
> + calc_load_nohz_remote(cpu);
> rq_unlock_irq(rq, &rf);
This gets skipped when the cpu is idle, so it still misses the update.
-Scott
Powered by blists - more mailing lists