linux-kernel - Re: [PATCH v4 00/19] sched: Migrate disable support

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:   Thu, 29 Oct 2020 19:03:17 +0000
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, ouwen210@...mail.com
Subject: Re: [PATCH v4 00/19] sched: Migrate disable support

On 23/10/20 11:11, Peter Zijlstra wrote:
> Hi,
>
> The fourth version of migrate_disable() for PREEMPT_RT.
>
> Two changes since last time:
>
>  - fixes !SMP builds (bigeasy)
>  - TLA+ validation of migrate_disable() vs sched_setaffinity() (valsch)
>
> Esp. that latter resulted in significant changes to patch #10. Huge thanks to
> Valentin.
>

I've been poking at that model some more; there's always going to be some
gap between what is being modelled vs what the code actually does, but
I'm mostly happy with the state it is in now.

So far it's been 1 victim task looping around a Migration-Disabled region
while some other threads fight over changing its affinity. The victim task
can also be preempted by some higher-priority task. I didn't make the
victim task block between Migration-Disabled regions.

Last run was 2 CPUs and 2 affinity-changing threads, I'll try to bump that
a bit but it is already somewhat lengthy to run (~10h).

In terms of what this has been checking
- Forward progress (no process forever blocked on some condition / lock)
- Refcounts always >= 0 (hit a few -1's in earlier versions)
- Affinity is respected
  I've "implemented" that as:

    If 'p' is running on a CPU, then that CPU must be in p->cpus_mask when
      !p->migration_pending && p->pi_lock is not held

I don't think I have much else to add, so feel free to add

  Reviewed-by: Valentin Schneider <valentin.schneider@....com>

To patches 01-14, 16-17.

The RT side of patch 15 looked fine to me, but I didn't spend much time on
DL. We might still want to steal those .pick_task() from coresched to
implement that XXX in push_cpu_stop(), but AIUI this is more of a
performance thing than a correctness one.