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

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

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 4393 bytes --]

From: David Woodhouse <dwmw@amazon.co.uk>

This effectively reverts commit ba170f76b69d ("mm, notifier: Catch
sleeping/blocking for !blockable") for the mmu_notifier call sites.

The non_block_start/end() annotation causes the scheduler to complain
about *any* voluntary sleep in a non-blockable notifier. But that was
never the actual constraint. As Michal Hocko put it when the
annotation was first proposed (quoted in commit 312364f3534c
("kernel.h: Add non_block_start/end()")), the OOM reaper "shouldn't
depend on any locks or sleepable conditionals" and checking for
sleepable context was "the best thing we could come up with that would
describe these demands at least partially". The real requirement is that the reaper
must not block on anything which may itself depend on memory
allocation (or on the dying mm) to make progress — which is why
spinning locks were always considered fine.

That distinction now matters in both directions:

 - On PREEMPT_RT, spinning locks become sleeping locks, and perfectly
   legitimate spinlock/rwlock usage in notifier implementations (e.g.
   KVM's mn_invalidate_lock and gfn_to_pfn_cache locks) triggers the
   splat despite having no allocator dependency whatsoever. This is
   reproducible today on a PREEMPT_RT kernel: KVM takes
   kvm->mn_invalidate_lock in kvm_mmu_notifier_invalidate_range_start(),
   and if the OOM reaper reaps a KVM process the result is a "BUG:
   sleeping function called from invalid context" from
   rt_spin_lock().

 - A notifier implementation may legitimately need to wait for an RCU
   grace period before allowing the caller to proceed with unmapping
   (in the manner of a TLB shootdown, waiting for readers of a cached
   translation to drain). A grace period completes without any memory
   allocation and cannot deadlock against the reaper, but the
   annotation forbids it.

Checking for genuinely forbidden dependencies mechanically would
require tracking *what* is being waited on, which this annotation
never did. Remove it from the notifier invocation and leave the
constraint where it always really lived: in review and documentation
of the notifier implementations.

Fixes: ba170f76b69d ("mm, notifier: Catch sleeping/blocking for !blockable")
Closes: https://lore.kernel.org/all/787aa26cf62dfd361eea8ed19f384fc517892501.camel@infradead.org/
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Claude:claude-mythos-5
---
This is a prerequisite for converting KVM's gfn_to_pfn_cache to use
SRCU for its readers, where the invalidate_range_start() notifier
waits for an SRCU grace period before the caller zaps the page
tables — in the manner of a TLB shootdown. Discussion of that series
(and of the annotation problem) at
https://lore.kernel.org/all/787aa26cf62dfd361eea8ed19f384fc517892501.camel@infradead.org/

Maybe a "non_alloc_start() / non_alloc_end()" would be closer to what
we need, but even that doesn't actually protect against the case where
we *transitively* wait for allocations from the OOM path (qv).

 mm/mmu_notifier.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/mm/mmu_notifier.c b/mm/mmu_notifier.c
index 245b74f39f91..cd5d15cd646a 100644
--- a/mm/mmu_notifier.c
+++ b/mm/mmu_notifier.c
@@ -520,11 +520,7 @@ static int mn_hlist_invalidate_range_start(
 		if (ops->invalidate_range_start) {
 			int _ret;
 
-			if (!mmu_notifier_range_blockable(range))
-				non_block_start();
 			_ret = ops->invalidate_range_start(subscription, range);
-			if (!mmu_notifier_range_blockable(range))
-				non_block_end();
 			if (_ret) {
 				pr_info("%pS callback failed with %d in %sblockable context.\n",
 					ops->invalidate_range_start, _ret,
@@ -591,14 +587,9 @@ mn_hlist_invalidate_end(struct mmu_notifier_subscriptions *subscriptions,
 	id = srcu_read_lock(&srcu);
 	hlist_for_each_entry_srcu(subscription, &subscriptions->list, hlist,
 				 srcu_read_lock_held(&srcu)) {
-		if (subscription->ops->invalidate_range_end) {
-			if (!mmu_notifier_range_blockable(range))
-				non_block_start();
+		if (subscription->ops->invalidate_range_end)
 			subscription->ops->invalidate_range_end(subscription,
 								range);
-			if (!mmu_notifier_range_blockable(range))
-				non_block_end();
-		}
 	}
 	srcu_read_unlock(&srcu, id);
 }
-- 
2.43.0


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 6179 bytes --]

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

[Jason Gunthorpe]

On Tue, Aug 11, 2026 at 09:58:44AM +0100, David Woodhouse wrote:
> As Michal Hocko put it when the
> annotation was first proposed (quoted in commit 312364f3534c
> ("kernel.h: Add non_block_start/end()")), the OOM reaper "shouldn't
> depend on any locks or sleepable conditionals" and checking for
> sleepable context was "the best thing we could come up with that would
> describe these demands at least partially". 

Sure, but we translated this into the notifier must run in an atomic
context and everyone has been happy with this.

> That distinction now matters in both directions:
> 
>  - On PREEMPT_RT, spinning locks become sleeping locks, and perfectly
>    legitimate spinlock/rwlock usage in notifier implementations (e.g.
>    KVM's mn_invalidate_lock and gfn_to_pfn_cache locks) triggers the
>    splat despite having no allocator dependency whatsoever. This is
>    reproducible today on a PREEMPT_RT kernel: KVM takes
>    kvm->mn_invalidate_lock in kvm_mmu_notifier_invalidate_range_start(),
>    and if the OOM reaper reaps a KVM process the result is a "BUG:
>    sleeping function called from invalid context" from
>    rt_spin_lock().

I don't know anything about PREEEMPT_RT, but this seems like an issue
with RT if a traditionally atomic safe functions are now triggering
might sleep failures?

>  - A notifier implementation may legitimately need to wait for an RCU
>    grace period before allowing the caller to proceed with unmapping

That's not allowed. We really want to forbid that, it is not an
acceptable way to implement a driver using these APIs due to
performance.

Maybe change this to #ifdef it around PREEMPT_RT.

Jason

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

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 2540 bytes --]

On Tue, 2026-08-11 at 10:55 -0300, Jason Gunthorpe wrote:
> On Tue, Aug 11, 2026 at 09:58:44AM +0100, David Woodhouse wrote:
> > As Michal Hocko put it when the
> > annotation was first proposed (quoted in commit 312364f3534c
> > ("kernel.h: Add non_block_start/end()")), the OOM reaper "shouldn't
> > depend on any locks or sleepable conditionals" and checking for
> > sleepable context was "the best thing we could come up with that would
> > describe these demands at least partially". 
> 
> Sure, but we translated this into the notifier must run in an atomic
> context and everyone has been happy with this.
> 
> > That distinction now matters in both directions:
> > 
> >  - On PREEMPT_RT, spinning locks become sleeping locks, and perfectly
> >    legitimate spinlock/rwlock usage in notifier implementations (e.g.
> >    KVM's mn_invalidate_lock and gfn_to_pfn_cache locks) triggers the
> >    splat despite having no allocator dependency whatsoever. This is
> >    reproducible today on a PREEMPT_RT kernel: KVM takes
> >    kvm->mn_invalidate_lock in kvm_mmu_notifier_invalidate_range_start(),
> >    and if the OOM reaper reaps a KVM process the result is a "BUG:
> >    sleeping function called from invalid context" from
> >    rt_spin_lock().
> 
> I don't know anything about PREEEMPT_RT, but this seems like an issue
> with RT if a traditionally atomic safe functions are now triggering
> might sleep failures?

I can sympathise with that point of view. In fact I've spent the last
couple of years mostly ignoring this "problem" and just blaming RT for
doing exactly that, but I don't think we can really get away with it
any more.

cf. https://lore.kernel.org/all/787aa26cf62dfd361eea8ed19f384fc517892501.camel@infradead.org/

> >  - A notifier implementation may legitimately need to wait for an RCU
> >    grace period before allowing the caller to proceed with unmapping
> 
> That's not allowed. We really want to forbid that, it is not an
> acceptable way to implement a driver using these APIs due to
> performance.

Speak for yourself. For the KVM gfn-to-pfn-cache the performance scales
*much* better with RCU than with explicit locking:
https://lore.kernel.org/all/8f41cb82b7c99d5a3d1dda016e4841326b4d8a52.camel@infradead.org/

Perhaps we could find a way to push down an *accurate* sanity check
into the code paths where what you say is *true*? I guess it could be
done with a flag on each notifier? Or *into* the notifier callback
function(s)?

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 6179 bytes --]

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

[Jason Gunthorpe]

On Tue, Aug 11, 2026 at 03:21:35PM +0100, David Woodhouse wrote:
> > >  - On PREEMPT_RT, spinning locks become sleeping locks, and perfectly
> > >    legitimate spinlock/rwlock usage in notifier implementations (e.g.
> > >    KVM's mn_invalidate_lock and gfn_to_pfn_cache locks) triggers the
> > >    splat despite having no allocator dependency whatsoever. This is
> > >    reproducible today on a PREEMPT_RT kernel: KVM takes
> > >    kvm->mn_invalidate_lock in kvm_mmu_notifier_invalidate_range_start(),
> > >    and if the OOM reaper reaps a KVM process the result is a "BUG:
> > >    sleeping function called from invalid context" from
> > >    rt_spin_lock().
> > 
> > I don't know anything about PREEEMPT_RT, but this seems like an issue
> > with RT if a traditionally atomic safe functions are now triggering
> > might sleep failures?
> 
> I can sympathise with that point of view. In fact I've spent the last
> couple of years mostly ignoring this "problem" and just blaming RT for
> doing exactly that, but I don't think we can really get away with it
> any more.
> 
> cf. https://lore.kernel.org/all/787aa26cf62dfd361eea8ed19f384fc517892501.camel@infradead.org/

If might_sleep doesn't work sanely at all in preempt_rt then just
globally turn it off?

> > >  - A notifier implementation may legitimately need to wait for an RCU
> > >    grace period before allowing the caller to proceed with unmapping
> > 
> > That's not allowed. We really want to forbid that, it is not an
> > acceptable way to implement a driver using these APIs due to
> > performance.
> 
> Speak for yourself. For the KVM gfn-to-pfn-cache the performance scales
> *much* better with RCU than with explicit locking:
> https://lore.kernel.org/all/8f41cb82b7c99d5a3d1dda016e4841326b4d8a52.camel@infradead.org/

At the cost of completely destroying the mm shootdown performance with
1s RCU grace period waits every mm operation. No thanks.

The unstated secondary purprose of the atomic context is to force the
driver implementors to make sane choices that don't degrade the MM
spectacularly.

> Perhaps we could find a way to push down an *accurate* sanity check
> into the code paths where what you say is *true*? I guess it could be
> done with a flag on each notifier? Or *into* the notifier callback
> function(s)?

I think it is right and correct the way it is.

Jason

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

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 2597 bytes --]

(Correcting Sean's email address)

On Tue, 2026-08-11 at 11:27 -0300, Jason Gunthorpe wrote:
> On Tue, Aug 11, 2026 at 03:21:35PM +0100, David Woodhouse wrote:
> > > >  - On PREEMPT_RT, spinning locks become sleeping locks, and perfectly
> > > >    legitimate spinlock/rwlock usage in notifier implementations (e.g.
> > > >    KVM's mn_invalidate_lock and gfn_to_pfn_cache locks) triggers the
> > > >    splat despite having no allocator dependency whatsoever. This is
> > > >    reproducible today on a PREEMPT_RT kernel: KVM takes
> > > >    kvm->mn_invalidate_lock in kvm_mmu_notifier_invalidate_range_start(),
> > > >    and if the OOM reaper reaps a KVM process the result is a "BUG:
> > > >    sleeping function called from invalid context" from
> > > >    rt_spin_lock().
> > > 
> > > I don't know anything about PREEEMPT_RT, but this seems like an issue
> > > with RT if a traditionally atomic safe functions are now triggering
> > > might sleep failures?
> > 
> > I can sympathise with that point of view. In fact I've spent the last
> > couple of years mostly ignoring this "problem" and just blaming RT for
> > doing exactly that, but I don't think we can really get away with it
> > any more.
> > 
> > cf. https://lore.kernel.org/all/787aa26cf62dfd361eea8ed19f384fc517892501.camel@infradead.org/
> 
> If might_sleep doesn't work sanely at all in preempt_rt then just
> globally turn it off?

Turn might_sleep off? Or PREEMPT_RT? :)

The RT maintainers are on this thread if you want to pick either of
those fights... that was not the course of action I chose to take.

> > > >  - A notifier implementation may legitimately need to wait for an RCU
> > > >    grace period before allowing the caller to proceed with unmapping
> > > 
> > > That's not allowed. We really want to forbid that, it is not an
> > > acceptable way to implement a driver using these APIs due to
> > > performance.
> > 
> > Speak for yourself. For the KVM gfn-to-pfn-cache the performance scales
> > *much* better with RCU than with explicit locking:
> > https://lore.kernel.org/all/8f41cb82b7c99d5a3d1dda016e4841326b4d8a52.camel@infradead.org/
> 
> At the cost of completely destroying the mm shootdown performance with
> 1s RCU grace period waits every mm operation. No thanks.

I feel like we're not talking about the same things here.

The KVM patch which this enables does *not* behave as you have
described. Have you looked at it?

Nobody's suggesting that we force any *other* MMU notifiers to do
anything that they don't do today.

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 6179 bytes --]

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

[Steven Rostedt]

On Tue, 11 Aug 2026 15:33:18 +0100
David Woodhouse <dwmw2@infradead.org> wrote:

> > If might_sleep doesn't work sanely at all in preempt_rt then just
> > globally turn it off?  
> 
> Turn might_sleep off? Or PREEMPT_RT? :)
> 
> The RT maintainers are on this thread if you want to pick either of
> those fights... that was not the course of action I chose to take.

I guess the question is, what exactly is the reason for sleeping to be
prohibited? In RT, sleeping is allowed in most context because most context
are threads (like interrupt handlers and such). Now, you still can't sleep
in NMIs and hard interrupt handlers that were not converted to threads, but
I'm not sure that's the case here anyway.

If the non_block_start() is just a big hammer to make sure things are fine
in non-RT, it will likely still be fine in RT even though it may block and
sleep. But what it blocks on are sleeping spin locks that likely would not
cause an issue here if they didn't cause an issue in non-RT.

Thus, perhaps something like this:

 		if (ops->invalidate_range_start) {
 			int _ret;
 
			if (!IS_ENABLED(CONFIG_PREEMPT_RT) && !mmu_notifier_range_blockable(range))
				non_block_start();
 			_ret = ops->invalidate_range_start(subscription, range);
			if (!IS_ENABLED(CONFIG_PREEMPT_RT) && !mmu_notifier_range_blockable(range))
				non_block_end();

?

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

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 2815 bytes --]

On Tue, 2026-08-11 at 10:42 -0400, Steven Rostedt wrote:
> On Tue, 11 Aug 2026 15:33:18 +0100
> David Woodhouse <dwmw2@infradead.org> wrote:
> 
> > > If might_sleep doesn't work sanely at all in preempt_rt then just
> > > globally turn it off?  
> > 
> > Turn might_sleep off? Or PREEMPT_RT? :)
> > 
> > The RT maintainers are on this thread if you want to pick either of
> > those fights... that was not the course of action I chose to take.
> 
> I guess the question is, what exactly is the reason for sleeping to be
> prohibited? In RT, sleeping is allowed in most context because most context
> are threads (like interrupt handlers and such). Now, you still can't sleep
> in NMIs and hard interrupt handlers that were not converted to threads, but
> I'm not sure that's the case here anyway.
> 
> If the non_block_start() is just a big hammer to make sure things are fine
> in non-RT, it will likely still be fine in RT even though it may block and
> sleep. But what it blocks on are sleeping spin locks that likely would not
> cause an issue here if they didn't cause an issue in non-RT.

That's exactly the case in
https://lore.kernel.org/all/787aa26cf62dfd361eea8ed19f384fc517892501.camel@infradead.org/
which is currently being complained about, but it's actually harmless
and arguably a false positive.

I had a second reason for disabling the overzealous check too: to allow
SRCU grace periods within the notifier callbacks.

> Thus, perhaps something like this:
> 
>  		if (ops->invalidate_range_start) {
>  			int _ret;
>  
> 			if (!IS_ENABLED(CONFIG_PREEMPT_RT) && !mmu_notifier_range_blockable(range))
> 				non_block_start();
>  			_ret = ops->invalidate_range_start(subscription, range);
> 			if (!IS_ENABLED(CONFIG_PREEMPT_RT) && !mmu_notifier_range_blockable(range))
> 				non_block_end();
> 
> ?

Or put it in a per-mmu-notifier-ops flag (a bit like the one in commit
5ff7091f5a2c, but with almost opposite semantics), and let the drivers
Jason cares about still keep the guard, while KVM doesn't need to. With
or without the RT part...

  static bool mn_enforce_non_block(const struct mmu_notifier_ops *ops,
                                 const struct mmu_notifier_range *range)
  {
        /* 
         * On PREEMPT_RT even a plain spin_lock() schedules, so the
         * annotation splats on legitimate non-blocking implementations.
         */
        if (IS_ENABLED(CONFIG_PREEMPT_RT))
                return false;
        if (ops->flags & MMU_NOTIFIER_NONBLOCKABLE_MAY_WAIT)
                return false;
        return !mmu_notifier_range_blockable(range);
  } 

But honestly, I can't see the point in keeping it around at all, given
that I literally had to hack the kernel to make it trigger in the first
place.

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 6179 bytes --]

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

[Jason Gunthorpe]

On Tue, Aug 11, 2026 at 04:24:48PM +0100, David Woodhouse wrote:

> I had a second reason for disabling the overzealous check too: to allow
> SRCU grace periods within the notifier callbacks.

Don't agree with allowing this at all.
 
> > Thus, perhaps something like this:
> > 
> >  		if (ops->invalidate_range_start) {
> >  			int _ret;
> >  
> > 			if (!IS_ENABLED(CONFIG_PREEMPT_RT) && !mmu_notifier_range_blockable(range))
> > 				non_block_start();
> >  			_ret = ops->invalidate_range_start(subscription, range);
> > 			if (!IS_ENABLED(CONFIG_PREEMPT_RT) && !mmu_notifier_range_blockable(range))
> > 				non_block_end();
> > 
> > ?

This seems fine to me

> Or put it in a per-mmu-notifier-ops flag (a bit like the one in commit
> 5ff7091f5a2c, but with almost opposite semantics), and let the drivers
> Jason cares about still keep the guard, while KVM doesn't need to. With
> or without the RT part...

Nope, common API semantics please.

Jason

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

[Jason Gunthorpe]

On Tue, Aug 11, 2026 at 03:33:18PM +0100, David Woodhouse wrote:
> > At the cost of completely destroying the mm shootdown performance with
> > 1s RCU grace period waits every mm operation. No thanks.
> 
> I feel like we're not talking about the same things here.
> 
> The KVM patch which this enables does *not* behave as you have
> described. Have you looked at it?

It does, it calls synchronize_rcu() from
kvm_mmu_notifier_invalidate_range_start() - don't do that...

> Nobody's suggesting that we force any *other* MMU notifiers to do
> anything that they don't do today.

Sure, but nobody should implement their own notifiers with such a
heavy locking primitive.

Jason

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

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 504 bytes --]

On Tue, 2026-08-11 at 11:27 -0300, Jason Gunthorpe wrote:
> 
> The unstated secondary purprose of the atomic context is to force the
> driver implementors to make sane choices that don't degrade the MM
> spectacularly.

You realise it only does this is in the OOM handler context when the
normal death of the process takes too long, and I literally had to hack
the kernel to even get the splat to trigger in the first place, right?

I don't think it's keeping your driver authors honest... :)

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 6179 bytes --]

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

[Jason Gunthorpe]

On Tue, Aug 11, 2026 at 04:15:36PM +0100, David Woodhouse wrote:
> On Tue, 2026-08-11 at 11:27 -0300, Jason Gunthorpe wrote:
> > 
> > The unstated secondary purprose of the atomic context is to force the
> > driver implementors to make sane choices that don't degrade the MM
> > spectacularly.
> 
> You realise it only does this is in the OOM handler context when the
> normal death of the process takes too long, and I literally had to hack
> the kernel to even get the splat to trigger in the first place, right?
> 
> I don't think it's keeping your driver authors honest... :)

