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

On Tue, Aug 25, 2026 at 12:58:37PM -0700, Sean Christopherson wrote:
> 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.

Very good, preemption disabling it is!  It is quite possible that the
PREEMPT_RT guys will need something else, but one thing at a time.

> > > > > 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.

Again, whew!!!  ;-)

							Thanx, Paul