Re: [PATCH v1 4/4] Revert "x86/xen: allow nesting of same lazy mode"

[David Hildenbrand <david@xxxxxxxxxx>]

On 03.03.25 13:33, Ryan Roberts wrote:
> On 03/03/2025 11:52, David Hildenbrand wrote:
> > On 02.03.25 15:55, Ryan Roberts wrote:
> > > Commit 49147beb0ccb ("x86/xen: allow nesting of same lazy mode") was
> > > added as a solution for a core-mm code change where
> > > arch_[enter|leave]_lazy_mmu_mode() started to be called in a nested
> > > manner; see commit bcc6cc832573 ("mm: add default definition of
> > > set_ptes()").
> > >
> > > However, now that we have fixed the API to avoid nesting, we no longer
> > > need this capability in the x86 implementation.
> > >
> > > Additionally, from code review, I don't believe the fix was ever robust
> > > in the case of preemption occurring while in the nested lazy mode. The
> > > implementation usually deals with preemption by calling
> > > arch_leave_lazy_mmu_mode() from xen_start_context_switch() for the
> > > outgoing task if we are in the lazy mmu mode. Then in
> > > xen_end_context_switch(), it restarts the lazy mode by calling
> > > arch_enter_lazy_mmu_mode() for an incoming task that was in the lazy
> > > mode when it was switched out. But arch_leave_lazy_mmu_mode() will only
> > > unwind a single level of nesting. If we are in the double nest, then
> > > it's not fully unwound and per-cpu variables are left in a bad state.
> > >
> > > So the correct solution is to remove the possibility of nesting from the
> > > higher level (which has now been done) and remove this x86-specific
> > > solution.
> > >
> > > Fixes: 49147beb0ccb ("x86/xen: allow nesting of same lazy mode")
> > Does this patch here deserve this tag? IIUC, it's rather a cleanup now that it
> > was properly fixed elsewhere.
> Now that nesting is not possible, yes it is just a cleanup. But when nesting was
> possible, as far as I can tell it was buggy, as per my description.
Right, I understood that part.

> So it's a
> bug bug that won't ever trigger once the other fixes are applied. Happy to
> remove the Fixes and then not include it for stable for v2. That's probably
> simplest.
I was just curious, because it sounded like the actual fix was the other 
patch. Whatever you think is best :)
--
Cheers,

David / dhildenb