linux-kernel - Re: [PATCH -v2 09/17] sched: Add migrate

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] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 06 Oct 2020 12:20:27 +0100
From:   Valentin Schneider <valentin.schneider@....com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     tglx@...utronix.de, mingo@...nel.org, linux-kernel@...r.kernel.org,
        bigeasy@...utronix.de, qais.yousef@....com, swood@...hat.com,
        juri.lelli@...hat.com, vincent.guittot@...aro.org,
        dietmar.eggemann@....com, rostedt@...dmis.org, bsegall@...gle.com,
        mgorman@...e.de, bristot@...hat.com, vincent.donnefort@....com,
        tj@...nel.org
Subject: Re: [PATCH -v2 09/17] sched: Add migrate_disable()


On 05/10/20 15:57, Peter Zijlstra wrote:
> Add the base migrate_disable() support (under protest).
>
> While migrate_disable() is (currently) required for PREEMPT_RT, it is
> also one of the biggest flaws in the system.
>
> Notably this is just the base implementation, it is broken vs
> sched_setaffinity() and hotplug, both solved in additional patches for
> ease of review.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> ---
[...]
> @@ -2363,7 +2451,7 @@ int select_task_rq(struct task_struct *p
>  {
>       lockdep_assert_held(&p->pi_lock);
>
> -	if (p->nr_cpus_allowed > 1)
> +	if (p->nr_cpus_allowed > 1 && !is_migration_disabled(p))

So dissociating migrate_disable() from p->nr_cpus_allowed unlocks the whole
DL+RT push/pull thingie, but it also means we need to be somewhat careful
regarding checks like the above.

Looking at current p->nr_cpus_allowed readers, it's mostly DL/RT; I was
somewhat afraid CFS load balance would use it but turns out it doesn't. The
only borderline case I see is call_on_cpu(), and even then it would be just a
performance deal rather than a correctness one.

All that to say: it seems fine to me.

>               cpu = p->sched_class->select_task_rq(p, cpu, sd_flags, wake_flags);
>       else
>               cpu = cpumask_any(p->cpus_ptr);