It is documented to be like this, even if it is hard to test..

Even for normal blocking notifiers you should not be using
synchronize_rcu().

Jason

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

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 748 bytes --]

On Tue, 2026-08-11 at 12:24 -0300, Jason Gunthorpe wrote:
> It is documented to be like this, even if it is hard to test..

So don't change your documentation :)

> Even for normal blocking notifiers you should not be using
> synchronize_rcu().

This is SRCU not RCU, and the read-side sections are converted from
rwlocks and never had any allocations inside them anyway. But under RT,
rwlocks can spin too, which causes all kinds of fun and that's what
pushed us to look at SRCU... which actually performs a whole lot better
at scale too.

But that's a separate thread. If you have input on how KVM should and
should not be dealing with that issue, it would be welcome in the
context of the specific patches which are doing so...

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 6179 bytes --]

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

[Jason Gunthorpe]

On Tue, Aug 11, 2026 at 04:29:55PM +0100, David Woodhouse wrote:
> On Tue, 2026-08-11 at 12:24 -0300, Jason Gunthorpe wrote:
> > It is documented to be like this, even if it is hard to test..
> 
> So don't change your documentation :)
> 
> > Even for normal blocking notifiers you should not be using
> > synchronize_rcu().
> 
> This is SRCU not RCU, and the read-side sections are converted from
> rwlocks and never had any allocations inside them anyway.

