[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

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 Woodhouse]

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

On Tue, 2026-08-11 at 22:21 +0200, Paolo Bonzini wrote:
> 
> 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);


I'm literally sitting here watching you reinvent what SRCU already
does.

Which, again, is *fine* given the behaviour of *these* read-side
sections.

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

And now we've come full circle to the rwlock we already *have* — but
which is back to a shared cache line for readers, and which RT turns
into a sleeping lock, which is what I started out trying to fix in the
first place.

I get it. RCU is a versatile tool, and in *some* cases the *Sleepable*
variant of RCU gets used in ways which would cause grace period
latencies which are thoroughly unacceptable in an MMU notifier
callback.

But in the specific case of a dedicated kvm->gpc_srcu whose only
readers are tiny snippets of code that until today were always run with
rwlock_irqsave? That is *not* Jason's worst-case bugbear. It's fine.

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

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

[Sean Christopherson]

On Tue, Aug 11, 2026, David Woodhouse wrote:
> On Tue, 2026-08-11 at 22:21 +0200, Paolo Bonzini wrote:
> > 
> > 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);
> 
> 
> I'm literally sitting here watching you reinvent what SRCU already does.

The problem, for me at least, is that SRCU does this and a _lot_ more.

> Which, again, is *fine* given the behaviour of *these* read-side sections.

Well, yeah.  Why use a nailgun to put a nail into the wall to hang a picture
frame? (other than the indisputable fact that power tools are fun).  SRCU is
fantastic infrastructure, but I genuinely think it's not the right fit here,
especially since we'll likely need a different solution for in-guest readers
anyways.

> I get it. RCU is a versatile tool, and in *some* cases the *Sleepable*
> variant of RCU gets used in ways which would cause grace period latencies
> which are thoroughly unacceptable in an MMU notifier callback.
> 
> But in the specific case of a dedicated kvm->gpc_srcu whose only
> readers are tiny snippets of code that until today were always run with
> rwlock_irqsave? That is *not* Jason's worst-case bugbear. It's fine.

I agree it's more than likely fine for your use case, I'm not convinced it'll be
fine for use cases that heavily overcommit memory, or do NUMA balancing, or (close
your eyes) enable KSM.

E.g. it doesn't take all that much for even a single expedited sync to generate
a delay of 20ms.
https://lore.kernel.org/all/20260309193059.2244645-1-seanjc@google.com

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

[David Woodhouse]

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

On Tue, 2026-08-11 at 15:58 -0700, Sean Christopherson wrote:
> On Tue, Aug 11, 2026, David Woodhouse wrote:
> > On Tue, 2026-08-11 at 22:21 +0200, Paolo Bonzini wrote:
> > > 
> > > 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);
> > 
> > 
> > I'm literally sitting here watching you reinvent what SRCU already does.
> 
> The problem, for me at least, is that SRCU does this and a _lot_ more.
> 
> > Which, again, is *fine* given the behaviour of *these* read-side sections.
> 
> Well, yeah.  Why use a nailgun to put a nail into the wall to hang a picture
> frame? (other than the indisputable fact that power tools are fun). 

It's not a nailgun, it's a drill. And in this analogy, my observation
that it works absolutely fine for us with *these* SRCU read-side users
... is equivalent to finding the torque setting that lets you put it
into screwdriver mode.

Power tools *are* fun. But I try to avoid buying a new one when I
already have one that does the job correctly with care.


>  SRCU is
> fantastic infrastructure, but I genuinely think it's not the right fit here,
> especially since we'll likely need a different solution for in-guest readers
> anyways.

The cases where we have to hold the host physical address *during* the
time we enter guest mode to run a vCPU were never covered by the rwlock
either.

Fred's patches which reinstate that GUEST_USES_GPC mode are kicking the
vCPU from the invalidate_start handler, just as the original
implementation did. However the locking (or RCU) for the in-kernel
users ends up changing, I don't see the in-guest part changing.

Unless you're imagining some scheme which could handle both? But the
in-kernel readers are *brief* and will always go away on their own,
while the in-guest needs to be kicked; they are opposites.

> > I get it. RCU is a versatile tool, and in *some* cases the *Sleepable*
> > variant of RCU gets used in ways which would cause grace period latencies
> > which are thoroughly unacceptable in an MMU notifier callback.
> > 
> > But in the specific case of a dedicated kvm->gpc_srcu whose only
> > readers are tiny snippets of code that until today were always run with
> > rwlock_irqsave? That is *not* Jason's worst-case bugbear. It's fine.
> 
> I agree it's more than likely fine for your use case, I'm not convinced it'll be
> fine for use cases that heavily overcommit memory, or do NUMA balancing, or (close
> your eyes) enable KSM.
> 
> E.g. it doesn't take all that much for even a single expedited sync to generate
> a delay of 20ms.
> https://lore.kernel.org/all/20260309193059.2244645-1-seanjc@google.com

That looks to me like a pathological case where KVM is trying to use a
single srcu_struct in two different modes, and getting sad when they
conflict. That would never be the case for the separate gpc_srcu.

I've been playing with the soak tests, running in parallel with a load
of about 140, with KASAN/lockdep/etc enabled.

I also implemented that thing I suggested: a try_synchronize_srcu()
which doesn't even use the waitqueue when it can trivially determine
that it doesn't need to wait.

Even when we're stressing invalidation vs readers, it ends up taking
the fast path in about 99% of cases, taking around 4-16µs for those;
32-128µs when it does need to call srcu_synchronize_expedited().

It goes above 8ms in 4.4ppm of cases, with the p100 outlier being
17.6ms. My gut tells me we'd see those occasional outliers even with
the rwlock at this load, but I'm going to kick that test off and answer
that definitively in the morning.





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

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

[Michal Hocko]

On Tue 11-08-26 10:42:29, 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.

Yes, this makes a lot of sense to me. While it is not really great that
the oom_repaer gets blocked by a RT sleeping lock because it delays the
whole operation this shouldn't break the "do not make any direct or
indirect dependency to MM" assumption as those locks are normally
spinlocks so there is no way to depend on blockable allocations.

So the warning is mostly a false positive on RT configs and what you
propose below makes sense.

> 
> 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();
> 
> ?

-- 
Michal Hocko
SUSE Labs

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

[Michal Hocko]

On Tue 11-08-26 16:24:48, David Woodhouse wrote:
[...]
> I had a second reason for disabling the overzealous check too: to allow
> SRCU grace periods within the notifier callbacks.

Is there a way that srcu barrier could cause an indirect dependency on
memory allocation? In other words what might block the scru to complete?

-- 
Michal Hocko
SUSE Labs

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

[David Woodhouse]

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

On Wed, 2026-08-12 at 10:14 +0200, Michal Hocko wrote:
> On Tue 11-08-26 16:24:48, David Woodhouse wrote:
> [...]
> > I had a second reason for disabling the overzealous check too: to allow
> > SRCU grace periods within the notifier callbacks.
> 
> Is there a way that srcu barrier could cause an indirect dependency on
> memory allocation? In other words what might block the scru to complete?

Not after https://lore.kernel.org/all/6eed3fe3461e9690b486ca98fa7563f60d3940ff.camel@infradead.org/


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

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

On Wed, 2026-08-12 at 00:50 +0100, David Woodhouse wrote:
> On Tue, 2026-08-11 at 15:58 -0700, Sean Christopherson wrote:
> >  SRCU is
> > fantastic infrastructure, but I genuinely think it's not the right fit here,
> > especially since we'll likely need a different solution for in-guest readers
> > anyways.

Actually I think it fits quite nicely into the GPC SRCU model. We add a
GUEST_USING_GPC flag to the gpc->state atomic. A vCPU which is going to
use the GPC in guest mode will do an atomic cmpxchg to set
GUEST_USING_GPC while the GPC_VALID flag is still set.

And then in the flush side, *exactly* the same loop which already sets
'cleared' when a cache has GPC_VALID, will also spot that
GUEST_USING_GPC and add the corresponding vCPU to the bitmask of vCPUs
to be kicked.

It's a perfect fit.

