On Tue, 21 Sep 2021 18:13:18 +0200 Nicolas Saenz Julienne <nsaenzju@xxxxxxxxxx> 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.