To be clear you should not be using any synchronize_[s]rcu() primitive
inside the invalidation callbacks. These are well known to have
multi-second delays on loaded systems which are a completely
inappropriate performance characteristic for these mm callbacks.

This statement has nothing to do with deadlock.

RCU is always a trade off, you can make the read side run really fast
and the write side is ghastly slow. If you can't handle the slow write
you shouldn't use RCU techniques.

Jason

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

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 1664 bytes --]

On Tue, 2026-08-11 at 13:24 -0300, Jason Gunthorpe wrote:
> To be clear you should not be using any synchronize_[s]rcu() primitive
> inside the invalidation callbacks. These are well known to have
> multi-second delays on loaded systems which are a completely
> inappropriate performance characteristic for these mm callbacks.
> 
> This statement has nothing to do with deadlock.
> 
> RCU is always a trade off, you can make the read side run really fast
> and the write side is ghastly slow. If you can't handle the slow write
> you shouldn't use RCU techniques.

The multi-second horror stories are about the *global* RCU/SRCU
domains, where the grace period has to wait out arbitrary readers all
over the kernel.

This is not that. It is a dedicated srcu_struct, private to one VM,
and its entire reader population is a handful of KVM fast paths that
until now were under irqsave rwlocks.

Each of those read-side sections is a few hundred instructions over a
single page of guest memory — they never allocate, never take a lock,
never sleep. The number of readers in existence at any instant is
bounded by the number of vCPUs plus a few interrupt contexts, and the
grace period is bounded by the longest of those sections.

