Re: [PATCH v3] mm/mmu_notifier: prevent unpaired invalidate_start and invalidate_end

[Jason Gunthorpe <jgg@mellanox.com>]

On Thu, Mar 26, 2020 at 09:06:12AM -0400, Qian Cai wrote:
> 
> 
> > On Feb 11, 2020, at 3:52 PM, Jason Gunthorpe <jgg@mellanox.com> wrote:
> > 
> > Many users of the mmu_notifier invalidate_range callbacks maintain
> > locking/counters/etc on a paired basis and have long expected that
> > invalidate_range_start/end() are always paired.
> > 
> > For instance kvm_mmu_notifier_invalidate_range_end() undoes
> > kvm->mmu_notifier_count which was incremented during start().
> > 
> > The recent change to add non-blocking notifiers breaks this assumption
> > when multiple notifiers are present in the list. When EAGAIN is returned
> > from an invalidate_range_start() then no invalidate_range_ends() are
> > called, even if the subscription's start had previously been called.
> > 
> > Unfortunately, due to the RCU list traversal we can't reliably generate a
> > subset of the linked list representing the notifiers already called to
> > generate an invalidate_range_end() pairing.
> > 
> > One case works correctly, if only one subscription requires
> > invalidate_range_end() and it is the last entry in the hlist. In this
> > case, when invalidate_range_start() returns -EAGAIN there will be nothing
> > to unwind.
> > 
> > Keep the notifier hlist sorted so that notifiers that require
> > invalidate_range_end() are always last, and if two are added then disable
> > non-blocking invalidation for the mm.
> > 
> > A warning is printed for this case, if in future we determine this never
> > happens then we can simply fail during registration when there are
> > unsupported combinations of notifiers.
> 
> This will generate a warning when running a simple qemu-kvm on arm64,
> 
> qemu-kvm (37712) created two mmu_notifier's with
> invalidate_range_end(): kvm_mmu_notifier_invalidate_range_end and
> kvm_mmu_notifier_invalidate_range_end, non-blocking notifiers
> disabled

Thank you Qian, this is very valuable information

It seems this solution will not work if kvm is registering multiple
notifiers.

I will take the patch out of linux-next

Jason