Re: [PATCH 2/7] sched/uclamp: Make task_fits_capacity() use util_fits_cpu()

[Qais Yousef <qais.yousef@arm.com>]

On 07/20/22 15:23, Xuewen Yan wrote:
> On Thu, Jun 30, 2022 at 3:48 AM Qais Yousef <qais.yousef@arm.com> wrote:
> >
> > So that the new uclamp rules in regard to migration margin and capacity
> > pressure are taken into account correctly.
> >
> > To cater for update_sg_wakeup_stats() user, we add new
> > {min,max}_capacity_cpu to struct sched_group_capacity since
> > util_fits_cpu() takes the cpu rather than capacity as an argument.
> >
> > This includes updating capacity_greater() definition to take cpu as an
> > argument instead of capacity.
> >
> > Fixes: a7008c07a568 ("sched/fair: Make task_fits_capacity() consider uclamp restrictions")
> > Signed-off-by: Qais Yousef <qais.yousef@arm.com>
> > ---
> >  kernel/sched/fair.c     | 67 ++++++++++++++++++++++++++---------------
> >  kernel/sched/sched.h    | 13 ++++++--
> >  kernel/sched/topology.c | 18 ++++++-----
> >  3 files changed, 64 insertions(+), 34 deletions(-)
> >
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 5eecae32a0f6..313437bea5a2 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -160,7 +160,7 @@ int __weak arch_asym_cpu_priority(int cpu)
> >   *
> >   * (default: ~5%)
> >   */
> > -#define capacity_greater(cap1, cap2) ((cap1) * 1024 > (cap2) * 1078)
> > +#define capacity_greater(cpu1, cpu2) ((capacity_of(cpu1)) * 1024 > (capacity_of(cpu2)) * 1078)
> >  #endif
> >
> >  #ifdef CONFIG_CFS_BANDWIDTH
> > @@ -4317,10 +4317,12 @@ static inline int util_fits_cpu(unsigned long util,
> >         return fits;
> >  }
> >
> > -static inline int task_fits_capacity(struct task_struct *p,
> > -                                    unsigned long capacity)
> > +static inline int task_fits_cpu(struct task_struct *p, int cpu)
> >  {
> > -       return fits_capacity(uclamp_task_util(p), capacity);
> > +       unsigned long uclamp_min = uclamp_eff_value(p, UCLAMP_MIN);
> > +       unsigned long uclamp_max = uclamp_eff_value(p, UCLAMP_MAX);
> > +       unsigned long util = task_util_est(p);
> > +       return util_fits_cpu(util, uclamp_min, uclamp_max, cpu);
> >  }
> 
> May we should consider the CONFIG_UCLAMP_TASK...

The uclamp functions are protected with CONFIG_UCLAMP_TASK and should result in
dummy implementation and dead code to be compiled out.

It avoids sprinkling ifdefs all over the place this way.


Cheers

--
Qais Yousef