I should actually change to synchronize_srcu_expedited() — and perhaps
we could ponder an even more expedited mode which does the first check
directly and in the common case doesn't even *use* the workqueue — but
even without doing that, the write side is never going to be as
"ghastly slow" as you seem to think, *even* for the tiny handful of
virtual addresses for which it even kicks in at all.

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 6179 bytes --]

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

[Jason Gunthorpe]

On Tue, Aug 11, 2026 at 06:22:12PM +0100, David Woodhouse wrote:
> On Tue, 2026-08-11 at 13:24 -0300, Jason Gunthorpe wrote:
> > To be clear you should not be using any synchronize_[s]rcu() primitive
> > inside the invalidation callbacks. These are well known to have
> > multi-second delays on loaded systems which are a completely
> > inappropriate performance characteristic for these mm callbacks.
> > 
> > This statement has nothing to do with deadlock.
> > 
> > RCU is always a trade off, you can make the read side run really fast
> > and the write side is ghastly slow. If you can't handle the slow write
> > you shouldn't use RCU techniques.
> 
> The multi-second horror stories are about the *global* RCU/SRCU
> domains, where the grace period has to wait out arbitrary readers all
> over the kernel.
> 
> This is not that. It is a dedicated srcu_struct, private to one VM,
> and its entire reader population is a handful of KVM fast paths that
> until now were under irqsave rwlocks.

