Received-SPF: pass (google.com: domain of linux-kernel+bounces-90776-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <aa13842e-ab81-45e0-87d4-2b5360ff4782@linux.ibm.com>
Date: Mon, 4 Mar 2024 20:35:26 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 2/9] sched/balancing: Remove reliance on 'enum
 cpu_idle_type' ordering when iterating [CPU_MAX_IDLE_TYPES] arrays in
 show_schedstat()
Content-Language: en-US
To: Ingo Molnar <mingo@kernel.org>, linux-kernel@vger.kernel.org
Cc: Peter Zijlstra <peterz@infradead.org>,
        Vincent Guittot <vincent.guittot@linaro.org>,
        Dietmar Eggemann <dietmar.eggemann@arm.com>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Valentin Schneider <vschneid@redhat.com>
References: <20240304094831.3639338-1-mingo@kernel.org>
 <20240304094831.3639338-3-mingo@kernel.org>
From: Shrikanth Hegde <sshegde@linux.ibm.com>
In-Reply-To: <20240304094831.3639338-3-mingo@kernel.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit


On 3/4/24 3:18 PM, Ingo Molnar wrote:
> From: Shrikanth Hegde <sshegde@linux.ibm.com>
> 
> Shrikanth Hegde reported that show_schedstat() output broke when
> the ordering of the definitions in 'enum cpu_idle_type' is changed,
> because show_schedstat() assumed that 'CPU_IDLE' is 0.
>
Hi Ingo. 
Feel free to drop me from the changelog. 

> @@ -150,8 +150,7 @@ static int show_schedstat(struct seq_file *seq, void *v)
>  
>  			seq_printf(seq, "domain%d %*pb", dcount++,
>  				   cpumask_pr_args(sched_domain_span(sd)));
> -			for (itype = CPU_IDLE; itype < CPU_MAX_IDLE_TYPES;
> -					itype++) {
> +			for (itype = 0; itype < CPU_MAX_IDLE_TYPES; itype++) {


It would still not be same order as current documentation of schedstat. no? The documentation 
would need changes too. Change SCHEDSTAT_VERSION to 16? 

 
Current documentation says this. 
--------------------
The next 24 are a variety of load_balance() statistics in grouped into types
of idleness (idle, busy, and newly idle):

Above code will do. 
(busy, idle and newly idle)


--------------------
Verified with the v3 patch as well using the previous method. 
Before patch:
cpu0 0 0 4400 1485 1624 1229 301472313236 120382198 7714    
				   [-------- idle --------][-----------busy--------][-------new-idle--]                    
domain0 00000000,00000000,00000055 1661 1661 0 0 0 0 0 1661 2495 2495 0 0 0 0 0 2495 67 66 1 2 0 0 0 66 0 0 0 0 0 0 0 0 0 133 38 0
domain1 ff000000,00ff0000,ffffffff 382 369 13 13 4 0 2 207 198 195 3 36 0 0 0 195 67 64 3 3 0 0 0 64 4 0 4 0 0 0 0 0 0 124 9 0
domain2 ff00ffff,00ffffff,ffffffff 586 585 1 6 0 0 0 365 118 116 2 96 0 0 0 116 67 67 0 0 0 0 0 67 0 0 0 0 0 0 0 0 0 59 0 0
domain3 ffffffff,ffffffff,ffffffff 481 479 2 58 0 0 0 387 97 97 0 0 0 0 0 96 67 67 0 0 0 0 0 67 0 0 0 0 0 0 0 0 0 79 0 0

After patch:
cpu0 0 0 3924 728 1940 1540 302019558490 425784368 8793
				   [-------- busy ----------][-----------idle--------][-------new-idle--]   
domain0 00000000,00000000,00000055 2494 2489 3 37 2 0 0 2489 1691 1691 0 0 0 0 0 1691 21 19 0 2 2 0 0 19 0 0 0 0 0 0 0 0 0 89 2 0
domain1 ff000000,00ff0000,ffffffff 196 193 3 44 0 0 0 193 411 400 10 2060 4 1 4 260 19 16 3 1028 0 0 0 16 3 0 3 0 0 0 0 0 0 59 2 0
domain2 ff00ffff,00ffffff,ffffffff 116 116 0 0 0 0 0 116 590 588 2 3 0 0 0 447 19 18 1 2 0 0 0 18 0 0 0 0 0 0 0 0 0 192 0 0
domain3 ffffffff,ffffffff,ffffffff 97 97 0 0 0 0 0 96 457 457 0 0 0 0 0 427 19 18 1 27 0 0 0 18 0 0 0 0 0 0 0 0 0 60 0 0


>  				seq_printf(seq, " %u %u %u %u %u %u %u %u",
>  				    sd->lb_count[itype],
>  				    sd->lb_balanced[itype],