linux-kernel - Re: [PATCH] sched/rt: Fix live lock between select_fallback

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 28 Sep 2023 22:47:03 +0100
From:   Qais Yousef <qyousef@...alina.io>
To:     "Joel Fernandes (Google)" <joel@...lfernandes.org>
Cc:     linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Juri Lelli <juri.lelli@...hat.com>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Dietmar Eggemann <dietmar.eggemann@....com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Ben Segall <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>,
        Daniel Bristot de Oliveira <bristot@...hat.com>,
        Valentin Schneider <vschneid@...hat.com>,
        "Paul E . McKenney" <paulmck@...nel.org>, stable@...r.kernel.org,
        Xuewen Yan <xuewen.yan94@...il.com>
Subject: Re: [PATCH] sched/rt: Fix live lock between select_fallback_rq() and
 RT push

Hey Joel

On 09/23/23 01:14, Joel Fernandes (Google) wrote:
> During RCU-boost testing with the TREE03 rcutorture config, I found that
> after a few hours, the machine locks up.
> 
> On tracing, I found that there is a live lock happening between 2 CPUs.
> One CPU has an RT task running, while another CPU is being offlined
> which also has an RT task running.  During this offlining, all threads
> are migrated. The migration thread is repeatedly scheduled to migrate
> actively running tasks on the CPU being offlined. This results in a live
> lock because select_fallback_rq() keeps picking the CPU that an RT task
> is already running on only to get pushed back to the CPU being offlined.
> 
> It is anyway pointless to pick CPUs for pushing tasks to if they are
> being offlined only to get migrated away to somewhere else. This could
> also add unwanted latency to this task.
> 
> Fix these issues by not selecting CPUs in RT if they are not 'active'
> for scheduling, using the cpu_active_mask. Other parts in core.c already
> use cpu_active_mask to prevent tasks from being put on CPUs going
> offline.

I think this is the same report as this one from Xuewen

	https://lore.kernel.org/lkml/20221114120453.3233-1-xuewen.yan@unisoc.com/

You also have only 2 CPUs system, so when one CPU is offline, the other becomes
overloaded.

I think it is worth trying to fix rto_next_cpu() to consider the
cpu_active_mask too (see my reply to Xuewen then). Keep your change as well.

I think Xuewen and I considered disabling the overloaded logic when we devolve
to a UP system too as the whole logic doesn't make sense anymore then. This is
more of an optimization than correctness though. Don't feel strongly about it
now though.


Thanks!

--
Qais Yousef


> 
> Tested-by: Paul E. McKenney <paulmck@...nel.org>
> Cc: stable@...r.kernel.org
> Signed-off-by: Joel Fernandes (Google) <joel@...lfernandes.org>
> ---
>  kernel/sched/cpupri.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/kernel/sched/cpupri.c b/kernel/sched/cpupri.c
> index a286e726eb4b..42c40cfdf836 100644
> --- a/kernel/sched/cpupri.c
> +++ b/kernel/sched/cpupri.c
> @@ -101,6 +101,7 @@ static inline int __cpupri_find(struct cpupri *cp, struct task_struct *p,
>  
>  	if (lowest_mask) {
>  		cpumask_and(lowest_mask, &p->cpus_mask, vec->mask);
> +		cpumask_and(lowest_mask, lowest_mask, cpu_active_mask);
>  
>  		/*
>  		 * We have to ensure that we have at least one bit
> -- 
> 2.42.0.515.g380fc7ccd1-goog
>