On Wed, 16 Sep 2020 at 21:32, Linus Torvalds torvalds@linux-foundation.org wrote:
But something like a driver list walking thing should not be doing different things behind peoples back depending on whether they hold spinlocks or not. It should either just work regardless, or there should be a flag (or special interface) for the "you're being called in a crtitical region".
Because dynamically changing behavior really is very confusing.
By the same reasoning, I don't think a generic crypto library should be playing tricks with preemption en/disabling under the hood when iterating over some data that is all directly accessible via the linear map on the platforms that most people care about. And using kmap_atomic() unconditionally achieves exactly that.
As I argued before, the fact that kmap_atomic() can be called from an atomic context, and the fact that its implementation on HIGHMEM platforms requires preemption to be disabled until the next kunmap() are two different things, and I don't agree with your assertion that the name kmap_atomic() implies the latter semantics. If we can avoid disabling preemption on HIGHMEM, as Thomas suggests, we surely don't need it on !HIGHMEM either, and given that kmap_atomic() is preferred today anyway, we can just merge the two implementations. Are there any existing debug features that could help us spot [ab]use of things like raw per-CPU data within kmap_atomic regions?
Re your point about deprecating HIGHMEM: some work is underway on ARM to implement a 3.75/3.75 GB kernel/user split on recent LPAE capable hardware (which shouldn't suffer from the performance issues that plagued the 4/4 split on i686), and so hopefully, there is a path forward for ARM that does not rely on HIGHMEM as it does today.