Re: [RFC PATCH v2 11/17] sched: Basic tracking of matching tasks

[Tim Chen <tim.c.chen@linux.intel.com>]

On 4/28/19 11:15 PM, Aaron Lu wrote:
> On Tue, Apr 23, 2019 at 04:18:16PM +0000, Vineeth Remanan Pillai wrote:
>> +/*
>> + * Find left-most (aka, highest priority) task matching @cookie.
>> + */
>> +struct task_struct *sched_core_find(struct rq *rq, unsigned long cookie)
>> +{
>> +	struct rb_node *node = rq->core_tree.rb_node;
>> +	struct task_struct *node_task, *match;
>> +
>> +	/*
>> +	 * The idle task always matches any cookie!
>> +	 */
>> +	match = idle_sched_class.pick_task(rq);
>> +
>> +	while (node) {
>> +		node_task = container_of(node, struct task_struct, core_node);
>> +
>> +		if (node_task->core_cookie < cookie) {
>> +			node = node->rb_left;
> 
> Should go right here?
> 

I think Aaron is correct.  We order the rb tree where tasks with smaller core cookies
go to the left part of the tree.

In this case, the cookie we are looking for is larger than the current node's cookie.
It seems like we should move to the right to look for a node with matching cookie.

At least making the following change still allow us to run the system stably for sysbench.
Need to gather more data to see how performance changes.

Tim

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 25638a47c408..ed4cfa49e3f2 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -208,9 +208,9 @@ static struct task_struct *sched_core_find(struct rq *rq, unsigned long cookie)
        while (node) {
                node_task = container_of(node, struct task_struct, core_node);
 
-               if (node_task->core_cookie < cookie) {
+               if (cookie < node_task->core_cookie) {
                        node = node->rb_left;
-               } else if (node_task->core_cookie > cookie) {
+               } else if (cookie > node_task->core_cookie) {
                        node = node->rb_right;
                } else {
                        match = node_task;