Are you sure? I've never heard that srcu has those kinds of properties.

If its so fast you should just propose a non-sleeping version and
leave the notifiers out of it

Jason

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

[David Woodhouse]

On 11 August 2026 18:26:27 BST, Jason Gunthorpe <jgg@ziepe.ca> wrote:
>On Tue, Aug 11, 2026 at 06:22:12PM +0100, David Woodhouse wrote:
>> On Tue, 2026-08-11 at 13:24 -0300, Jason Gunthorpe wrote:
>> > To be clear you should not be using any synchronize_[s]rcu() primitive
>> > inside the invalidation callbacks. These are well known to have
>> > multi-second delays on loaded systems which are a completely
>> > inappropriate performance characteristic for these mm callbacks.
>> > 
>> > This statement has nothing to do with deadlock.
>> > 
>> > RCU is always a trade off, you can make the read side run really fast
>> > and the write side is ghastly slow. If you can't handle the slow write
>> > you shouldn't use RCU techniques.
>> 
>> The multi-second horror stories are about the *global* RCU/SRCU
>> domains, where the grace period has to wait out arbitrary readers all
>> over the kernel.
>> 
>> This is not that. It is a dedicated srcu_struct, private to one VM,
>> and its entire reader population is a handful of KVM fast paths that
>> until now were under irqsave rwlocks.
>
>Are you sure? I've never heard that srcu has those kinds of properties.
>
>If its so fast you should just propose a non-sleeping version and
>leave the notifiers out of it
>
>Jason

I've got torture tests running for correctness on the GPC RCU conversion. I'll throw in some metrics on how often even in that pathological case we hit the wait case, and how long it actually takes.

And I can prototype the extra-expedited case that never even falls back to the WQ, but frankly I don't know if it's even worth it.

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

[Jason Gunthorpe]

On Tue, Aug 11, 2026 at 06:59:24PM +0100, David Woodhouse wrote:
> On 11 August 2026 18:26:27 BST, Jason Gunthorpe <jgg@ziepe.ca> wrote:
> >On Tue, Aug 11, 2026 at 06:22:12PM +0100, David Woodhouse wrote:
> >> On Tue, 2026-08-11 at 13:24 -0300, Jason Gunthorpe wrote:
> >> > To be clear you should not be using any synchronize_[s]rcu() primitive
> >> > inside the invalidation callbacks. These are well known to have
> >> > multi-second delays on loaded systems which are a completely
> >> > inappropriate performance characteristic for these mm callbacks.
> >> > 
> >> > This statement has nothing to do with deadlock.
> >> > 
> >> > RCU is always a trade off, you can make the read side run really fast
> >> > and the write side is ghastly slow. If you can't handle the slow write
> >> > you shouldn't use RCU techniques.
> >> 
> >> The multi-second horror stories are about the *global* RCU/SRCU
> >> domains, where the grace period has to wait out arbitrary readers all
> >> over the kernel.
> >> 
> >> This is not that. It is a dedicated srcu_struct, private to one VM,
> >> and its entire reader population is a handful of KVM fast paths that
> >> until now were under irqsave rwlocks.
> >
> >Are you sure? I've never heard that srcu has those kinds of properties.
> >
> >If its so fast you should just propose a non-sleeping version and
> >leave the notifiers out of it
>
> I've got torture tests running for correctness on the GPC RCU
> conversion. I'll throw in some metrics on how often even in that
> pathological case we hit the wait case, and how long it actually
> takes.

Well, to hit the bad RCU cases you need to usually do some other
workload too..

I guess srcu does have some meaningful functional differences, but it
is hardly guaranteed to be fast or non-sleeping out of the box.

I guess you are making an arugment that if SRCU critical sections are
atomic themselves then the synchronize could also reasonably be
atomic. That seems plausible, and may be worth some additional API
surface on the SRCU side to expose this use model and drop the might
sleep that is causing the trouble.

