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

[David Woodhouse]

From: David Woodhouse <dwmw@xxxxxxxxxxxx>

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@xxxxxxxxxxxxx/
Signed-off-by: David Woodhouse <dwmw@xxxxxxxxxxxx>
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@xxxxxxxxxxxxx/

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: smime.p7s
Description: S/MIME cryptographic signature