Ira,
On Fri, Oct 09 2020 at 12:49, ira weiny wrote:
From: Ira Weiny ira.weiny@intel.com
To correctly support the semantics of kmap() with Kernel protection keys (PKS), kmap() may be required to set the protections on multiple processors (globally). Enabling PKS globally can be very expensive depending on the requested operation. Furthermore, enabling a domain globally reduces the protection afforded by PKS.
Most kmap() (Aprox 209 of 229) callers use the map within a single thread and have no need for the protection domain to be enabled globally. However, the remaining callers do not follow this pattern and, as best I can tell, expect the mapping to be 'global' and available to any thread who may access the mapping.[1]
We don't anticipate global mappings to pmem, however in general there is a danger in changing the semantics of kmap(). Effectively, this would cause an unresolved page fault with little to no information about why the failure occurred.
To resolve this a number of options were considered.
- Attempt to change all the thread local kmap() calls to kmap_atomic()[2]
- Introduce a flags parameter to kmap() to indicate if the mapping should be global or not
- Change ~20 call sites to 'kmap_global()' to indicate that they require a global enablement of the pages.
- Change ~209 call sites to 'kmap_thread()' to indicate that the mapping is to be used within that thread of execution only
Option 1 is simply not feasible. Option 2 would require all of the call sites of kmap() to change. Option 3 seems like a good minimal change but there is a danger that new code may miss the semantic change of kmap() and not get the behavior the developer intended. Therefore, #4 was chosen.
There is Option #5:
Convert the thread local kmap() invocations to the proposed kmap_local() interface which is coming along [1].
That solves a couple of issues:
1) It relieves the current kmap_atomic() usage sites from the implict pagefault/preempt disable semantics which apply even when CONFIG_HIGHMEM is disabled. kmap_local() still can be invoked from atomic context.
2) Due to #1 it allows to replace the conditional usage of kmap() and kmap_atomic() for purely thread local mappings.
3) It puts the burden on the HIGHMEM inflicted systems
4) It is actually more efficient for most of the pure thread local use cases on HIGHMEM inflicted systems because it avoids the overhead of the global lock and the potential kmap slot exhaustion. A potential preemption will be more expensive, but that's not really the case we want to optimize for.
5) It solves the RT issue vs. kmap_atomic()
So instead of creating yet another variety of kmap() which is just scratching the particular PKRS itch, can we please consolidate all of that on the wider reaching kmap_local() approach?
Thanks,
tglx
[1] https://lore.kernel.org/lkml/20201103092712.714480842@linutronix.de/