linux-kernel - Re: [PATCH 0/6] mm: Remote LRU per-cpu pagevec cache/per-cpu page list drain 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:   Tue, 21 Sep 2021 10:51:55 -0700
From:   Andrew Morton <akpm@...ux-foundation.org>
To:     Nicolas Saenz Julienne <nsaenzju@...hat.com>
Cc:     frederic@...nel.org, linux-kernel@...r.kernel.org,
        linux-mm@...ck.org, tglx@...utronix.de, cl@...ux.com,
        peterz@...radead.org, juri.lelli@...hat.com, mingo@...hat.com,
        mtosatti@...hat.com, nilal@...hat.com, mgorman@...e.de,
        ppandit@...hat.com, williams@...hat.com, bigeasy@...utronix.de,
        anna-maria@...utronix.de, linux-rt-users@...r.kernel.org
Subject: Re: [PATCH 0/6] mm: Remote LRU per-cpu pagevec cache/per-cpu page
 list drain support

On Tue, 21 Sep 2021 18:13:18 +0200 Nicolas Saenz Julienne <nsaenzju@...hat.com> wrote:

> This series introduces an alternative locking scheme around mm/swap.c's per-cpu
> LRU pagevec caches and mm/page_alloc.c's per-cpu page lists which will allow
> for remote CPUs to drain them. Currently, only a local CPU is permitted to
> change its per-cpu lists, and it's expected to do so, on-demand, whenever a
> process demands it (by means of queueing an drain task on the local CPU). Most
> systems will handle this promptly, but it'll cause problems for NOHZ_FULL CPUs
> that can't take any sort of interruption without breaking their functional
> guarantees (latency, bandwidth, etc...). Having a way for these processes to
> remotely drain the lists themselves will make co-existing with isolated CPUs
> possible, at the cost of more constraining locks.
> 
> Fortunately for non-NOHZ_FULL users, the alternative locking scheme and remote
> drain code are conditional to a static key which is disabled by default. This
> guarantees minimal functional or performance regressions. The feature will only
> be enabled if NOHZ_FULL's initialization process was successful.

That all looks pretty straightforward.  Obvious problems are:

- Very little test coverage for the spinlocked code paths.  Virtually
  all test setups will be using local_lock() and the code path you care
  about will go untested.

  I hope that whoever does test the spinlock version will be running
  full debug kernels, including lockdep.  Because adding a spinlock
  where the rest of the code expects local_lock might introduce
  problems.

  A fix for all of this would be to enable the spin_lock code paths
  to be tested more widely.  Perhaps you could add a boot-time kernel
  parameter (or, not as good, a Kconfig thing) which forces the use of
  the spinlock code even on non-NOHZ_FULL systems.

  Or perhaps this debug/testing mode _should_ be enabled by Kconfig,
  so kernel fuzzers sometimes turn it on.

  Please have a think about all of this?

- Maintainability.  Few other MM developers will think about this new
  spinlocked mode much, and they are unlikely to runtime test the
  spinlock mode.  Adding the force-spinlocks-mode-on knob will help
  with this.