Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org D4C5FC433CB
Date:   Wed, 11 Mar 2020 16:23:58 +0530
From:   Pavan Kondeti <pkondeti@codeaurora.org>
To:     Qais Yousef <qais.yousef@arm.com>
Cc:     Ingo Molnar <mingo@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Dietmar Eggemann <dietmar.eggemann@arm.com>,
        Juri Lelli <juri.lelli@redhat.com>,
        Vincent Guittot <vincent.guittot@linaro.org>,
        Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 6/6] sched/rt: Fix pushing unfit tasks to a better CPU
Message-ID: <20200311105358.GO28029@codeaurora.org>
References: <20200302132721.8353-1-qais.yousef@arm.com>
 <20200302132721.8353-7-qais.yousef@arm.com>
 <20200306175112.vkpeouec2c47yujl@e107158-lin.cambridge.arm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200306175112.vkpeouec2c47yujl@e107158-lin.cambridge.arm.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Mar 06, 2020 at 05:51:13PM +0000, Qais Yousef wrote:
> Hi Pavan
> 
> On 03/02/20 13:27, Qais Yousef wrote:
> > If a task was running on unfit CPU we could ignore migrating if the
> > priority level of the new fitting CPU is the *same* as the unfit one.
> > 
> > Add an extra check to select_task_rq_rt() to allow the push in case:
> > 
> > 	* p->prio == new_cpu.highest_priority
> > 	* task_fits(p, new_cpu)
> > 
> > Fixes: 804d402fb6f6 ("sched/rt: Make RT capacity-aware")
> > Signed-off-by: Qais Yousef <qais.yousef@arm.com>
> > ---
> 
> Can you please confirm if you have any objection to this patch? Without it
> I see large delays in the 2 tasks test like I outlined in [1]. It wasn't clear
> from [2] whether you are in agreement now or not.
> 
> [1] https://lore.kernel.org/lkml/20200217135306.cjc2225wdlwqiicu@e107158-lin.cambridge.arm.com/
> [2] https://lore.kernel.org/lkml/20200227033608.GN28029@codeaurora.org/
> 

I am not very sure about this. Like we discussed, this patch is addressing a
specific scenario i.e two equal prio tasks waking at the same time. We allow
the packing so that task_woken_rt() spread the tasks. The enqueue operation
is waste here.

At the same time, I can't think of a better alternative. Retrying
find_lowest_rq() may still give the same result until the previous task
is fully woken on the CPU.

btw, the commit description does not talk about the race at all. If there is
no race, we won't even end up in this scenario i.e find_lowest_rq() may simply
return -1.

Thanks,
Pavan

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.