Re: [PATCH 2/2] sched/fair: Use rq in idle_cpu_without()

[Pierre Gondois]

Hello Shrikanth,

On 1/4/24 06:15, Shrikanth Hegde wrote:
> On 1/3/24 6:26 PM, Pierre Gondois wrote:
> > idle_cpu_without() could receive a 'struct rq' instead of a
> > cpu number to avoid converting the cpu number to a 'struct rq'
>
> nit: s/cpu/CPU
>
> > two times. Indeed update_sg_wakeup_stats() already makes the
> > conversion.
>
> This change looks good. There maybe other candidates which might get simplified
> as well. for example, update_blocked_averages. (and then there are some
> like balance_push_set which maybe borderline when it comes to such simplification)

Ok yes, I'll check the functions you pointed out.

> > Signed-off-by: Pierre Gondois <pierre.gondois@xxxxxxx>
> > ---
> >   kernel/sched/fair.c | 8 +++-----
> >   1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 93e928e76959..d38fec26fd3d 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -10184,15 +10184,13 @@ static unsigned int task_running_on_cpu(int cpu, struct task_struct *p)
> >   
> >   /**
> >    * idle_cpu_without - would a given CPU be idle without p ?
> > - * @cpu: the processor on which idleness is tested.
> > + * @rq: the rq on which idleness is tested.
> >    * @p: task which should be ignored.
> >    *
> >    * Return: 1 if the CPU would be idle. 0 otherwise.
> >    */
> > -static int idle_cpu_without(int cpu, struct task_struct *p)
> > +static int idle_cpu_without(struct rq *rq, struct task_struct *p)
>
> This might need change in the function name too. perception here is that, is the
> CPU idle without task p.

Yes right, I'll rename to idle_rq_without() in the next version.

Regards,
Pierre

> Otherwise LGTM.
>
> Reviewed-by: Shrikanth Hegde <sshegde@xxxxxxxxxxxxxxxxxx>
>
> >   {
> > -	struct rq *rq = cpu_rq(cpu);
> > -
> >   	if (rq->curr != rq->idle && rq->curr != p)
> >   		return 0;
> >   
> > @@ -10247,7 +10245,7 @@ static inline void update_sg_wakeup_stats(struct sched_domain *sd,
> >   		/*
> >   		 * No need to call idle_cpu_without() if nr_running is not 0
> >   		 */
> > -		if (!nr_running && idle_cpu_without(i, p))
> > +		if (!nr_running && idle_cpu_without(rq, p))
> >   			sgs->idle_cpus++;
> >   
> >   		/* Check if task fits in the CPU */