In fact, we could observe that the GUEST_USES_GPC concept has *always*
been closer to the RCU model of "readers do as they like; the writer
has to wait until they're done".

It was mixing that with rwlock that was odd.

> The cases where we have to hold the host physical address *during* the
> time we enter guest mode to run a vCPU were never covered by the rwlock
> either.
> 
> Fred's patches which reinstate that GUEST_USES_GPC mode are kicking the
> vCPU from the invalidate_start handler, just as the original
> implementation did. However the locking (or RCU) for the in-kernel
> users ends up changing, I don't see the in-guest part changing.
> 
> Unless you're imagining some scheme which could handle both? But the
> in-kernel readers are *brief* and will always go away on their own,
> while the in-guest needs to be kicked; they are opposites.

Honestly, if we're going to start re-inventing concurrency primitives
just because "SRCU *can* be used badly", even though we weren't *going*
to use SRCU badly in this case... maybe I should revisit my starting
assumption when I first started digging into this, which was that
"nobody wants a raw_rwlock_t as a workaround to the RT issues".

And if we want to invent new primitives, because that's *always* a
barrel of laughs, maybe we *could* try to build something really
dedicated to this use case...

Since every access is at least tenuously associated with *a* vCPU, we
could have a spinlock per vCPU and each read side takes "its own" while
the flush side takes all of them in turn, one at a time. That probably
only needs one such spinlock per vCPU (not per-GPC per-vCPU).

But maybe we can handle the GUEST_USES_GPC mode too... instead of a
spinlock, have an atomic_t of the number of readers, with a high bit
indicating that it's in-guest.... but honestly, I just don't want to.

SRCU is *fine* here. Let's not overcomplicate things.

> > > I get it. RCU is a versatile tool, and in *some* cases the *Sleepable*
> > > variant of RCU gets used in ways which would cause grace period latencies
> > > which are thoroughly unacceptable in an MMU notifier callback.
> > > 
> > > But in the specific case of a dedicated kvm->gpc_srcu whose only
> > > readers are tiny snippets of code that until today were always run with
> > > rwlock_irqsave? That is *not* Jason's worst-case bugbear. It's fine.
> > 
> > I agree it's more than likely fine for your use case, I'm not convinced it'll be
> > fine for use cases that heavily overcommit memory, or do NUMA balancing, or (close
> > your eyes) enable KSM.
> > 
> > E.g. it doesn't take all that much for even a single expedited sync to generate
> > a delay of 20ms.
> > https://lore.kernel.org/all/20260309193059.2244645-1-seanjc@google.com
> 
> That looks to me like a pathological case where KVM is trying to use a
> single srcu_struct in two different modes, and getting sad when they
> conflict. That would never be the case for the separate gpc_srcu.
> 
> I've been playing with the soak tests, running in parallel with a load
> of about 140, with KASAN/lockdep/etc enabled.
> 
> I also implemented that thing I suggested: a try_synchronize_srcu()
> which doesn't even use the waitqueue when it can trivially determine
> that it doesn't need to wait.
> 
> Even when we're stressing invalidation vs readers, it ends up taking
> the fast path in about 99% of cases, taking around 4-16µs for those;
> 32-128µs when it does need to call srcu_synchronize_expedited().
> 
> It goes above 8ms in 4.4ppm of cases, with the p100 outlier being
> 17.6ms. My gut tells me we'd see those occasional outliers even with
> the rwlock at this load, but I'm going to kick that test off and answer
> that definitively in the morning.

That one crashed (reminding us that I was actually doing this to fix
*bugs* not just chase the last bits of performance). A rerun this
morning shows a p100 latency of ~11ms for the rwlock case too.

I redid the tests to also track the costs of the *other* thing that 
the KVM mmu_notifier does; poking pages out of the EPT and kicking
vCPUs. The p100 there was only about 2½ms for both EPT and shadow
paging mode. I wasn't able to test with ATS+PASID capable devices,
which I suspect would have led to larger numbers :)

Really, I just don't think Jason's tail-latency bugbear is actually
anything to be scared of. It's not new, and it's not dramatically
different. 