Some sort of "atomic RCU" that has a slower reader but a faster atomic
writer.

I'm much happier to see a formal API under the notifiers that has
strong properties of being reasonable than KVM using SRCU in a way
that just happens to do that by accident, under the current
implementation..

Jason

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

[Sean Christopherson]

On Tue, Aug 11, 2026, Jason Gunthorpe wrote:
> On Tue, Aug 11, 2026 at 06:59:24PM +0100, David Woodhouse wrote:
> > On 11 August 2026 18:26:27 BST, Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > >On Tue, Aug 11, 2026 at 06:22:12PM +0100, David Woodhouse wrote:
> > >> On Tue, 2026-08-11 at 13:24 -0300, Jason Gunthorpe wrote:
> > >> > To be clear you should not be using any synchronize_[s]rcu() primitive
> > >> > inside the invalidation callbacks. These are well known to have
> > >> > multi-second delays on loaded systems which are a completely
> > >> > inappropriate performance characteristic for these mm callbacks.
> > >> > 
> > >> > This statement has nothing to do with deadlock.
> > >> > 
> > >> > RCU is always a trade off, you can make the read side run really fast
> > >> > and the write side is ghastly slow. If you can't handle the slow write
> > >> > you shouldn't use RCU techniques.
> > >> 
> > >> The multi-second horror stories are about the *global* RCU/SRCU
> > >> domains, where the grace period has to wait out arbitrary readers all
> > >> over the kernel.
> > >> 
> > >> This is not that. It is a dedicated srcu_struct, private to one VM,
> > >> and its entire reader population is a handful of KVM fast paths that
> > >> until now were under irqsave rwlocks.
> > >
> > >Are you sure? I've never heard that srcu has those kinds of properties.
> > >
> > >If its so fast you should just propose a non-sleeping version and
> > >leave the notifiers out of it
> >
> > I've got torture tests running for correctness on the GPC RCU
> > conversion. I'll throw in some metrics on how often even in that
> > pathological case we hit the wait case, and how long it actually
> > takes.
> 
> Well, to hit the bad RCU cases you need to usually do some other
> workload too..

Yeah, and we've had several (recent) examples of SRCU tail latencies causing
problems for KVM.

> I guess srcu does have some meaningful functional differences, but it
> is hardly guaranteed to be fast or non-sleeping out of the box.
> 
> I guess you are making an arugment that if SRCU critical sections are
> atomic themselves then the synchronize could also reasonably be
> atomic. That seems plausible, and may be worth some additional API
> surface on the SRCU side to expose this use model and drop the might
> sleep that is causing the trouble.
> 
> Some sort of "atomic RCU" that has a slower reader but a faster atomic
> writer.
> 
> I'm much happier to see a formal API under the notifiers that has
> strong properties of being reasonable than KVM using SRCU in a way
> that just happens to do that by accident, under the current
> implementation..

Agreed, I suspect shoving a synchronize_*rcu() of any kind in the mmu_notifier
invalidation path will come back to bite us, hard.

But I don't think we need an entirely new type of RCU for KVM.  Unlike (S)RCU,
KVM can and _must_ block relevant readers when an invalidation is in-flight.
I.e. the invalidation path doesn't need to ensure *all* readers go away, only
that the relevant readers have observed the invalidation.

