Re: [PATCH] mm/mmu_notifier: Remove non_block_start/end() from notifier invocation
David Woodhouse <[email protected]>
| Newsgroups | org.kernel.vger.kvm,dev.linux.lists.linux-rt-devel,org.kernel.vger.linux-kernel,org.kvack.linux-mm |
|---|---|
| Message-ID | <[email protected]> |
On Wed, 2026-08-20 at 15:30 +0200, Sebastian Andrzej Siewior wrote: > So if it is *just* the annoying splat on RT, I could try to teach > might_sleep() no to be upset here. It *wasn't* just the splat on RT... but I think it can be reduced to that. My full set of reasons were: • RT turns rwlock_t/spinlock_t into sleeping locks. • We believed a waiting KVM_REQ (IPI + wait for acks) would sleep; it actually spins with preemption disabled¹. • synchronize_srcu_expedited() in the invalidation path really does sleep on a workqueue completion, even on !RT. If you deal with the first, the second turns out not to have been needed, and that just leaves the third which is the SRCU atomic thing (qv). Aside from those selfish technical reasons, the check is actually *wrong* because "must not sleep" isn't the real requirement here, as discussed. And when I went looking, not only did I have to hack the kernel to even make it trigger at all, I also couldn't find any example of it finding anything that *wasn't* a false positive. The only concrete thing it *did* achieve was making us strip a necessary KVM_REQUEST_WAIT to appease it, which opened up a use-after-free. So if you fix the might_sleep() and Paul comes through with the atomic SRCU thing, I no longer care *much*, but still think the whole thing deserves to die. Going back to SRCU atomic (citing your mail in reverse order): > In > https://lore.kernel.org/all/[email protected] > there was a suggestion for "srcu: Add an ATOMIC reader flavor and a spinning > synchronize_srcu_atomic()". This one had preempt_disable() in > srcu_read_lock_atomic(). This is bad as not only forbids to acquire any > sleeping locks (such as spinlock_t) but it also does not allow any > scheduling of any kind within this section. That was kind of the point :) My srcu_read_lock_atomic/synchronize_srcu_atomic() were born of the "need" to avoid scheduling in OOM (RT or no), but actually I think they survive even when that requirement goes away. Even with my try_synchronize_srcu() optimisation, we saw higher tail latencies when deferring to the workqueue, and Sean was very much in favour of eliminating those. So I think we do want synchronize_srcu_atomic() if Paul can find a way to give us that. To your point about preempt_disable() in srcu_read_lock_atomic() on the read side... I don't know if we *need* it to be like that. When I threw it together, I was thinking of it as a raw_spinlock, in the "must not sleep" mindset. In fact, as long as the splats are dealt with, it would probably be OK for it to be equivalent to a non-raw spinlock: disable preemption on non-RT, but not on RT. I think that's perfectly OK for us from the KVM point of view. But does heavily depend on how Paul wants to implement it, of course. ¹ I wonder if we might want to change that on RT?
smime.p7s
(application/pkcs7-signature, 6 KB) - not displayed