diff mbox series

[v2,5/6] sched/fair: pack SD_ASYM_PACKING into sched_use_asym_prio

Message ID 20240130131708.429425-5-alexs@kernel.org
State New
Headers
Series [v2,1/6] sched/fair: add SD_CLUSTER in comments |

Commit Message

alexs@kernel.org Jan. 30, 2024, 1:17 p.m. UTC 
  From: Alex Shi <alexs@kernel.org>

Then the flags check passed into sched_asym and sched_group_asym.
It's a code cleanup, no func changes.

Signed-off-by: Alex Shi <alexs@kernel.org>
To: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
To: Ben Segall <bsegall@google.com>
To: Steven Rostedt <rostedt@goodmis.org>
To: Dietmar Eggemann <dietmar.eggemann@arm.com>
To: Valentin Schneider <vschneid@redhat.com>
To: Daniel Bristot de Oliveira <bristot@redhat.com>
To: Vincent Guittot <vincent.guittot@linaro.org>
To: Juri Lelli <juri.lelli@redhat.com>
To: Peter Zijlstra <peterz@infradead.org>
To: Ingo Molnar <mingo@redhat.com>
---
 kernel/sched/fair.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

Comments

Ricardo Neri Feb. 1, 2024, 12:55 a.m. UTC | #1 
On Tue, Jan 30, 2024 at 09:17:07PM +0800, alexs@kernel.org wrote:
> From: Alex Shi <alexs@kernel.org>
pack SD_ASYM_PACKING into sched_use_asym_prio

Instead of saying `pack` you could say "Check the SD_ASYM_PACKING flag
in sched_use_asym_prio()"

> 
> Then the flags check passed into sched_asym and sched_group_asym.
> It's a code cleanup, no func changes.

I'd the changelog as follows:

"sched_use_asym_prio() checks whether CPU priorities should be used. It
makes sense to check for the SD_ASYM_PACKING() inside the function.
Since both sched_asym() and sched_group_asym() use sched_use_asym_prio(),
remove the now superfluous checks for the flag in various places"
kuiliang Shi Feb. 1, 2024, 8:45 a.m. UTC | #2 
On 2/1/24 8:55 AM, Ricardo Neri wrote:
> On Tue, Jan 30, 2024 at 09:17:07PM +0800, alexs@kernel.org wrote:
>> From: Alex Shi <alexs@kernel.org>
> pack SD_ASYM_PACKING into sched_use_asym_prio
> 
> Instead of saying `pack` you could say "Check the SD_ASYM_PACKING flag
> in sched_use_asym_prio()"
> 
>>
>> Then the flags check passed into sched_asym and sched_group_asym.
>> It's a code cleanup, no func changes.
> 
> I'd the changelog as follows:
> 
> "sched_use_asym_prio() checks whether CPU priorities should be used. It
> makes sense to check for the SD_ASYM_PACKING() inside the function.
> Since both sched_asym() and sched_group_asym() use sched_use_asym_prio(),
> remove the now superfluous checks for the flag in various places"

Thanks a lot for all comments for this series patches, will take your versions in all commits' logs.

Thanks!
Alex
diff mbox series

Patch

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 835dbe77b260..6680cb39c787 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -9741,6 +9741,9 @@  group_type group_classify(unsigned int imbalance_pct,
  */
 static bool sched_use_asym_prio(struct sched_domain *sd, int cpu)
 {
+	if (!(sd->flags & SD_ASYM_PACKING))
+		return false;
+
 	return (!sched_smt_active()) ||
 		(sd->flags & SD_SHARE_CPUCAPACITY) || is_core_idle(cpu);
 }
@@ -9935,11 +9938,9 @@  static inline void update_sg_lb_stats(struct lb_env *env,
 	sgs->group_weight = group->group_weight;
 
 	/* Check if dst CPU is idle and preferred to this group */
-	if (!local_group && env->sd->flags & SD_ASYM_PACKING &&
-	    env->idle != CPU_NOT_IDLE && sgs->sum_h_nr_running &&
-	    sched_group_asym(env, sgs, group)) {
+	if (!local_group && env->idle != CPU_NOT_IDLE && sgs->sum_h_nr_running &&
+			sched_group_asym(env, sgs, group))
 		sgs->group_asym_packing = 1;
-	}
 
 	/* Check for loaded SMT group to be balanced to dst CPU */
 	if (!local_group && smt_balance(env, sgs, group))
@@ -11035,9 +11036,7 @@  static struct rq *find_busiest_queue(struct lb_env *env,
 		 * If balancing between cores, let lower priority CPUs help
 		 * SMT cores with more than one busy sibling.
 		 */
-		if ((env->sd->flags & SD_ASYM_PACKING) &&
-		    sched_asym(env->sd, i, env->dst_cpu) &&
-		    nr_running == 1)
+		if (sched_asym(env->sd, i, env->dst_cpu) && nr_running == 1)
 			continue;
 
 		switch (env->migration_type) {
@@ -11133,7 +11132,7 @@  asym_active_balance(struct lb_env *env)
 	 * the lower priority @env::dst_cpu help it. Do not follow
 	 * CPU priority.
 	 */
-	return env->idle != CPU_NOT_IDLE && (env->sd->flags & SD_ASYM_PACKING) &&
+	return env->idle != CPU_NOT_IDLE &&
 	       sched_use_asym_prio(env->sd, env->dst_cpu) &&
 	       (sched_asym_prefer(env->dst_cpu, env->src_cpu) ||
 		!sched_use_asym_prio(env->sd, env->src_cpu));