[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <251f9b0d-3e00-4ca7-829b-e622fc0b4c49@linux.ibm.com>
Date: Tue, 5 Mar 2024 20:33:14 +0530
From: Shrikanth Hegde <sshegde@...ux.ibm.com>
To: Vincent Guittot <vincent.guittot@...aro.org>
Cc: mingo@...nel.org, peterz@...radead.org, yu.c.chen@...el.com,
dietmar.eggemann@....com, linux-kernel@...r.kernel.org,
nysal@...ux.ibm.com, aboorvad@...ux.ibm.com, srikar@...ux.ibm.com,
vschneid@...hat.com, pierre.gondois@....com, morten.rasmussen@....com,
qyousef@...alina.io
Subject: Re: [PATCH v4 2/2] sched/fair: Use helper function to access
rd->overutilized
On 3/5/24 7:35 PM, Vincent Guittot wrote:
> On Fri, 1 Mar 2024 at 16:18, Shrikanth Hegde <sshegde@...ux.ibm.com> wrote:
>
>
> It seems that is_rd_overutilized() is always used with
> sched_energy_enabled() in the pattern:
>
> If (sched_energy_enabled() && !is_rd_overutilized(rd))
> do something
>
> This pattern includes feec() case where we have in select_task_rq_fair():
>
> If (sched_energy_enabled())
> feec():
> |-> if (is_rd_overutilized())
> |-> goto unlock
>
> which could be changed into
> If (sched_energy_enabled() && !is_rd_overutilized(rd))
> feec()
>
> Then you can create the function is_rd_not_overutilized() instead of
> is_rd_overutilized()
>
> -static inline int is_rd_overutilized(struct root_domain *rd)
> +static inline int is_rd_not_overutilized(struct root_domain *rd)
> {
> - return READ_ONCE(rd->overutilized);
> + return sched_energy_enabled() && READ_ONCE(rd->overutilized);
> }
>
> and use is_rd_not_overutilized() instead
>
Ok. Makes sense. I will keep this patch as is. and use the above
approach in a new patch.
>> +
>> static inline void set_rd_overutilized_status(struct root_domain *rd,
>> unsigned int status)
>> {
>> @@ -6686,13 +6695,14 @@ static inline void check_update_overutilized_status(struct rq *rq)
>> if (!sched_energy_enabled())
>> return;
>>
>> - if (!READ_ONCE(rq->rd->overutilized) && cpu_overutilized(rq->cpu))
>> + if (!is_rd_overutilized(rq->rd) && cpu_overutilized(rq->cpu))
>> set_rd_overutilized_status(rq->rd, SG_OVERUTILIZED);
>> }
>> #else
>> static inline void check_update_overutilized_status(struct rq *rq) { }
>> static inline void set_rd_overutilized_status(struct root_domain *rd,
>> unsigned int status) { }
>> +static inline int is_rd_overutilized(struct root_domain *rd) { }
>
> It should be
> static inline int is_rd_overutilized(struct root_domain *rd) { return 0; }
ok.
>
>> #endif
>>
>> /* Runqueue only has SCHED_IDLE tasks enqueued */
>> @@ -7974,7 +7984,7 @@ static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu)
>>
>> rcu_read_lock();
>>
Powered by blists - more mailing lists