quoted

quoted

+ * currently enabled.
    */
   #ifdef CONFIG_ARCH_LAZY_MMU
   static inline void lazy_mmu_mode_enable(void)
   {
-    arch_enter_lazy_mmu_mode();
+    struct lazy_mmu_state *state = &current->lazy_mmu_state;
+
+    VM_BUG_ON(state->count == U8_MAX);

No VM_BUG_ON() please.

I did wonder if this would be acceptable!

Use VM_WARN_ON_ONCE() and let early testing find any such issues.

VM_* is active in debug kernels only either way! :)

If you'd want to handle this in production kernels you'd need

if (WARN_ON_ONCE()) {
	/* Try to recover */
}

And that seems unnecessary/overly-complicated for something that should 
never happen, and if it happens, can be found early during testing.

What should we do in case of underflow/overflow then? Saturate or just
let it wrap around? If an overflow occurs we're probably in some
infinite recursion and we'll crash anyway, but an underflow is likely
due to a double disable() and saturating would probably allow to recover.

quoted

quoted

+    /* enable() must not be called while paused */
+    VM_WARN_ON(state->count > 0 && !state->enabled);
+
+    if (state->count == 0) {
+        arch_enter_lazy_mmu_mode();
+        state->enabled = true;
+    }
+    ++state->count;

Can do

if (state->count++ == 0) {

My idea here was to have exactly the reverse order between enable() and
disable(), so that arch_enter() is called before lazy_mmu_state is
updated, and arch_leave() afterwards. arch_* probably shouldn't rely on
this (or care), but I liked the symmetry.

I see, but really the arch callback should never have to care about that
value -- unless something is messed up :)

[...]

quoted

quoted

+static inline bool in_lazy_mmu_mode(void)

So these functions will reveal the actual arch state, not whether
_enabled() was called.

As I can see in later patches, in interrupt context they are also
return "not in lazy mmu mode".

Yes - the idea is that a task is in lazy MMU mode if it enabled it and
is in process context. The mode is never enabled in interrupt context.
This has always been the intention, but it wasn't formalised until patch
12 (except on arm64).

Okay, thanks for clarifying.

-- 
Cheers

David / dhildenb