From: Suren Baghdasaryan <surenb@google.com> To: Patrick Bellasi <patrick.bellasi@arm.com> Cc: linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, Ingo Molnar <mingo@redhat.com>, Peter Zijlstra <peterz@infradead.org>, Tejun Heo <tj@kernel.org>, "Rafael J . Wysocki" <rafael.j.wysocki@intel.com>, Viresh Kumar <viresh.kumar@linaro.org>, Vincent Guittot <vincent.guittot@linaro.org>, Paul Turner <pjt@google.com>, Dietmar Eggemann <dietmar.eggemann@arm.com>, Morten Rasmussen <morten.rasmussen@arm.com>, Juri Lelli <juri.lelli@redhat.com>, Todd Kjos <tkjos@google.com>, Joel Fernandes <joelaf@google.com>, Steve Muckle <smuckle@google.com> Subject: Re: [PATCH v2 07/12] sched/core: uclamp: enforce last task UCLAMP_MAX Date: Fri, 20 Jul 2018 18:23:57 -0700 Message-ID: <CAJuCfpH6CsDVG35Z3NV87mUgJW9TmoCuOs8GeW2TEqkP0K0f9g@mail.gmail.com> (raw) In-Reply-To: <20180716082906.6061-8-patrick.bellasi@arm.com> Hi Patrick, On Mon, Jul 16, 2018 at 1:29 AM, Patrick Bellasi <patrick.bellasi@arm.com> wrote: > When a util_max clamped task sleeps, its clamp constraints are removed > from the CPU. However, the blocked utilization on that CPU can still be > higher than the max clamp value enforced while that task was running. > This max clamp removal when a CPU is going to be idle could thus allow > unwanted CPU frequency increases, right while the task is not running. > > This can happen, for example, where there is another (smaller) task > running on a different CPU of the same frequency domain. > In this case, when we aggregates the utilization of all the CPUs in a typo: we aggregate > shared frequency domain, schedutil can still see the full non clamped > blocked utilization of all the CPUs and thus eventually increase the > frequency. > > Let's fix this by using: > > uclamp_cpu_put_id(UCLAMP_MAX) > uclamp_cpu_update(last_clamp_value) > > to detect when a CPU has no more RUNNABLE clamped tasks and to flag this > condition. Thus, while a CPU is idle, we can still enforce the last used > clamp value for it. > > To the contrary, we do not track any UCLAMP_MIN since, while a CPU is > idle, we don't want to enforce any minimum frequency > Indeed, we relay just on blocked load decay to smoothly reduce the typo: We rely > frequency. > > Signed-off-by: Patrick Bellasi <patrick.bellasi@arm.com> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Cc: Viresh Kumar <viresh.kumar@linaro.org> > Cc: Todd Kjos <tkjos@google.com> > Cc: Joel Fernandes <joelaf@google.com> > Cc: Juri Lelli <juri.lelli@redhat.com> > Cc: Dietmar Eggemann <dietmar.eggemann@arm.com> > Cc: Morten Rasmussen <morten.rasmussen@arm.com> > Cc: linux-kernel@vger.kernel.org > Cc: linux-pm@vger.kernel.org > --- > kernel/sched/core.c | 30 ++++++++++++++++++++++++++---- > kernel/sched/sched.h | 2 ++ > 2 files changed, 28 insertions(+), 4 deletions(-) > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index b2424eea7990..0cb6e0aa4faa 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -930,7 +930,8 @@ uclamp_group_find(int clamp_id, unsigned int clamp_value) > * For the specified clamp index, this method computes the new CPU utilization > * clamp to use until the next change on the set of RUNNABLE tasks on that CPU. > */ > -static inline void uclamp_cpu_update(struct rq *rq, int clamp_id) > +static inline void uclamp_cpu_update(struct rq *rq, int clamp_id, > + unsigned int last_clamp_value) > { > struct uclamp_group *uc_grp = &rq->uclamp.group[clamp_id][0]; > int max_value = UCLAMP_NONE; > @@ -948,6 +949,19 @@ static inline void uclamp_cpu_update(struct rq *rq, int clamp_id) > if (max_value >= SCHED_CAPACITY_SCALE) > break; > } > + > + /* > + * Just for the UCLAMP_MAX value, in case there are no RUNNABLE > + * task, we keep the CPU clamped to the last task's clamp value. > + * This avoids frequency spikes to MAX when one CPU, with an high > + * blocked utilization, sleeps and another CPU, in the same frequency > + * domain, do not see anymore the clamp on the first CPU. > + */ > + if (clamp_id == UCLAMP_MAX && max_value == UCLAMP_NONE) { > + rq->uclamp.flags |= UCLAMP_FLAG_IDLE; > + max_value = last_clamp_value; > + } > + > rq->uclamp.value[clamp_id] = max_value; > } > > @@ -977,13 +991,21 @@ static inline void uclamp_cpu_get_id(struct task_struct *p, > uc_grp = &rq->uclamp.group[clamp_id][0]; > uc_grp[group_id].tasks += 1; > > + /* Force clamp update on idle exit */ > + uc_cpu = &rq->uclamp; > + clamp_value = p->uclamp[clamp_id].value; > + if (unlikely(uc_cpu->flags & UCLAMP_FLAG_IDLE)) { The condition below is not needed because UCLAMP_FLAG_IDLE is set only for UCLAMP_MAX clamp_id, therefore the above condition already covers the one below. > + if (clamp_id == UCLAMP_MAX) > + uc_cpu->flags &= ~UCLAMP_FLAG_IDLE; > + uc_cpu->value[clamp_id] = clamp_value; > + return; > + } > + > /* > * If this is the new max utilization clamp value, then we can update > * straight away the CPU clamp value. Otherwise, the current CPU clamp > * value is still valid and we are done. > */ > - uc_cpu = &rq->uclamp; > - clamp_value = p->uclamp[clamp_id].value; > if (uc_cpu->value[clamp_id] < clamp_value) > uc_cpu->value[clamp_id] = clamp_value; > } > @@ -1028,7 +1050,7 @@ static inline void uclamp_cpu_put_id(struct task_struct *p, > uc_cpu = &rq->uclamp; > clamp_value = uc_grp[group_id].value; > if (clamp_value >= uc_cpu->value[clamp_id]) > - uclamp_cpu_update(rq, clamp_id); > + uclamp_cpu_update(rq, clamp_id, clamp_value); > } > > /** > diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h > index 1207add36478..7e4f10c507b7 100644 > --- a/kernel/sched/sched.h > +++ b/kernel/sched/sched.h > @@ -783,6 +783,8 @@ struct uclamp_group { > * values, i.e. no min/max clamping at all. > */ > struct uclamp_cpu { > +#define UCLAMP_FLAG_IDLE 0x01 > + int flags; > int value[UCLAMP_CNT]; > struct uclamp_group group[UCLAMP_CNT][CONFIG_UCLAMP_GROUPS_COUNT + 1]; > }; > -- > 2.17.1 >
next prev parent reply index Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-07-16 8:28 [PATCH v2 00/12] Add utilization clamping support Patrick Bellasi 2018-07-16 8:28 ` [PATCH v2 01/12] sched/core: uclamp: extend sched_setattr to support utilization clamping Patrick Bellasi 2018-07-17 17:50 ` Joel Fernandes 2018-07-18 8:42 ` Patrick Bellasi 2018-07-18 17:02 ` Joel Fernandes 2018-07-17 18:04 ` Joel Fernandes 2018-07-16 8:28 ` [PATCH v2 02/12] sched/core: uclamp: map TASK's clamp values into CPU's clamp groups Patrick Bellasi 2018-07-19 23:51 ` Suren Baghdasaryan 2018-07-20 15:11 ` Patrick Bellasi 2018-07-21 0:25 ` Suren Baghdasaryan 2018-07-23 13:36 ` Patrick Bellasi 2018-07-16 8:28 ` [PATCH v2 03/12] sched/core: uclamp: add CPU's clamp groups accounting Patrick Bellasi 2018-07-20 20:25 ` Suren Baghdasaryan 2018-07-16 8:28 ` [PATCH v2 04/12] sched/core: uclamp: update CPU's refcount on clamp changes Patrick Bellasi 2018-07-16 8:28 ` [PATCH v2 05/12] sched/cpufreq: uclamp: add utilization clamping for FAIR tasks Patrick Bellasi 2018-07-16 8:29 ` [PATCH v2 06/12] sched/cpufreq: uclamp: add utilization clamping for RT tasks Patrick Bellasi 2018-07-16 8:29 ` [PATCH v2 07/12] sched/core: uclamp: enforce last task UCLAMP_MAX Patrick Bellasi 2018-07-21 1:23 ` Suren Baghdasaryan [this message] 2018-07-23 15:02 ` Patrick Bellasi 2018-07-23 16:40 ` Suren Baghdasaryan 2018-07-16 8:29 ` [PATCH v2 08/12] sched/core: uclamp: extend cpu's cgroup controller Patrick Bellasi 2018-07-21 2:37 ` Suren Baghdasaryan 2018-07-21 3:16 ` Suren Baghdasaryan 2018-07-23 15:17 ` Patrick Bellasi 2018-07-23 15:30 ` Tejun Heo 2018-07-23 17:22 ` Patrick Bellasi 2018-07-24 13:29 ` Tejun Heo 2018-07-24 15:39 ` Patrick Bellasi 2018-07-27 0:39 ` Joel Fernandes 2018-07-27 8:09 ` Quentin Perret 2018-07-16 8:29 ` [PATCH v2 09/12] sched/core: uclamp: map TG's clamp values into CPU's clamp groups Patrick Bellasi 2018-07-16 8:29 ` [PATCH v2 10/12] sched/core: uclamp: use TG's clamps to restrict Task's clamps Patrick Bellasi 2018-07-22 3:05 ` Suren Baghdasaryan 2018-07-23 15:40 ` Patrick Bellasi 2018-07-23 17:11 ` Suren Baghdasaryan 2018-07-24 9:56 ` Patrick Bellasi 2018-07-24 15:28 ` Suren Baghdasaryan 2018-07-24 15:49 ` Patrick Bellasi 2018-07-16 8:29 ` [PATCH v2 11/12] sched/core: uclamp: update CPU's refcount on TG's clamp changes Patrick Bellasi 2018-07-22 3:17 ` Suren Baghdasaryan 2018-07-16 8:29 ` [PATCH v2 12/12] sched/core: uclamp: use percentage clamp values Patrick Bellasi 2018-07-22 4:04 ` Suren Baghdasaryan 2018-07-24 16:43 ` Patrick Bellasi 2018-07-24 17:11 ` Suren Baghdasaryan 2018-07-24 17:17 ` Patrick Bellasi 2018-07-17 13:03 ` [PATCH v2 00/12] Add utilization clamping support Joel Fernandes 2018-07-17 13:41 ` Patrick Bellasi
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAJuCfpH6CsDVG35Z3NV87mUgJW9TmoCuOs8GeW2TEqkP0K0f9g@mail.gmail.com \ --to=surenb@google.com \ --cc=dietmar.eggemann@arm.com \ --cc=joelaf@google.com \ --cc=juri.lelli@redhat.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=mingo@redhat.com \ --cc=morten.rasmussen@arm.com \ --cc=patrick.bellasi@arm.com \ --cc=peterz@infradead.org \ --cc=pjt@google.com \ --cc=rafael.j.wysocki@intel.com \ --cc=smuckle@google.com \ --cc=tj@kernel.org \ --cc=tkjos@google.com \ --cc=vincent.guittot@linaro.org \ --cc=viresh.kumar@linaro.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
LKML Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \ linux-kernel@vger.kernel.org public-inbox-index lkml Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel AGPL code for this site: git clone https://public-inbox.org/public-inbox.git