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

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