On 05/18/20 10:31, Dietmar Eggemann wrote:
> On 11/05/2020 17:40, Qais Yousef wrote:
>
> [..]
>
> > @@ -790,6 +790,26 @@ unsigned int sysctl_sched_uclamp_util_min =
> > SCHED_CAPACITY_SCALE;
> > /* Max allowed maximum utilization */
> > unsigned int sysctl_sched_uclamp_util_max = SCHED_CAPACITY_SCALE;
> >
> > +/*
> > + * By default RT tasks run at the maximum performance point/capacity of the
> > + * system. Uclamp enforces this by always setting UCLAMP_MIN of RT tasks to
> > + * SCHED_CAPACITY_SCALE.
> > + *
> > + * This knob allows admins to change the default behavior when uclamp is
> > being
> > + * used. In battery powered devices, particularly, running at the maximum
> > + * capacity and frequency will increase energy consumption and shorten the
> > + * battery life.
> > + *
> > + * This knob only affects RT tasks that their uclamp_se->user_defined ==
> > false.
>
> Nit pick: Isn't there a verb missing in this sentence?
>
> [...]
>
> > @@ -1114,12 +1161,13 @@ int sysctl_sched_uclamp_handler(struct ctl_table
> > *table, int write,
> > loff_t *ppos)
> > {
> > bool update_root_tg = false;
> > - int old_min, old_max;
> > + int old_min, old_max, old_min_rt;
>
> Nit pick: Order local variable declarations according to length.
>
> [...]
>
> > @@ -1128,7 +1176,9 @@ int sysctl_sched_uclamp_handler(struct ctl_table
> > *table, int write,
> > goto done;
> >
> > if (sysctl_sched_uclamp_util_min > sysctl_sched_uclamp_util_max ||
> > - sysctl_sched_uclamp_util_max > SCHED_CAPACITY_SCALE) {
> > + sysctl_sched_uclamp_util_max > SCHED_CAPACITY_SCALE ||
>
> Nit pick: This extra space looks weird to me.
>
> [...]
>
> Apart from that, LGTM
>
> For both patches of this v5:
>
> Reviewed-by: Dietmar Eggemann <dietmar.eggem...@arm.com>
Thanks Dietmar and Patrick.
Peter, let me know if you'd like me to address the nitpicks or you're okay with
this as-is.
Thanks!
--
Qais Yousef
