Date: Mon, 25 Sep 2017 15:40:57 -0400
From: Steven Rostedt <rostedt@goodmis.org>
To: Zhou Chengming <zhouchengming1@huawei.com>
Cc: <linux-kernel@vger.kernel.org>, <mingo@redhat.com>,
        <peterz@infradead.org>, <huawei.libin@huawei.com>
Subject: Re: [PATCH] sched/rt.c: pick and check task if
 double_lock_balance() unlock the rq
Message-ID: <20170925154057.191e3fd1@vmware.local.home>
In-Reply-To: <1505112709-102019-1-git-send-email-zhouchengming1@huawei.com>
References: <1505112709-102019-1-git-send-email-zhouchengming1@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1680
Lines: 56

On Mon, 11 Sep 2017 14:51:49 +0800
Zhou Chengming <zhouchengming1@huawei.com> wrote:

> push_rt_task() pick the first pushable task and find an eligible
> lowest_rq, then double_lock_balance(rq, lowest_rq). So if
> double_lock_balance() unlock the rq (when double_lock_balance() return 1),
> we have to check if this task is still on the rq.
> 
> The problem is that the check conditions are not sufficient:
> 
> if (unlikely(task_rq(task) != rq ||
> 	     !cpumask_test_cpu(lowest_rq->cpu, &task->cpus_allowed) ||
> 	     task_running(rq, task) ||
> 	     !rt_task(task) ||
> 	     !task_on_rq_queued(task))) {
> 
> cpu2				cpu1			cpu0
> push_rt_task(rq1)
>   pick task_A on rq1
>   find rq0
>     double_lock_balance(rq1, rq0)
>       unlock(rq1)
> 				rq1 __schedule
> 				  pick task_A run
> 				task_A sleep (dequeued)
>       lock(rq0)
>       lock(rq1)
>     do_above_check(task_A)
>       task_rq(task_A) == rq1
>       cpus_allowed unchanged
>       task_running == false
>       rt_task(task_A) == true
> 							try_to_wake_up(task_A)
> 							  select_cpu = cpu3
> 							  enqueue(rq3, task_A)

How can this happen? The try_to_wake_up(task_A) needs to grab the rq
that task A is on, and we have that rq lock. 

/me confused.

-- Steve


> 							  task_A->on_rq = 1
>       task_on_rq_queued(task_A)
>     above_check passed, return rq0
>     ...
>     migrate task_A from rq1 to rq0
> 
> So we can't rely on these checks of task_A to make sure the task_A is
> still on the rq1, even though we hold the rq1->lock. This patch will
> repick the first pushable task to be sure the task is still on the rq.
> 
> Signed-off-by: Zhou Chengming <zhouchengming1@huawei.com>
>