[-- 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 Wed, Aug 12, 2026 at 09:21:31AM +0100, David Woodhouse wrote:
> On Wed, 2026-08-12 at 10:14 +0200, Michal Hocko wrote:
> > On Tue 11-08-26 16:24:48, David Woodhouse wrote:
> > [...]
> > > I had a second reason for disabling the overzealous check too: to allow
> > > SRCU grace periods within the notifier callbacks.
> > 
> > Is there a way that srcu barrier could cause an indirect dependency on
> > memory allocation? In other words what might block the scru to complete?
> 
> Not after https://lore.kernel.org/all/6eed3fe3461e9690b486ca98fa7563f60d3940ff.camel@infradead.org/

The user of the SRCU might have a read side that wraps an allocate. In
general it is not safe.

I'm skeptical that without special API and documentation the KVM
special use of SRCU you've outlined will not remain working long term
too..

Jason

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

[David Woodhouse]

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

On Wed, 2026-08-12 at 09:27 -0300, Jason Gunthorpe wrote:
> On Wed, Aug 12, 2026 at 09:21:31AM +0100, David Woodhouse wrote:
> > On Wed, 2026-08-12 at 10:14 +0200, Michal Hocko wrote:
> > > On Tue 11-08-26 16:24:48, David Woodhouse wrote:
> > > [...]
> > > > I had a second reason for disabling the overzealous check too: to allow
> > > > SRCU grace periods within the notifier callbacks.
> > > 
> > > Is there a way that srcu barrier could cause an indirect dependency on
> > > memory allocation? In other words what might block the scru to complete?
> > 
> > Not after https://lore.kernel.org/all/6eed3fe3461e9690b486ca98fa7563f60d3940ff.camel@infradead.org/
> 
> The user of the SRCU might have a read side that wraps an allocate. In
> general it is not safe.

Sure, the *general* case of Sleepable RCU can do all kinds of stuff in
the read-side sections.

But in *this* case they were all read_lock_irqsave() thus far, and they
absolutely categorically do not.

> I'm skeptical that without special API and documentation the KVM
> special use of SRCU you've outlined will not remain working long term
> too..

I hear you. I'm less sceptical, given the use case, but I'll certainly
concede that the universe is known for inventing better idiots.

I'm certainly not averse to having a wrapper so that the read sections
don't just srcu_read_lock() directly, but also get a sanity check
(non_block_start? local_irq_save? what would it actually be? does it
have to be different for the hardirq vs. process-context invocations?)
for free.

Or instead of a GPC-specific wrapper, do we make it a first-class SRCU
citizen, SRCU_READ_FLAVOR_ATOMIC? If it disables preemption, we'd know
that lock+unlock must happen on the *same* CPU, which could potentially
simplify my new try_synchronize_srcu() and/or allow for a *spinning*
grace period if we really do want that (although I'm unconvinced; it's
*latency* we care about here, and punting to the WQ when we *already*
have to wait is fair enough).

Either way, I see that as a mostly cosmetic follow-on from the real
fixes I'm chasing here.

[-- 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 Wed, Aug 12, 2026 at 02:46:08PM +0100, David Woodhouse wrote:
> I hear you. I'm less sceptical, given the use case, but I'll certainly
> concede that the universe is known for inventing better idiots.
> 
> I'm certainly not averse to having a wrapper so that the read sections
> don't just srcu_read_lock() directly, but also get a sanity check
> (non_block_start? local_irq_save? what would it actually be? does it
> have to be different for the hardirq vs. process-context invocations?)
> for free.
> 
> Or instead of a GPC-specific wrapper, do we make it a first-class SRCU
> citizen, SRCU_READ_FLAVOR_ATOMIC? If it disables preemption, we'd know
> that lock+unlock must happen on the *same* CPU, which could potentially
> simplify my new try_synchronize_srcu() and/or allow for a *spinning*
> grace period if we really do want that (although I'm unconvinced; it's
> *latency* we care about here, and punting to the WQ when we *already*
> have to wait is fair enough).

These all seem like good ideas, then you don't need to touch the
notifiers..

Jason

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

[David Woodhouse]

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

On Wed, 2026-08-12 at 10:49 -0300, Jason Gunthorpe wrote:
> On Wed, Aug 12, 2026 at 02:46:08PM +0100, David Woodhouse wrote:
> > I hear you. I'm less sceptical, given the use case, but I'll certainly
> > concede that the universe is known for inventing better idiots.
> > 
> > I'm certainly not averse to having a wrapper so that the read sections
> > don't just srcu_read_lock() directly, but also get a sanity check
> > (non_block_start? local_irq_save? what would it actually be? does it
> > have to be different for the hardirq vs. process-context invocations?)
> > for free.
> > 
> > Or instead of a GPC-specific wrapper, do we make it a first-class SRCU
> > citizen, SRCU_READ_FLAVOR_ATOMIC? If it disables preemption, we'd know
> > that lock+unlock must happen on the *same* CPU, which could potentially
> > simplify my new try_synchronize_srcu() and/or allow for a *spinning*
> > grace period if we really do want that (although I'm unconvinced; it's
> > *latency* we care about here, and punting to the WQ when we *already*
> > have to wait is fair enough).
> 
> These all seem like good ideas, then you don't need to touch the
> notifiers..

I'll rephrase that for my own understanding:

*If* we go all the way to building a whole SRCU flavour for this *and*
implementing a spin-only variant of srcu_synchronize() which is
tailored to the atomic-reader use case, *then* we don't need to remove
the non_block_{start,end} guards around the MMU notifiers, which are
basically never being called anyway and don't actually seem to protect
against any real bugs.

Yes?

[-- 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 Wed, Aug 12, 2026 at 03:05:43PM +0100, David Woodhouse wrote:
> the non_block_{start,end} guards around the MMU notifiers, which are
> basically never being called anyway and don't actually seem to protect
> against any real bugs.

If you think this blocked stuff is dead code then lets remove it, but
I'm pretty sure it is called and I remember seeing bug reports about
it being triggered in the wild. Vetter certainly added it because
their tests were actually triggering and they had bugs in their DRM
stack directly connected to this.

It might not trigger for your hypervisor case but we aren't here just
to make only kvm work now are we?

Jason

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

[David Woodhouse]

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

On Wed, 2026-08-12 at 15:05 +0100, David Woodhouse wrote:
> I'll rephrase that for my own understanding:
> 
> *If* we go all the way to building a whole SRCU flavour for this *and*
> implementing a spin-only variant of srcu_synchronize() which is
> tailored to the atomic-reader use case, *then* we don't need to remove
> the non_block_{start,end} guards around the MMU notifiers, which are
> basically never being called anyway and don't actually seem to protect
> against any real bugs.
> 
> Yes?

FWIW it looks something like this. I'll throw it into my torture and
latency tests, and we can see what Paul thinks of it. I'm still utterly
unconvinced it's needed, but I concede it has its good points.

From f912e839bf55759a1969e38d9e7897a30967fdb1 Mon Sep 17 00:00:00 2001
From: David Woodhouse <dwmw@amazon.co.uk>
Date: Wed, 12 Aug 2026 15:29:12 +0100
Subject: [PATCH 1/2] srcu: Add an ATOMIC reader flavor and a spinning
 synchronize_srcu_atomic()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Some dedicated srcu_struct users have read-side critical sections
which are short, never sleep, and never block on anything which may
itself depend on memory allocation — because they were, until
recently, spinlock or rwlock critical sections. For such a domain the
update side can safely wait for readers by spinning, from contexts
where sleeping is undesirable or the grace-period machinery's
latency (workqueue scheduling, jiffy-paced retries) dominates the
actual reader drain time.

But that is only safe if every reader keeps the promise. Make the
promise explicit and machine-checkable:

 - srcu_read_lock_atomic() / srcu_read_unlock_atomic() enter the
   usual (smp_mb-based) read-side critical section with preemption
   disabled and (except in hardirq, where it is redundant)
   non_block_start() armed, recording SRCU_READ_FLAVOR_ATOMIC in the
   per-CPU reader flavor. Disabling preemption enforces the
   no-sleeping promise on every configuration and bounds the section,
   so it is always running on some CPU; non_block_start() extends the
   enforcement to even potentially-sleeping calls on paths which
   happen not to block. The existing reader-flavor consistency checks
   complain about any mixing with other flavors.

 - synchronize_srcu_atomic() waits for all pre-existing readers by
   repeating the try_synchronize_srcu() both-epoch counter proof with
   cpu_relax() until it succeeds: no sleeping, no index flip, no
   grace-period sequence update, and therefore no interaction with
   concurrent call_srcu(), synchronize_srcu() or srcu_barrier(). It
   always provides the full grace-period guarantee: if the domain
   turns out to have had readers of any other flavor — a caller bug,
   since such a reader may be asleep and spinning on it would be
   unbounded — it complains and falls back to a real (sleeping) grace
   period internally, that being the only correct wait for a
   possibly-sleeping reader. The flavor mask is rechecked on every
   iteration so a first non-atomic reader appearing mid-spin takes
   the same path.

On Tiny SRCU (!SMP), spinning cannot be sane — an observed mid-section
reader can only make progress if we yield — but it is also never
needed: an atomic-flavor reader cannot be observed mid-section from
process context on the sole CPU, so the try_synchronize_srcu() proof
either succeeds immediately or the fallback is required anyway.

The immediate motivation is the proposed conversion of KVM's
gfn_to_pfn_cache to SRCU¹, whose mmu_notifier invalidation path drains
readers before the primary MMU zaps a page. With the readers declared
atomic, that drain becomes spin-only: no sleeping at all in the
notifier, bounded by the longest reader section, satisfying even the
strictest reading of the OOM-reaper non-blocking requirement without
needing to touch the non_block_start() annotation².

¹ https://lore.kernel.org/all/20260811132237.102400-1-dwmw2@infradead.org/
² https://lore.kernel.org/all/20260812134934.GC662699@ziepe.ca/

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Claude:claude-mythos-5
---
 include/linux/srcu.h  | 59 ++++++++++++++++++++++++++++++++++++++-
 kernel/rcu/srcutiny.c | 21 ++++++++++++++
 kernel/rcu/srcutree.c | 65 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 144 insertions(+), 1 deletion(-)

diff --git a/include/linux/srcu.h b/include/linux/srcu.h
index 0d7543c7becc..c81dfd0896ad 100644
--- a/include/linux/srcu.h
+++ b/include/linux/srcu.h
@@ -71,8 +71,10 @@ int init_srcu_struct_fast_updown(struct srcu_struct *ssp);
 //					0x4		// SRCU-lite is no longer with us.
 #define SRCU_READ_FLAVOR_FAST		0x4		// srcu_read_lock_fast(), also NMI-safe.
 #define SRCU_READ_FLAVOR_FAST_UPDOWN	0x8		// srcu_read_lock_fast_updown().
+#define SRCU_READ_FLAVOR_ATOMIC		0x10		// srcu_read_lock_atomic().
 #define SRCU_READ_FLAVOR_ALL		(SRCU_READ_FLAVOR_NORMAL | SRCU_READ_FLAVOR_NMI | \
-					 SRCU_READ_FLAVOR_FAST | SRCU_READ_FLAVOR_FAST_UPDOWN)
+					 SRCU_READ_FLAVOR_FAST | SRCU_READ_FLAVOR_FAST_UPDOWN | \
+					 SRCU_READ_FLAVOR_ATOMIC)
 						// All of the above.
 #define SRCU_READ_FLAVOR_SLOWGP		(SRCU_READ_FLAVOR_FAST | SRCU_READ_FLAVOR_FAST_UPDOWN)
 						// Flavors requiring synchronize_rcu()
@@ -92,6 +94,7 @@ void call_srcu(struct srcu_struct *ssp, struct rcu_head *head,
 void cleanup_srcu_struct(struct srcu_struct *ssp);
 void synchronize_srcu(struct srcu_struct *ssp);
 bool try_synchronize_srcu(struct srcu_struct *ssp);
+void synchronize_srcu_atomic(struct srcu_struct *ssp);
 
 #define SRCU_GET_STATE_COMPLETED 0x1
 
@@ -296,6 +299,43 @@ static inline int srcu_read_lock(struct srcu_struct *ssp)
 	return retval;
 }
 
+/**
+ * srcu_read_lock_atomic - register a new reader promising an atomic section
+ * @ssp: srcu_struct in which to register the new reader.
+ *
+ * As srcu_read_lock(), but the caller promises that the read-side
+ * critical section never sleeps and never blocks on anything which
+ * may itself depend on memory allocation to make progress. Preemption
+ * is disabled for the duration, which both enforces that promise (any
+ * sleepable call in the section will splat on every configuration)
+ * and bounds the section so that the update side may spin rather
+ * than sleep when waiting for readers: see synchronize_srcu_atomic().
+ *
+ * The lock and matching srcu_read_unlock_atomic() must be invoked on
+ * the same CPU, from the same context; passing the return value to
+ * another task is not permitted for this flavor.
+ */
+static inline int srcu_read_lock_atomic(struct srcu_struct *ssp)
+	__acquires_shared(ssp)
+{
+	int retval;
+
+	preempt_disable();
+	/*
+	 * Arm might_sleep() to catch even a *potentially* sleeping call
+	 * in the section, not just an actual schedule: the atomic-domain
+	 * promise must hold on every path, contended or not. In hardirq
+	 * the annotation would land on the interrupted task; it is also
+	 * redundant there, so skip it.
+	 */
+	if (!in_hardirq())
+		non_block_start();
+	srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_ATOMIC);
+	retval = __srcu_read_lock(ssp);
+	srcu_lock_acquire(&ssp->dep_map);
+	return retval;
+}
+
 /**
  * srcu_read_lock_fast - register a new reader for an SRCU-protected structure.
  * @ssp: srcu_struct in which to register the new reader.
@@ -488,6 +528,23 @@ static inline void srcu_read_unlock(struct srcu_struct *ssp, int idx)
 	__srcu_read_unlock(ssp, idx);
 }
 
+/**
+ * srcu_read_unlock_atomic - unregister an atomic-section reader
+ * @ssp: srcu_struct from which to unregister the old reader.
+ * @idx: return value from corresponding srcu_read_lock_atomic().
+ */
+static inline void srcu_read_unlock_atomic(struct srcu_struct *ssp, int idx)
+	__releases_shared(ssp)
+{
+	WARN_ON_ONCE(idx & ~0x1);
+	srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_ATOMIC);
+	srcu_lock_release(&ssp->dep_map);
+	__srcu_read_unlock(ssp, idx);
+	if (!in_hardirq())
+		non_block_end();
+	preempt_enable();
+}
+
 /**
  * srcu_read_unlock_fast - unregister a old reader from an SRCU-protected structure.
  * @ssp: srcu_struct in which to unregister the old reader.
diff --git a/kernel/rcu/srcutiny.c b/kernel/rcu/srcutiny.c
index f07159cae241..c04112d9c332 100644
--- a/kernel/rcu/srcutiny.c
+++ b/kernel/rcu/srcutiny.c
@@ -283,6 +283,27 @@ bool try_synchronize_srcu(struct srcu_struct *ssp)
 }
 EXPORT_SYMBOL_GPL(try_synchronize_srcu);
 
+/**
+ * synchronize_srcu_atomic - spinning grace period for atomic-reader domains
+ * @ssp: srcu_struct with which to synchronize.
+ *
+ * On !SMP this cannot spin: a reader observed mid-section is preempted
+ * or interrupted-out, and can only finish if we yield the CPU. But it
+ * is also never needed: an atomic-flavor reader (preemption disabled)
+ * cannot be observed mid-section from process context on the sole CPU.
+ * So a reader observed here has broken the atomic-domain promise, and
+ * the only correct wait for it is a real grace period.
+ */
+void synchronize_srcu_atomic(struct srcu_struct *ssp)
+{
+	if (try_synchronize_srcu(ssp))
+		return;
+	WARN_ONCE(1, "non-atomic readers on srcu_struct at %ps; falling back to sleeping grace period\n",
+		  ssp);
+	synchronize_srcu(ssp);
+}
+EXPORT_SYMBOL_GPL(synchronize_srcu_atomic);
+
 /*
  * get_state_synchronize_srcu - Provide an end-of-grace-period cookie
  */
diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
index 76a78336feaa..5fa10c07cb3f 100644
--- a/kernel/rcu/srcutree.c
+++ b/kernel/rcu/srcutree.c
@@ -1715,6 +1715,71 @@ bool try_synchronize_srcu(struct srcu_struct *ssp)
 }
 EXPORT_SYMBOL_GPL(try_synchronize_srcu);
 
+/**
+ * synchronize_srcu_atomic - spinning grace period for atomic-reader domains
+ * @ssp: srcu_struct with which to synchronize.
+ *
+ * Wait for all pre-existing readers of @ssp to complete, without
+ * sleeping and without involving the SRCU grace-period machinery —
+ * provided the domain keeps the promise that all of its readers use
+ * srcu_read_lock_atomic().
+ *
+ * Atomic-flavor readers run with preemption disabled, so every
+ * read-side critical section is bounded and running (not blocked) on
+ * some CPU, which is what makes spinning here sane: the wait is
+ * bounded by the longest such section. The proof of reader absence is
+ * the same both-epoch counter comparison as try_synchronize_srcu(),
+ * repeated until it succeeds; no index flip and no grace-period
+ * sequence update occur, so concurrent call_srcu(), synchronize_srcu()
+ * and srcu_barrier() are entirely unaffected.
+ *
+ * If the domain has ever had readers of any other flavor, the promise
+ * is broken — such a reader may be asleep, and spinning on it would be
+ * unbounded. That is a caller bug: complain, and fall back to a real
+ * (sleeping) grace period, which is the only correct wait for a
+ * possibly-sleeping reader. The reader-flavor mask is rechecked on
+ * every iteration so a first non-atomic reader appearing mid-spin
+ * takes the same path.
+ */
+void synchronize_srcu_atomic(struct srcu_struct *ssp)
+{
+	unsigned long unlocks0, unlocks1;
+	unsigned long rdm0, rdm1;
+
+	check_init_srcu_struct(ssp);
+
+	/*
+	 * Order the caller's prior stores before the counter reads;
+	 * pairs with the smp_mb() in __srcu_read_lock() as described
+	 * in try_synchronize_srcu().
+	 */
+	smp_mb();
+
+	for (;;) {
+		unlocks0 = srcu_readers_unlock_idx(ssp, 0, &rdm0);
+		unlocks1 = srcu_readers_unlock_idx(ssp, 1, &rdm1);
+
+		if (WARN_ONCE((rdm0 | rdm1) & ~SRCU_READ_FLAVOR_ATOMIC,
+			      "non-atomic readers on srcu_struct at %ps; falling back to sleeping grace period\n",
+			      ssp)) {
+			synchronize_srcu_expedited(ssp);
+			return;
+		}
+
+		smp_mb();
+
+		if (srcu_readers_lock_idx(ssp, 0, false, unlocks0) &&
+		    srcu_readers_lock_idx(ssp, 1, false, unlocks1))
+			break;
+
+		cpu_relax();
+	}
+
+	/* Order the caller's subsequent accesses after the proof. */
+	smp_mb();
+}
+EXPORT_SYMBOL_GPL(synchronize_srcu_atomic);
+
 /**
  * get_state_synchronize_srcu - Provide an end-of-grace-period cookie
  * @ssp: srcu_struct to provide cookie for.
-- 
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

[David Woodhouse]

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

On Wed, 2026-08-12 at 11:26 -0300, Jason Gunthorpe wrote:
> On Wed, Aug 12, 2026 at 03:05:43PM +0100, David Woodhouse wrote:
> > the non_block_{start,end} guards around the MMU notifiers, which are
> > basically never being called anyway and don't actually seem to protect
> > against any real bugs.
> 
> If you think this blocked stuff is dead code then lets remove it, but
> I'm pretty sure it is called and I remember seeing bug reports about
> it being triggered in the wild. Vetter certainly added it because
> their tests were actually triggering and they had bugs in their DRM
> stack directly connected to this.
> 
> It might not trigger for your hypervisor case but we aren't here just
> to make only kvm work now are we?

I'm sure there can be real-world use cases where it can trigger, but
when I was *trying* to exercise the code path, to validate my belief
(from source inspection) that even the plain spinlock would trigger the
splat, I ended up having to hack the kernel to do so:

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

Perhaps in the past it was easier to trigger 'naturally'?

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

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

On Wed, 2026-08-12 at 15:34 +0100, David Woodhouse wrote:
> On Wed, 2026-08-12 at 15:05 +0100, David Woodhouse wrote:
> > I'll rephrase that for my own understanding:
> > 
> > *If* we go all the way to building a whole SRCU flavour for this *and*
> > implementing a spin-only variant of srcu_synchronize() which is
> > tailored to the atomic-reader use case, *then* we don't need to remove
> > the non_block_{start,end} guards around the MMU notifiers, which are
> > basically never being called anyway and don't actually seem to protect
> > against any real bugs.
> > 
> > Yes?
> 
> FWIW it looks something like this. I'll throw it into my torture and
> latency tests, and we can see what Paul thinks of it. I'm still utterly
> unconvinced it's needed, but I concede it has its good points.

This slightly refactored version is the one that's actually going into
my torture tests...

I'll defer to Paul here. If we think this whole srcu_read_lock_atomic
thing is really worth it for SRCU in the general case — perhaps we can
use it to convert existing rwlock_t users, some of whom have been also
blindsided by those becoming sleepable on RT — then I guess it makes
sense.

I can't honestly defend the added complexity *purely* for the KVM GPC
use case though; I just can't bring myself to lose sleep over the use
of the wait queue from the MMU notifier even in the OOM reaper path.
It's *latency* that matters.

From f09baac8dc596596018e7009c15a1cd73cdfd421 Mon Sep 17 00:00:00 2001
From: David Woodhouse <dwmw@amazon.co.uk>
Date: Wed, 12 Aug 2026 15:29:12 +0100
Subject: [PATCH 1/2] srcu: Add an ATOMIC reader flavor and a spinning
 synchronize_srcu_atomic()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Some dedicated srcu_struct users have read-side critical sections
which are short, never sleep, and never block on anything which may
itself depend on memory allocation — because they were, until
recently, spinlock or rwlock critical sections. For such a domain the
update side can safely wait for readers by spinning, from contexts
where sleeping is undesirable or the grace-period machinery's
latency (workqueue scheduling, jiffy-paced retries) dominates the
actual reader drain time.

But that is only safe if every reader keeps the promise. Make the
promise explicit and machine-checkable:

 - srcu_read_lock_atomic() / srcu_read_unlock_atomic() enter the
   usual (smp_mb-based) read-side critical section with preemption
   disabled and (except in hardirq, where it is redundant)
   non_block_start() armed, recording SRCU_READ_FLAVOR_ATOMIC in the
   per-CPU reader flavor. Disabling preemption enforces the
   no-sleeping promise on every configuration and bounds the section,
   so it is always running on some CPU; non_block_start() extends the
   enforcement to even potentially-sleeping calls on paths which
   happen not to block. The existing reader-flavor consistency checks
   complain about any mixing with other flavors.

 - synchronize_srcu_atomic() waits for all pre-existing readers by
   repeating the try_synchronize_srcu() both-epoch counter proof with
   cpu_relax() until it succeeds: no sleeping, no index flip, no
   grace-period sequence update, and therefore no interaction with
   concurrent call_srcu(), synchronize_srcu() or srcu_barrier(). It
   always provides the full grace-period guarantee: if the domain
   turns out to have had readers of any other flavor — a caller bug,
   since such a reader may be asleep and spinning on it would be
   unbounded — it complains and falls back to a real (sleeping) grace
   period internally, that being the only correct wait for a
   possibly-sleeping reader. The flavor mask is rechecked on every
   iteration so a first non-atomic reader appearing mid-spin takes
   the same path.

On Tiny SRCU (!SMP), spinning cannot be sane — an observed mid-section
reader can only make progress if we yield — but it is also never
needed: an atomic-flavor reader cannot be observed mid-section from
process context on the sole CPU, so the try_synchronize_srcu() proof
either succeeds immediately or the fallback is required anyway.

The immediate motivation is the proposed conversion of KVM's
gfn_to_pfn_cache to SRCU¹, whose mmu_notifier invalidation path drains
readers before the primary MMU zaps a page. With the readers declared
atomic, that drain becomes spin-only: no sleeping at all in the
notifier, bounded by the longest reader section, satisfying even the
strictest reading of the OOM-reaper non-blocking requirement without
needing to touch the non_block_start() annotation².

¹ https://lore.kernel.org/all/20260811132237.102400-1-dwmw2@infradead.org/
² https://lore.kernel.org/all/20260812134934.GC662699@ziepe.ca/

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Claude:claude-mythos-5
---
 include/linux/srcu.h  |  59 +++++++++++++++++-
 kernel/rcu/srcutiny.c |  21 +++++++
 kernel/rcu/srcutree.c | 135 ++++++++++++++++++++++++++++++------------
 3 files changed, 175 insertions(+), 40 deletions(-)

diff --git a/include/linux/srcu.h b/include/linux/srcu.h
index 0d7543c7becc..c81dfd0896ad 100644
--- a/include/linux/srcu.h
+++ b/include/linux/srcu.h
@@ -71,8 +71,10 @@ int init_srcu_struct_fast_updown(struct srcu_struct *ssp);
 //					0x4		// SRCU-lite is no longer with us.
 #define SRCU_READ_FLAVOR_FAST		0x4		// srcu_read_lock_fast(), also NMI-safe.
 #define SRCU_READ_FLAVOR_FAST_UPDOWN	0x8		// srcu_read_lock_fast_updown().
+#define SRCU_READ_FLAVOR_ATOMIC		0x10		// srcu_read_lock_atomic().
 #define SRCU_READ_FLAVOR_ALL		(SRCU_READ_FLAVOR_NORMAL | SRCU_READ_FLAVOR_NMI | \
-					 SRCU_READ_FLAVOR_FAST | SRCU_READ_FLAVOR_FAST_UPDOWN)
+					 SRCU_READ_FLAVOR_FAST | SRCU_READ_FLAVOR_FAST_UPDOWN | \
+					 SRCU_READ_FLAVOR_ATOMIC)
 						// All of the above.
 #define SRCU_READ_FLAVOR_SLOWGP		(SRCU_READ_FLAVOR_FAST | SRCU_READ_FLAVOR_FAST_UPDOWN)
 						// Flavors requiring synchronize_rcu()
@@ -92,6 +94,7 @@ void call_srcu(struct srcu_struct *ssp, struct rcu_head *head,
 void cleanup_srcu_struct(struct srcu_struct *ssp);
 void synchronize_srcu(struct srcu_struct *ssp);
 bool try_synchronize_srcu(struct srcu_struct *ssp);
+void synchronize_srcu_atomic(struct srcu_struct *ssp);
 
 #define SRCU_GET_STATE_COMPLETED 0x1
 
@@ -296,6 +299,43 @@ static inline int srcu_read_lock(struct srcu_struct *ssp)
 	return retval;
 }
 
+/**
+ * srcu_read_lock_atomic - register a new reader promising an atomic section
+ * @ssp: srcu_struct in which to register the new reader.
+ *
+ * As srcu_read_lock(), but the caller promises that the read-side
+ * critical section never sleeps and never blocks on anything which
+ * may itself depend on memory allocation to make progress. Preemption
+ * is disabled for the duration, which both enforces that promise (any
+ * sleepable call in the section will splat on every configuration)
+ * and bounds the section so that the update side may spin rather
+ * than sleep when waiting for readers: see synchronize_srcu_atomic().
+ *
+ * The lock and matching srcu_read_unlock_atomic() must be invoked on
+ * the same CPU, from the same context; passing the return value to
+ * another task is not permitted for this flavor.
+ */
+static inline int srcu_read_lock_atomic(struct srcu_struct *ssp)
+	__acquires_shared(ssp)
+{
+	int retval;
+
+	preempt_disable();
+	/*
+	 * Arm might_sleep() to catch even a *potentially* sleeping call
+	 * in the section, not just an actual schedule: the atomic-domain
+	 * promise must hold on every path, contended or not. In hardirq
+	 * the annotation would land on the interrupted task; it is also
+	 * redundant there, so skip it.
+	 */
+	if (!in_hardirq())
+		non_block_start();
+	srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_ATOMIC);
+	retval = __srcu_read_lock(ssp);
+	srcu_lock_acquire(&ssp->dep_map);
+	return retval;
+}
+
 /**
  * srcu_read_lock_fast - register a new reader for an SRCU-protected structure.
  * @ssp: srcu_struct in which to register the new reader.
@@ -488,6 +528,23 @@ static inline void srcu_read_unlock(struct srcu_struct *ssp, int idx)
 	__srcu_read_unlock(ssp, idx);
 }
 
+/**
+ * srcu_read_unlock_atomic - unregister an atomic-section reader
+ * @ssp: srcu_struct from which to unregister the old reader.
+ * @idx: return value from corresponding srcu_read_lock_atomic().
+ */
+static inline void srcu_read_unlock_atomic(struct srcu_struct *ssp, int idx)
+	__releases_shared(ssp)
+{
+	WARN_ON_ONCE(idx & ~0x1);
+	srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_ATOMIC);
+	srcu_lock_release(&ssp->dep_map);
+	__srcu_read_unlock(ssp, idx);
+	if (!in_hardirq())
+		non_block_end();
+	preempt_enable();
+}
+
 /**
  * srcu_read_unlock_fast - unregister a old reader from an SRCU-protected structure.
  * @ssp: srcu_struct in which to unregister the old reader.
diff --git a/kernel/rcu/srcutiny.c b/kernel/rcu/srcutiny.c
index f07159cae241..c04112d9c332 100644
--- a/kernel/rcu/srcutiny.c
+++ b/kernel/rcu/srcutiny.c
@@ -283,6 +283,27 @@ bool try_synchronize_srcu(struct srcu_struct *ssp)
 }
 EXPORT_SYMBOL_GPL(try_synchronize_srcu);
 
+/**
+ * synchronize_srcu_atomic - spinning grace period for atomic-reader domains
+ * @ssp: srcu_struct with which to synchronize.
+ *
+ * On !SMP this cannot spin: a reader observed mid-section is preempted
+ * or interrupted-out, and can only finish if we yield the CPU. But it
+ * is also never needed: an atomic-flavor reader (preemption disabled)
+ * cannot be observed mid-section from process context on the sole CPU.
+ * So a reader observed here has broken the atomic-domain promise, and
+ * the only correct wait for it is a real grace period.
+ */
+void synchronize_srcu_atomic(struct srcu_struct *ssp)
+{
+	if (try_synchronize_srcu(ssp))
+		return;
+	WARN_ONCE(1, "non-atomic readers on srcu_struct at %ps; falling back to sleeping grace period\n",
+		  ssp);
+	synchronize_srcu(ssp);
+}
+EXPORT_SYMBOL_GPL(synchronize_srcu_atomic);
+
 /*
  * get_state_synchronize_srcu - Provide an end-of-grace-period cookie
  */
diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
index 76a78336feaa..3cafb1c2160b 100644
--- a/kernel/rcu/srcutree.c
+++ b/kernel/rcu/srcutree.c
@@ -1645,6 +1645,47 @@ void synchronize_srcu(struct srcu_struct *ssp)
 }
 EXPORT_SYMBOL_GPL(synchronize_srcu);
 
+/*
+ * Attempt to prove that @ssp has no readers in either epoch. Returns:
+ *   1  - proof succeeded: no pre-existing readers, sums stable.
+ *   0  - readers (or counter movement) observed; try again later.
+ *   -1 - the domain has reader flavors incompatible with @allowed
+ *        (those which elide the read-side smp_mb() that the proof
+ *        depends on, or, for the spinning caller, those which may
+ *        sleep); the caller needs a real grace period.
+ *
+ * The caller must provide the leading smp_mb() ordering its prior
+ * stores before this proof (pairing with the smp_mb() in
+ * __srcu_read_lock()), and is responsible for ordering its subsequent
+ * accesses after a successful proof.
+ */
+static int srcu_readers_provably_absent(struct srcu_struct *ssp, unsigned long allowed)
+{
+	unsigned long unlocks0, unlocks1;
+	unsigned long rdm0, rdm1;
+
+	unlocks0 = srcu_readers_unlock_idx(ssp, 0, &rdm0);
+	unlocks1 = srcu_readers_unlock_idx(ssp, 1, &rdm1);
+
+	if ((rdm0 | rdm1) & ~allowed)
+		return -1;
+
+	/*
+	 * As in srcu_readers_active_idx_check(): ensure that a lock is
+	 * always counted if the corresponding unlock is counted, so that
+	 * a reader racing with these sums can only inflate the lock sum
+	 * and force the (safe) retry or fallback. Summing both epochs
+	 * means no index flip is needed: a stable equality proves there
+	 * was a moment at which no readers existed at all.
+	 */
+	smp_mb();
+
+	if (srcu_readers_lock_idx(ssp, 0, false, unlocks0) &&
+	    srcu_readers_lock_idx(ssp, 1, false, unlocks1))
+		return 1;
+	return 0;
+}
+
 /**
  * try_synchronize_srcu - inline grace period for a reader-free srcu_struct
  * @ssp: srcu_struct with which to synchronize.
@@ -1656,64 +1697,80 @@ EXPORT_SYMBOL_GPL(synchronize_srcu);
  * failure the caller must fall back to synchronize_srcu() or
  * synchronize_srcu_expedited().
  *
- * This serves dedicated srcu_struct structures whose read-side critical
- * sections are short, atomic, and usually absent — where even an
- * expedited grace period costs two trips through the workqueue and an
- * unconditional sleep of the caller, three orders of magnitude more
- * than the check below.
- *
- * Only readers of the srcu_read_lock() and srcu_read_lock_nmisafe()
- * flavors are compatible with this proof; if the _fast() flavors have
- * ever been used on @ssp, this function always returns false.
+ * Only readers of the flavors which include a read-side smp_mb() are
+ * compatible with this proof; if the _fast() flavors have ever been
+ * used on @ssp, this function always returns false.
  */
 bool try_synchronize_srcu(struct srcu_struct *ssp)
 {
-	unsigned long unlocks0, unlocks1;
-	unsigned long rdm0, rdm1;
-
 	check_init_srcu_struct(ssp);
 
 	/*
-	 * Order the caller's prior stores before the counter reads below.
-	 * Pairs (store-buffering pattern) with the smp_mb() in
-	 * __srcu_read_lock(): any reader whose lock increment is not
-	 * observed by the sums below is guaranteed to observe, within its
+	 * Order the caller's prior stores before the counter reads in
+	 * the proof. Pairs (store-buffering pattern) with the smp_mb()
+	 * in __srcu_read_lock(): any reader whose lock increment is not
+	 * observed by the sums is guaranteed to observe, within its
 	 * critical section, every store the caller made before calling
 	 * this function.
 	 */
 	smp_mb();
 
-	unlocks0 = srcu_readers_unlock_idx(ssp, 0, &rdm0);
-	unlocks1 = srcu_readers_unlock_idx(ssp, 1, &rdm1);
-
-	/*
-	 * Reader flavors which elide the read-side smp_mb() that the
-	 * pairing above depends on cannot be proven absent this way;
-	 * they need a real grace period.
-	 */
-	if ((rdm0 | rdm1) & SRCU_READ_FLAVOR_SLOWGP)
+	if (srcu_readers_provably_absent(ssp, ~SRCU_READ_FLAVOR_SLOWGP) != 1)
 		return false;
 
-	/*
-	 * As in srcu_readers_active_idx_check(): ensure that a lock is
-	 * always counted if the corresponding unlock is counted, so that
-	 * a reader racing with these sums can only inflate the lock sum
-	 * and force the (safe) fallback. Summing both epochs means no
-	 * index flip is needed: a stable equality proves there was a
-	 * moment in this function at which no readers existed at all.
-	 */
+	/* Order the caller's subsequent accesses after the proof. */
 	smp_mb();
+	return true;
+}
+EXPORT_SYMBOL_GPL(try_synchronize_srcu);
 
