Re: [PATCH v6 3/3] sched/fair: Combine EAS check with overutilized access

From: Ingo Molnar
Date: Tue Mar 26 2024 - 03:58:43 EST



* Shrikanth Hegde <sshegde@xxxxxxxxxxxxx> wrote:

> /*
> - * Ensure that caller can do EAS. overutilized value
> - * make sense only if EAS is enabled
> + * overutilized value make sense only if EAS is enabled
> */
> -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);
> }

While adding the sched_energy_enabled() condition looks OK, the _not prefix
This is silly: putting logical operators into functions names is far less
readable than a !fn()...

> - if (!is_rd_overutilized(rq->rd) && cpu_overutilized(rq->cpu))
> + if (is_rd_not_overutilized(rq->rd) && cpu_overutilized(rq->cpu))

Especially since we already have cpu_overutilized(). It's far more coherent
to have the same basic attribute functions and put any negation into
*actual* logical operators.

Thanks,

Ingo