From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 13412C433F5 for ; Fri, 7 Jan 2022 18:36:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229887AbiAGSgR (ORCPT ); Fri, 7 Jan 2022 13:36:17 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43978 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229528AbiAGSfw (ORCPT ); Fri, 7 Jan 2022 13:35:52 -0500 Received: from metanate.com (unknown [IPv6:2001:8b0:1628:5005::111]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 78954C061574; Fri, 7 Jan 2022 10:35:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=metanate.com; s=stronger; h=In-Reply-To:Content-Type:References:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-Transfer-Encoding:Content-ID: Content-Description; bh=+Gbab7Fnbxp9CRQp1CY+Df8nwrrWXPr1d5W8ipvlME8=; b=Ne7AU FALCLnVOJabo2OuSO1Hv6r1Ehk9p2dTlls8CCLBntRvC3ffJbtm71+1aSlspKAnpyHwdOkSB6zrE7 3y7O220veg6azUkjAb7JPgnoHSx8vd2Enk+o3o/rAzel8rjCsOOKegLl5TjcL997fhU5G7GVD+rIn vuzvx6kFjFicwqBZCRPioKj5R81p0u+cgACjXfQ9W8jJIYqB7gtw221L4UUzR8X/zNUvqrmz8puap 88zK+cyu7kRndtinjobjQCA2ApL/cUSDx4gewqN3lqUDZzceEKR1EFaK/eP+HuR9Ga8daAeyM835Y dI9yEFH1qXqVbUMG8m4wVhpYhR4HA==; Received: from [81.174.171.191] (helo=donbot) by email.metanate.com with esmtpsa (TLS1.3) tls TLS_AES_256_GCM_SHA384 (Exim 4.93) (envelope-from ) id 1n5u5W-00037k-8f; Fri, 07 Jan 2022 18:35:42 +0000 Date: Fri, 7 Jan 2022 18:35:40 +0000 From: John Keeping To: Dietmar Eggemann Cc: Valentin Schneider , linux-rt-users@vger.kernel.org, Ingo Molnar , Peter Zijlstra , Juri Lelli , Vincent Guittot , Steven Rostedt , Ben Segall , Mel Gorman , Daniel Bristot de Oliveira , linux-kernel@vger.kernel.org Subject: Re: [RT] BUG in sched/cpupri.c Message-ID: References: <71ddbe51-2b7f-2b13-5f22-9013506471dc@arm.com> <87zgou6iq1.mognet@arm.com> <20211221164528.3c84543f.john@metanate.com> <31a47e99-6de3-76ec-62ad-9c98d092ead5@arm.com> <87r1a4775a.mognet@arm.com> <88826618-5ce8-dd1f-c9db-ec273fede3ce@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <88826618-5ce8-dd1f-c9db-ec273fede3ce@arm.com> X-Authenticated: YES Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jan 07, 2022 at 03:25:21PM +0100, Dietmar Eggemann wrote: > On 07/01/2022 12:49, John Keeping wrote: > > On Fri, Jan 07, 2022 at 11:46:45AM +0100, Dietmar Eggemann wrote: > >> On 22/12/2021 20:48, Valentin Schneider wrote: > >>> /* > >>> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c > >>> index ef8228d19382..8f3e3a1367b6 100644 > >>> --- a/kernel/sched/rt.c > >>> +++ b/kernel/sched/rt.c > >>> @@ -1890,6 +1890,16 @@ static int push_rt_task(struct rq *rq, bool pull) > >>> if (!next_task) > >>> return 0; > >>> > >>> + /* > >>> + * It's possible that the next_task slipped in of higher priority than > >>> + * current, or current has *just* changed priority. If that's the case > >>> + * just reschedule current. > >>> + */ > >>> + if (unlikely(next_task->prio < rq->curr->prio)) { > >>> + resched_curr(rq); > >>> + return 0; > >>> + } > >> > >> IMHO, that's the bit which prevents the BUG. > >> > >> But this would also prevent the case in which rq->curr is an RT task > >> with lower prio than next_task. > >> > >> Also `rq->curr = migration/X` goes still though which is somehow fine > >> since find_lowest_rq() bails out for if (task->nr_cpus_allowed == 1). > >> > >> And DL tasks (like sugov:X go through and they can have > >> task->nr_cpus_allowed > 1 (arm64 slow-switching boards with shared > >> freuency domains with schedutil). cpupri_find_fitness()->convert_prio() > >> can handle task_pri, p->prio = -1 (CPUPRI_INVALID) although its somehow > >> by coincidence. > >> > >> So maybe something like this: > > > > Do you mean to replace just the one hunk from Valentin's patch with the > > change below (keeping the rest), or are you saying that only the change > > below is needed? > > The latter. Thanks! I tested the patch below and can confirm that I no longer see any BUGs with this applied. Tested-By: John Keeping --- a/kernel/sched/rt.c +++ b/kernel/sched/rt.c @@ -1898,6 +1898,11 @@ static int push_rt_task(struct rq *rq, bool pull) if (!pull || rq->push_busy) return 0; + if (rq->curr->sched_class != &rt_sched_class) { + resched_curr(rq); + return 0; + } + cpu = find_lowest_rq(rq->curr); if (cpu == -1 || cpu == rq->cpu) return 0;