-	if (!srcu_readers_lock_idx(ssp, 0, false, unlocks0))
-		return false;
-	if (!srcu_readers_lock_idx(ssp, 1, false, unlocks1))
-		return false;
+/**
+ * synchronize_srcu_atomic - spinning grace period for atomic-reader domains
+ * @ssp: srcu_struct with which to synchronize.
+ *
+ * Wait for all pre-existing readers of @ssp to complete, without
+ * sleeping and without involving the SRCU grace-period machinery —
+ * provided the domain keeps the promise that all of its readers use
+ * srcu_read_lock_atomic().
+ *
+ * Atomic-flavor readers run with preemption disabled, so every
+ * read-side critical section is bounded and running (not blocked) on
+ * some CPU, which is what makes spinning here sane: the wait is
+ * bounded by the longest such section. No index flip and no
+ * grace-period sequence update occur, so concurrent call_srcu(),
+ * synchronize_srcu() and srcu_barrier() are entirely unaffected.
+ *
+ * If the domain has ever had readers of any other flavor, the promise
+ * is broken — such a reader may be asleep, and spinning on it would be
+ * unbounded. That is a caller bug: complain, and fall back to a real
+ * (sleeping) grace period, which is the only correct wait for a
+ * possibly-sleeping reader. The reader-flavor mask is rechecked on
+ * every iteration so a first non-atomic reader appearing mid-spin
+ * takes the same path.
+ */
+void synchronize_srcu_atomic(struct srcu_struct *ssp)
+{
+	int ret;
+
+	check_init_srcu_struct(ssp);
+
+	/* As in try_synchronize_srcu(). */
+	smp_mb();
+
+	while ((ret = srcu_readers_provably_absent(ssp, SRCU_READ_FLAVOR_ATOMIC)) == 0)
+		cpu_relax();
+
+	if (WARN_ONCE(ret < 0,
+		      "non-atomic readers on srcu_struct at %ps; falling back to sleeping grace period\n",
+		      ssp)) {
+		synchronize_srcu_expedited(ssp);
+		return;
+	}
 
 	/* Order the caller's subsequent accesses after the proof. */
 	smp_mb();
-	return true;
 }