The readers also don't need to be allowed to sleep; I suggested using SRCU instead
of RCU purely because the tail latencies for regular RCU are typically much, much
worse than SRCU (and I agree that they're bad for SRCU).

Earlier, David described KVM's GPCs as de facto software TLBs, and KVM already
has code to protect walks of what are effectively software TLBs, specifically
walk_shadow_page_lockless_{begin,end}() and the associated write-side handling
of READING_SHADOW_PAGE_TABLES in kvm_request_needs_ipi().

And looking to the future, if/when we use GPCs to track PFNs that are mapped into
the guest through control structures, i.e. not through page tables, we'll already
need to rely on kicking CPUs via IPI to ensure readers see the invalidation.

So rather than use (S)RCU, what if KVM tracks which CPUs are reading and then
blasts IPIs to complete the "TLB" shootdown?

The biggest wrinkle I can think of is that unlike READING_SHADOW_PAGE_TABLES,
there isn't a 1:1 association between vCPUs and CPUs, i.e. KVM can't walk its
array of vCPUs to see which CPUs need to be kicked.  But that should be easy enough
to solve with a cpumask.  Cache line contention might be a problem, but if so, it
seems like a solvable problem.

Very roughly and incomplete, relative to David's series to use SRCU:

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ac961f4c91da..b4a7b613ad91 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1719,18 +1719,18 @@ static void kvm_setup_guest_pvclock(struct pvclock_vcpu_time_info *ref_hv_clock,
 {
 	struct pvclock_vcpu_time_info *guest_hv_clock;
 	struct pvclock_vcpu_time_info hv_clock;
-	int idx;
+	unsigned long flags;
 
 	memcpy(&hv_clock, ref_hv_clock, sizeof(hv_clock));
 
-	idx = srcu_read_lock(&vcpu->kvm->gpc_srcu);
+	flags = kvm_gpc_read_begin(vcpu->kvm);
 	while (!kvm_gpc_check(gpc, offset + sizeof(*guest_hv_clock))) {
-		srcu_read_unlock(&vcpu->kvm->gpc_srcu, idx);
+		kvm_gpc_read_end(vcpu->kvm, flags);
 
 		if (kvm_gpc_refresh(gpc, offset + sizeof(*guest_hv_clock)))
 			return;
 
-		idx = srcu_read_lock(&vcpu->kvm->gpc_srcu);
+		flags = kvm_gpc_read_begin(vcpu->kvm);
 	}
 
 	guest_hv_clock = (void *)(gpc->khva + offset);
@@ -1755,7 +1755,7 @@ static void kvm_setup_guest_pvclock(struct pvclock_vcpu_time_info *ref_hv_clock,
 	guest_hv_clock->version = ++hv_clock.version;
 
 	kvm_gpc_mark_dirty_in_slot(gpc);
-	srcu_read_unlock(&vcpu->kvm->gpc_srcu, idx);
+	kvm_gpc_read_end(vcpu->kvm, flags);
 
 	trace_kvm_pvclock_update(vcpu->vcpu_id, &hv_clock);
 }
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 7b2dbbd6b104..54ec1082c5ec 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -189,6 +189,8 @@ bool kvm_make_vcpus_request_mask(struct kvm *kvm, unsigned int req,
 				 unsigned long *vcpu_bitmap);
 bool kvm_make_all_cpus_request(struct kvm *kvm, unsigned int req);
 
+void kvm_kick_many_cpus(cpumask_var_t __cpus, bool wait);
+
 #define KVM_USERSPACE_IRQ_SOURCE_ID		0
 #define KVM_IRQFD_RESAMPLE_IRQ_SOURCE_ID	1
 #define KVM_PIT_IRQ_SOURCE_ID			2
@@ -814,7 +816,7 @@ struct kvm {
 	 * A dedicated domain (rather than kvm->srcu) keeps those waits from
 	 * being lengthened by unrelated memslot readers.
 	 */
-	struct srcu_struct gpc_srcu;
+	cpumask_var_t gpc_readers;
 
 	/*
 	 * created_vcpus is protected by kvm->lock, and is incremented
@@ -1569,6 +1571,20 @@ static inline bool kvm_gpc_is_hva_active(struct gfn_to_pfn_cache *gpc)
 	return gpc->active && kvm_is_error_gpa(gpc->gpa);
 }
 
+static inline unsigned long kvm_gpc_read_begin(struct kvm *kvm)
+{
+	unsigned long flags;
+
+	local_irq_save(flags);
+	cpumask_set_cpu(smp_processor_id(), kvm->gpc_readers);
+}
+
+static inline void kvm_gpc_read_end(struct kvm *kvm, unsigned long flags)
+{
+	cpumask_clear_cpu(smp_processor_id(), kvm->gpc_readers);
+	local_irq_restore(flags);
+}
+
 void kvm_sigset_activate(struct kvm_vcpu *vcpu);
 void kvm_sigset_deactivate(struct kvm_vcpu *vcpu);
 
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index c6e1c9c28b7e..9ef14057e477 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -205,7 +205,7 @@ static void ack_kick(void *_completed)
 {
 }
 
-static inline bool kvm_kick_many_cpus(struct cpumask *cpus, bool wait)
+static inline bool __kvm_kick_many_cpus(struct cpumask *cpus, bool wait)
 {
 	if (cpumask_empty(cpus))
 		return false;
@@ -214,6 +214,18 @@ static inline bool kvm_kick_many_cpus(struct cpumask *cpus, bool wait)
 	return true;
 }
 
+void kvm_kick_many_cpus(cpumask_var_t __cpus, bool wait)
+{
+	struct cpumask *cpus;
+
+	guard(preempt)();
+
+	cpus = this_cpu_cpumask_var_ptr(cpu_kick_mask);
+	cpumask_copy(cpus, __cpus);
+
+	__kvm_kick_many_cpus(cpus, wait);
+}
+
 static void kvm_make_vcpu_request(struct kvm_vcpu *vcpu, unsigned int req,
 				  struct cpumask *tmp, int current_cpu)
 {
@@ -262,7 +274,7 @@ bool kvm_make_vcpus_request_mask(struct kvm *kvm, unsigned int req,
 		kvm_make_vcpu_request(vcpu, req, cpus, me);
 	}
 
-	called = kvm_kick_many_cpus(cpus, !!(req & KVM_REQUEST_WAIT));
+	called = __kvm_kick_many_cpus(cpus, !!(req & KVM_REQUEST_WAIT));
 	put_cpu();
 
 	return called;
@@ -284,7 +296,7 @@ bool kvm_make_all_cpus_request(struct kvm *kvm, unsigned int req)
 	kvm_for_each_vcpu(i, vcpu, kvm)
 		kvm_make_vcpu_request(vcpu, req, cpus, me);
 
-	called = kvm_kick_many_cpus(cpus, !!(req & KVM_REQUEST_WAIT));
+	called = __kvm_kick_many_cpus(cpus, !!(req & KVM_REQUEST_WAIT));
 	put_cpu();
 
 	return called;
diff --git a/virt/kvm/pfncache.c b/virt/kvm/pfncache.c
index 97958af667fb..305706ba35dd 100644
--- a/virt/kvm/pfncache.c
+++ b/virt/kvm/pfncache.c
@@ -121,7 +121,7 @@ void gfn_to_pfn_cache_invalidate_start(struct kvm *kvm, unsigned long start,
 	 * "size at init" flag, or GFP_NOWAIT in the upgrade).
 	 */
 	if (cleared)
-		synchronize_srcu(&kvm->gpc_srcu);
+		kvm_kick_many_cpus(kvm->gpc_readers, true);
 
 	/*
 	 * Note the GPC_INVALIDATING markers set above are deliberately NOT

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

[Paolo Bonzini]

If you want to avoid global contention on reads, you can trade it with
for_each_present_cpu().

On Tue, Aug 11, 2026 at 10:06 PM Sean Christopherson <seanjc@google.com> wrote:
> +static inline unsigned long kvm_gpc_read_begin(struct kvm *kvm)
> +{
> +       unsigned long flags;
> +
> +       local_irq_save(flags);
> +       cpumask_set_cpu(smp_processor_id(), kvm->gpc_readers);

per_cpu(kvm_gpc_reader, smp_processor_id()) = gpc;

> +}
> +
> +static inline void kvm_gpc_read_end(struct kvm *kvm, unsigned long flags)
> +{
> +       cpumask_clear_cpu(smp_processor_id(), kvm->gpc_readers);

per_cpu(kvm_gpc_reader, smp_processor_id()) = NULL;

> +       local_irq_restore(flags);
> +}

and then:

for_each_present_cpu(cpu)
    if (per_cpu(kvm_gpc_reader, cpu) == gpc)
        cpumask_set_cpu(cpu, gpc_readers);
__kvm_kick_many_cpus(cpus, wait);

Alternatively, there's always the poor-man RCU using an rwlock_t; you
do read_lock/read_unlock as usual for begin/end of reads, while the
write side does

   write_lock(&kvm->gpc_readers);
   write_unlock(&kvm->gpc_readers);

in place of synchronize_src().

Paolo

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

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 1690 bytes --]

On Tue, 2026-08-11 at 13:06 -0700, Sean Christopherson wrote:
> 
> +static inline unsigned long kvm_gpc_read_begin(struct kvm *kvm)
> +{
> +	unsigned long flags;
> +
> +	local_irq_save(flags);
> +	cpumask_set_cpu(smp_processor_id(), kvm->gpc_readers);
> +}
> +
> +static inline void kvm_gpc_read_end(struct kvm *kvm, unsigned long flags)
> +{
> +	cpumask_clear_cpu(smp_processor_id(), kvm->gpc_readers);
> +	local_irq_restore(flags);
> +}
> +

Nah, those are all on the same cache line. SRCU doesn't do that, does it?
You end up basically reinventing SRCU.

I'm OK with having a special helper for kvm_gpc_read_begin/end. I was
actually tempted to do that as patch 1 in the series which converted to
RCU — it's then an implementation detail whether it's rwlock, spinlock,
rwlock-except-on-RT-which-gets-raw-spinlock, SRCU or whatever.

If we use the helper and still make it use SRCU, we can still impose
some of the invariants which make it actually *OK* to use SRCU because
we know we'll never hit those worst-case latencies that Jason is
worried about — like adding non_block_start()/non_block_end() in the
helpers to ensure that we keep the atomic semantics within the locks,
that rwlocks always previously enforced, etc.

It isn't *strictly* non_block_start/end because because we invoke it in
hardirq context, of course. We'd have to work that detail out. Maybe it
*is* local_irq_save(); not because it's strictly necessary to do so but
just because it comes with the desired sanity-checks.

But either way, the point is that whether you reinvent SRCU or not, it
all works purely because we know about the behaviour of the read-side
users.

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 6179 bytes --]

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

[David Hildenbrand (Arm)]

On 8/11/26 10:58, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
> 
> This effectively reverts commit ba170f76b69d ("mm, notifier: Catch
> sleeping/blocking for !blockable") for the mmu_notifier call sites.
> 
> The non_block_start/end() annotation causes the scheduler to complain
> about *any* voluntary sleep in a non-blockable notifier. But that was
> never the actual constraint. As Michal Hocko put it when the
> annotation was first proposed (quoted in commit 312364f3534c
> ("kernel.h: Add non_block_start/end()")), the OOM reaper "shouldn't
> depend on any locks or sleepable conditionals" and checking for
> sleepable context was "the best thing we could come up with that would
> describe these demands at least partially". The real requirement is that the reaper
> must not block on anything which may itself depend on memory
> allocation (or on the dying mm) to make progress — which is why
> spinning locks were always considered fine.

I think it's conceptually more than that: "we mostly do care about it to make a
forward progress". So yes, memory allocations are the obvious problem, but we
also wouldn't want to wait on any lock that will be hard/impossible to get while
reaping.

Just take a look at what some mmu_notifier_range_blockable() users end up doing:
they skip taking locks.

[...]

> 
> diff --git a/mm/mmu_notifier.c b/mm/mmu_notifier.c
> index 245b74f39f91..cd5d15cd646a 100644
> --- a/mm/mmu_notifier.c
> +++ b/mm/mmu_notifier.c
> @@ -520,11 +520,7 @@ static int mn_hlist_invalidate_range_start(
>  		if (ops->invalidate_range_start) {
>  			int _ret;
>  
> -			if (!mmu_notifier_range_blockable(range))
> -				non_block_start();
>  			_ret = ops->invalidate_range_start(subscription, range);
> -			if (!mmu_notifier_range_blockable(range))
> -				non_block_end();
>  			if (_ret) {
>  				pr_info("%pS callback failed with %d in %sblockable context.\n",
>  					ops->invalidate_range_start, _ret,
> @@ -591,14 +587,9 @@ mn_hlist_invalidate_end(struct mmu_notifier_subscriptions *subscriptions,
>  	id = srcu_read_lock(&srcu);
>  	hlist_for_each_entry_srcu(subscription, &subscriptions->list, hlist,
>  				 srcu_read_lock_held(&srcu)) {
> -		if (subscription->ops->invalidate_range_end) {
> -			if (!mmu_notifier_range_blockable(range))
> -				non_block_start();
> +		if (subscription->ops->invalidate_range_end)
>  			subscription->ops->invalidate_range_end(subscription,
>  								range);
> -			if (!mmu_notifier_range_blockable(range))
> -				non_block_end();
> -		}
>  	}
>  	srcu_read_unlock(&srcu, id);
>  }

It's a bit odd. We have infrastructure to disallow blocking, and do so on
multiple paths (just check for mmu_notifier_range_blockable() users where we
skip taking mutexes, not performing memory allocations!), but now essentially
allow blocking on some paths.

That's just inconsistent. If we want different semantics, I think the whole
thing should be re-thought: if blocking is suddenly allowed.

-- 
Cheers,

David