Re: [RFC PATCH v4 00/19] Core scheduling v4

From: "Li, Aubrey" <aubrey.li@linux.intel.com>
To: Tim Chen <tim.c.chen@linux.intel.com>,
	Vineeth Remanan Pillai <vpillai@digitalocean.com>,
	Aubrey Li <aubrey.intel@gmail.com>
Cc: "Aaron Lu" <aaron.lwe@gmail.com>,
	"Julien Desfossez" <jdesfossez@digitalocean.com>,
	"Nishanth Aravamudan" <naravamudan@digitalocean.com>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Ingo Molnar" <mingo@kernel.org>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Paul Turner" <pjt@google.com>,
	"Linus Torvalds" <torvalds@linux-foundation.org>,
	"Linux List Kernel Mailing" <linux-kernel@vger.kernel.org>,
	"Dario Faggioli" <dfaggioli@suse.com>,
	"Frédéric Weisbecker" <fweisbec@gmail.com>,
	"Kees Cook" <keescook@chromium.org>,
	"Greg Kerr" <kerrnel@google.com>, "Phil Auld" <pauld@redhat.com>,
	"Valentin Schneider" <valentin.schneider@arm.com>,
	"Mel Gorman" <mgorman@techsingularity.net>,
	"Pawan Gupta" <pawan.kumar.gupta@linux.intel.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>
Subject: Re: [RFC PATCH v4 00/19] Core scheduling v4
Date: Tue, 3 Mar 2020 22:59:25 +0800	[thread overview]
Message-ID: <a55bb7a5-bb20-d3f3-e634-4dfda1ac6005@linux.intel.com> (raw)
In-Reply-To: <bcd601e7-3f15-e340-bebe-a6ca3635dacb@linux.intel.com>

On 2020/2/29 7:55, Tim Chen wrote:
> On 2/26/20 1:54 PM, Vineeth Remanan Pillai wrote:
> 
>> rq->curr being NULL can mean that the sibling is idle or forced idle.
>> In both the cases, I think it makes sense to migrate a task so that it can
>> compete with the other sibling for a chance to run. This function
>> can_migrate_task actually only says if this task is eligible and
>> later part of the code decides whether it is okay to migrate it
>> based on factors like load and util and capacity. So I think its
>> fine to declare the task as eligible if the dest core is running
>> idle. Does this thinking make sense?
>>
>> On our testing, it did not show much degradation in performance with
>> this change. I am reworking the fix by removing the check for
>> task_est_util. It doesn't seem to be valid to check for util to migrate
>> the task.
>>
> 
> In Aaron's test case, there is a great imbalance in the load on one core
> where all the grp A tasks are vs the other cores where the grp B tasks are
> spread around.  Normally, load balancer will move the tasks for grp A.
> 
> Aubrey's can_migrate_task patch prevented the load balancer to migrate tasks if the core
> cookie on the target queue don't match.  The thought was it will induce
> force idle and reduces cpu utilization if we migrate task to it.
> That kept all the grp A tasks from getting migrated and kept the imbalance
> indefinitely in Aaron's test case.
> 
> Perhaps we should also look at the load imbalance between the src rq and
> target rq.  If the imbalance is big (say two full cpu bound tasks worth
> of load), we should migrate anyway despite the cookie mismatch.  We are willing
> to pay a bit for the force idle by balancing the load out more.
> I think Aubrey's patch on can_migrate_task should be more friendly to
> Aaron's test scenario if such logic is incorporated.
> 
> In Vinnet's fix, we only look at the currently running task's weight in
> src and dst rq.  Perhaps the load on the src and dst rq needs to be considered
> to prevent too great an imbalance between the run queues?

We are trying to migrate a task, can we just use cfs.h_nr_running? This signal
is used to find the busiest run queue as well.

Thanks,
-Aubrey