-EXPORT_SYMBOL_GPL(try_synchronize_srcu);
+EXPORT_SYMBOL_GPL(synchronize_srcu_atomic);
 
 /**
  * get_state_synchronize_srcu - Provide an end-of-grace-period cookie
-- 
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

[David Woodhouse]

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

On Wed, 2026-08-12 at 16:03 +0100, David Woodhouse wrote:
> On Wed, 2026-08-12 at 15:34 +0100, David Woodhouse wrote:
> > On Wed, 2026-08-12 at 15:05 +0100, David Woodhouse wrote:
> > > I'll rephrase that for my own understanding:
> > > 
> > > *If* we go all the way to building a whole SRCU flavour for this *and*
> > > implementing a spin-only variant of srcu_synchronize() which is
> > > tailored to the atomic-reader use case, *then* we don't need to remove
> > > the non_block_{start,end} guards around the MMU notifiers, which are
> > > basically never being called anyway and don't actually seem to protect
> > > against any real bugs.
> > > 
> > > Yes?
> > 
> > FWIW it looks something like this. I'll throw it into my torture and
> > latency tests, and we can see what Paul thinks of it. I'm still utterly
> > unconvinced it's needed, but I concede it has its good points.
> 
> This slightly refactored version is the one that's actually going into
> my torture tests...

Well, it survived first contact, and it's doing the soak testing now.

The average is basically no better than the try_synchronize_srcu()
case, unsurprisingly — as *both* of them just observe that there are no
readers and proceed immediately, in at least 99% of cases.

Like the existing rwlock case, it still manages double-digit p100
latency even when though *doesn't* actually sleep.

I don't *hate* it, but I do question the benefit of it over try-first.
Again, I'll defer to Paul, but personally I'd want to see a more
compelling use case for it.

  ┌───────────────┬─────────────────────┬───────────────────┬─────────────────────┐
  │               │      expedited      │     try-first     │       atomic        │
  ├───────────────┼─────────────────────┼───────────────────┼─────────────────────┤
  │ median drain  │ 32-128µs            │ 4-16µs            │ 4-16µs              │
  ├───────────────┼─────────────────────┼───────────────────┼─────────────────────┤
  │ avg           │ 118µs               │ 13.8µs            │ 12.3µs              │
  ├───────────────┼─────────────────────┼───────────────────┼─────────────────────┤
  │ >1ms          │ ~950ppm             │ ~990ppm           │ 838ppm              │
  ├───────────────┼─────────────────────┼───────────────────┼─────────────────────┤
  │ >8ms          │ 42ppm               │ 4.4ppm            │ 0.10ppm             │
  ├───────────────┼─────────────────────┼───────────────────┼─────────────────────┤
  │ max           │ 33.6ms              │ 17.6ms            │ 10.25ms             │
  ├───────────────┼─────────────────────┼───────────────────┼─────────────────────┤
  │ fallback rate │ —                   │ 1.2%              │ 0%                  │
  ├───────────────┼─────────────────────┼───────────────────┼─────────────────────┤
  │ sample        │ 32.6M drains, 10min │ 41M drains, 10min │ 40.5M drains, 10min │
  └───────────────┴─────────────────────┴───────────────────┴─────────────────────┘

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

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

[Paolo Bonzini]

> + * Atomic-flavor readers run with preemption disabled, so every
> + * read-side critical section is bounded and running (not blocked) on
> + * some CPU, which is what makes spinning here sane: the wait is
> + * bounded by the longest such section. No index flip and no
> + * grace-period sequence update occur, so concurrent call_srcu(),
> + * synchronize_srcu() and srcu_barrier() are entirely unaffected.

Making synchronize_srcu_atomic() spin in CONFIG_PREEMPT_RT is a
slightly tall request - it's basically putting it on the same level as
raw_spinlock. But if the MMU notifier guys really want to make OOM
notifiers atomic, this would be basically the only way to do so.

If instead it's okay to remove nonblock_start/end under
CONFIG_PREEMPT_RT, for RT kernels I'd rather avoid
preempt_disable/enable and only make synchronize_srcu_atomic() try to
skip the index flip:

   if (!try_synchronize_srcu(ssp))
      synchronize_srcu_expedited(ssp);

Paolo

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

[Sean Christopherson]

On Wed, Aug 12, 2026, David Woodhouse wrote:
> On Tue, 2026-08-11 at 15:58 -0700, Sean Christopherson wrote:
> > On Tue, Aug 11, 2026, David Woodhouse wrote:
> > > On Tue, 2026-08-11 at 22:21 +0200, Paolo Bonzini wrote:
> > > > 
> > > > 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);
> > > 
> > > 
> > > I'm literally sitting here watching you reinvent what SRCU already does.
> > 
> > The problem, for me at least, is that SRCU does this and a _lot_ more.
> > 
> > > Which, again, is *fine* given the behaviour of *these* read-side sections.
> > 
> > Well, yeah.  Why use a nailgun to put a nail into the wall to hang a picture
> > frame? (other than the indisputable fact that power tools are fun). 
> 
> It's not a nailgun, it's a drill. And in this analogy, my observation
> that it works absolutely fine for us with *these* SRCU read-side users
> ... is equivalent to finding the torque setting that lets you put it
> into screwdriver mode.

Yeah, but my concern is that there are KVM users out there that will put the
drill into hammer drill mode, likely unknowingly, and then complain that "we"
put a hole in their wall.

What I want is a strong guarantee that the user can't configure bad torque/mode
settings, even unintentionally.

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

[Jason Gunthorpe]

On Wed, Aug 12, 2026 at 06:04:47PM +0200, Paolo Bonzini wrote:
> > + * Atomic-flavor readers run with preemption disabled, so every
> > + * read-side critical section is bounded and running (not blocked) on
> > + * some CPU, which is what makes spinning here sane: the wait is
> > + * bounded by the longest such section. No index flip and no
> > + * grace-period sequence update occur, so concurrent call_srcu(),
> > + * synchronize_srcu() and srcu_barrier() are entirely unaffected.
> 
> Making synchronize_srcu_atomic() spin in CONFIG_PREEMPT_RT is a
> slightly tall request - it's basically putting it on the same level as
> raw_spinlock. But if the MMU notifier guys really want to make OOM
> notifiers atomic, this would be basically the only way to do so.

IMHO it is fine if PREEMPT_RT does something else, just that the
normal cases should follow the current arrangment where it is atomic

> If instead it's okay to remove nonblock_start/end under
> CONFIG_PREEMPT_RT, for RT kernels I'd rather avoid

This seems well agreed at least

Jason

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

[Sean Christopherson]

On Wed, Aug 12, 2026, David Woodhouse wrote:
> On Wed, 2026-08-12 at 16:03 +0100, David Woodhouse wrote:
> > On Wed, 2026-08-12 at 15:34 +0100, David Woodhouse wrote:
> > > On Wed, 2026-08-12 at 15:05 +0100, David Woodhouse wrote:
> > > > I'll rephrase that for my own understanding:
> > > > 
> > > > *If* we go all the way to building a whole SRCU flavour for this *and*
> > > > implementing a spin-only variant of srcu_synchronize() which is
> > > > tailored to the atomic-reader use case, *then* we don't need to remove
> > > > the non_block_{start,end} guards around the MMU notifiers, which are
> > > > basically never being called anyway and don't actually seem to protect
> > > > against any real bugs.
> > > > 
> > > > Yes?
> > > 
> > > FWIW it looks something like this. I'll throw it into my torture and
> > > latency tests, and we can see what Paul thinks of it. I'm still utterly
> > > unconvinced it's needed, but I concede it has its good points.
> > 
> > This slightly refactored version is the one that's actually going into
> > my torture tests...
> 
> Well, it survived first contact, and it's doing the soak testing now.
> 
> The average is basically no better than the try_synchronize_srcu()
> case, unsurprisingly — as *both* of them just observe that there are no
> readers and proceed immediately, in at least 99% of cases.
> 
> Like the existing rwlock case, it still manages double-digit p100
> latency even when though *doesn't* actually sleep.
> 
> I don't *hate* it, but I do question the benefit of it over try-first.

FWIW, the max latency and >8ms numbers are very appealing to me, as my concerns
with using SRCU are all about the tail latencies.

But I'm obviously not the one who'd be saddled with maintaining the code, so I'm
more than a little biased towards choosing the more complex version.

> Again, I'll defer to Paul, but personally I'd want to see a more
> compelling use case for it.
> 
>   ┌───────────────┬─────────────────────┬───────────────────┬─────────────────────┐
>   │               │      expedited      │     try-first     │       atomic        │
>   ├───────────────┼─────────────────────┼───────────────────┼─────────────────────┤
>   │ median drain  │ 32-128µs            │ 4-16µs            │ 4-16µs              │
>   ├───────────────┼─────────────────────┼───────────────────┼─────────────────────┤
>   │ avg           │ 118µs               │ 13.8µs            │ 12.3µs              │
>   ├───────────────┼─────────────────────┼───────────────────┼─────────────────────┤
>   │ >1ms          │ ~950ppm             │ ~990ppm           │ 838ppm              │
>   ├───────────────┼─────────────────────┼───────────────────┼─────────────────────┤
>   │ >8ms          │ 42ppm               │ 4.4ppm            │ 0.10ppm             │
>   ├───────────────┼─────────────────────┼───────────────────┼─────────────────────┤
>   │ max           │ 33.6ms              │ 17.6ms            │ 10.25ms             │
>   ├───────────────┼─────────────────────┼───────────────────┼─────────────────────┤
>   │ fallback rate │ —                   │ 1.2%              │ 0%                  │
>   ├───────────────┼─────────────────────┼───────────────────┼─────────────────────┤
>   │ sample        │ 32.6M drains, 10min │ 41M drains, 10min │ 40.5M drains, 10min │
>   └───────────────┴─────────────────────┴───────────────────┴─────────────────────┘

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

[David Woodhouse]

On 12 August 2026 17:20:51 BST, Sean Christopherson <seanjc@google.com> wrote:
>On Wed, Aug 12, 2026, David Woodhouse wrote:
>> On Wed, 2026-08-12 at 16:03 +0100, David Woodhouse wrote:
>> > On Wed, 2026-08-12 at 15:34 +0100, David Woodhouse wrote:
>> > > On Wed, 2026-08-12 at 15:05 +0100, David Woodhouse wrote:
>> > > > I'll rephrase that for my own understanding:
>> > > > 
>> > > > *If* we go all the way to building a whole SRCU flavour for this *and*
>> > > > implementing a spin-only variant of srcu_synchronize() which is
>> > > > tailored to the atomic-reader use case, *then* we don't need to remove
>> > > > the non_block_{start,end} guards around the MMU notifiers, which are
>> > > > basically never being called anyway and don't actually seem to protect
>> > > > against any real bugs.
>> > > > 
>> > > > Yes?
>> > > 
>> > > FWIW it looks something like this. I'll throw it into my torture and
>> > > latency tests, and we can see what Paul thinks of it. I'm still utterly
>> > > unconvinced it's needed, but I concede it has its good points.
>> > 
>> > This slightly refactored version is the one that's actually going into
>> > my torture tests...
>> 
>> Well, it survived first contact, and it's doing the soak testing now.
>> 
>> The average is basically no better than the try_synchronize_srcu()
>> case, unsurprisingly — as *both* of them just observe that there are no
>> readers and proceed immediately, in at least 99% of cases.
>> 
>> Like the existing rwlock case, it still manages double-digit p100
>> latency even when though *doesn't* actually sleep.
>> 
>> I don't *hate* it, but I do question the benefit of it over try-first.
>
>FWIW, the max latency and >8ms numbers are very appealing to me, as my concerns
>with using SRCU are all about the tail latencies.
>
>But I'm obviously not the one who'd be saddled with maintaining the code, so I'm
>more than a little biased towards choosing the more complex version.

Ack. Well, it isn't even *that* bad in terms of complexity. It's mostly just a while loop around the core of the try_synchronize_srcu() I posted before, and some syntactic sugar and safeguards.

Let's see what Paul thinks.



>> Again, I'll defer to Paul, but personally I'd want to see a more
>> compelling use case for it.
>> 
>>   ┌───────────────┬─────────────────────┬───────────────────┬─────────────────────┐
>>   │               │      expedited      │     try-first     │       atomic        │
>>   ├───────────────┼─────────────────────┼───────────────────┼─────────────────────┤
>>   │ median drain  │ 32-128µs            │ 4-16µs            │ 4-16µs              │
>>   ├───────────────┼─────────────────────┼───────────────────┼─────────────────────┤
>>   │ avg           │ 118µs               │ 13.8µs            │ 12.3µs              │
>>   ├───────────────┼─────────────────────┼───────────────────┼─────────────────────┤
>>   │ >1ms          │ ~950ppm             │ ~990ppm           │ 838ppm              │
>>   ├───────────────┼─────────────────────┼───────────────────┼─────────────────────┤
>>   │ >8ms          │ 42ppm               │ 4.4ppm            │ 0.10ppm             │
>>   ├───────────────┼─────────────────────┼───────────────────┼─────────────────────┤
>>   │ max           │ 33.6ms              │ 17.6ms            │ 10.25ms             │
>>   ├───────────────┼─────────────────────┼───────────────────┼─────────────────────┤
>>   │ fallback rate │ —                   │ 1.2%              │ 0%                  │
>>   ├───────────────┼─────────────────────┼───────────────────┼─────────────────────┤
>>   │ sample        │ 32.6M drains, 10min │ 41M drains, 10min │ 40.5M drains, 10min │
>>   └───────────────┴─────────────────────┴───────────────────┴─────────────────────┘