Re: [PATCH] mm/mmu_notifier: Remove non_block_start/end() from notifier invocation

[Sean Christopherson <seanjc@google.com>]

On Tue, Aug 25, 2026, Paul E. McKenney wrote:
> On Tue, Aug 25, 2026 at 06:48:08PM +0100, David Woodhouse wrote:
> > On Tue, 2026-08-25 at 10:19 -0700, Paul E. McKenney wrote:
> > > On Tue, Aug 25, 2026 at 06:05:54PM +0100, David Woodhouse wrote:
> > > > On Tue, 2026-08-25 at 09:47 -0700, Paul E. McKenney wrote:
> > > > > 
> > > > > On the tail latencies...
> > > > > 
> > > > > The easiest way to reduce them is to require that preemption be disabled
> > > > > across srcu_read_lock_atomic()/srcu_read_unlock_atomic() regions and
> > > > > across all calls to synchronize_srcu_atomic().  Without that, the problem
> > > > > is that the scheduler does not know that the spinning is pointless,
> > > > > and we cannot use the blocking primitives that we could otherwise use
> > > > > to tell it what is going on.
> > > > > 
> > > > > So, is it feasible to simply require preemption be disabled as called
> > > > > out above?
> > > > 
> > > > I'd experimented with disabling it around the GP driver loop in
> > > > synchronize_srcu_atomic() as seen in
> > > > https://git.infradead.org/?p=users/dwmw2/linux.git;a=commitdiff;h=07165e79340e
> > > > and that didn't seem to change anything (which seems reasonable, as
> > > > it's the *waiters* that were descheduled, not the threads driving the
> > > > actual GP). So your suggestion that we do it around the whole function
> > > > certainly makes sense too. I'll test it.
> > > > 
> > > > I do wonder if we're really doing the right thing here by selfishly
> > > > blocking preemption because we want a specific tail latency to remain
> > > > low in a contended system. Maybe we should allow preemption and trust
> > > > that the right thing will happen?

In my experience, preempting MMU operations, especially mmu_notifier invalidations,
is rarely a good idea.  E.g. see commit d02c357e5bfa ("KVM: x86/mmu: Retry fault
before acquiring mmu_lock if mapping is changing"), which worked around an issue
where KVM would drop mmu_lock and yield in an mmu_notifier callback on preemptible
kernels.  We "fixed" the issue by avoiding mmu_lock contention, because it was the
easiest fix and benefited all setups, but the underlying problem that made us take
action was very specifically yielding mmu_lock on preemptible kernels.

This isn't exactly the same, but it sounds quite similar: being greedy and hogging
the CPU to complete an operation can actually be beneficial for overall throughput,
not just for the immediate operation's latency, by avoiding trash and overhead that
is incurred as a result of yielding or being preempted.

> > > > Maybe the p100 isn't the right benchmark to be chasing... I'm looking
> > > > at it because Sean expressed concerns about it, but it's not the only
> > > > consideration.
> > > 
> > > My concern is algorithmic, not benchmark optimization.
> > > 
> > > Suppose that there is only one CPU, or, alternatively, that one of the
> > > atomic SRCU readers is pinned to the same CPU occupied by the (higher
> > > priority) task running synchronize_srcu_atomic().  In this case, the
> > > call to synchronize_srcu_atomic() uselessly burns CPU time until its
> > > priority decays, real-time throttling kicks in, or in some configurations,
> > > maybe never.
> > 
> > I certainly have no problem with a blanket preempt_disable() around
> > both sides for algorithmic reasons. As long as we aren't *just* doing
> > it for the selfish reasons I described. 
> 
> Suppose I simply disable preemption in srcu_read_lock_atomic(),
> enable it in srcu_read_unlock_atomic(), and disable it internally to
> synchronize_srcu_atomic()?  It might be against all RCU tradition,
> but might also be easier to use.  ;-)
> 
> > > Requiring preemption be disabled across both the atomic SRCU readers
> > > and the synchronize_srcu_atomic() avoids this, at least when running on
> > > bare metal.  My (perhaps naive) hope is that guest OSes get some use
> > > out of those cpu_relax() calls.
> > 
> > Yeah, an overcommited guest vCPU should be able to get preempted there
> > by the hypervisor, allowing other vCPUs to run.
> 
> Whew!!!  ;-)

Ya, and on KVM x86 at least, cpu_relax() => PAUSE will conditionally trigger a
VM-Exit after enough spins that causes KVM-the-host to try to yield the vCPU to
another vCPU in the same VM.  The intended use case is to detect when a vCPU is
spinning waiting for a lock, to try and give cycles to the vCPU that is holding
said lock.  IIUC, the same principle should apply here.