Re: [PATCH v2] mm/mmu_notifiers: Esnure range_end() is paired with range_start()

[Jason Gunthorpe]

On Thu, Mar 11, 2021 at 10:00:57AM -0800, Sean Christopherson wrote:
> If one or more notifiers fails .invalidate_range_start(), invoke
> .invalidate_range_end() for "all" notifiers.  If there are multiple
> notifiers, those that did not fail are expecting _start() and _end() to
> be paired, e.g. KVM's mmu_notifier_count would become imbalanced.
> Disallow notifiers that can fail _start() from implementing _end() so
> that it's unnecessary to either track which notifiers rejected _start(),
> or had already succeeded prior to a failed _start().
> 
> Note, the existing behavior of calling _start() on all notifiers even
> after a previous notifier failed _start() was an unintented "feature".
> Make it canon now that the behavior is depended on for correctness.
> 
> As of today, the bug is likely benign:
> 
>   1. The only caller of the non-blocking notifier is OOM kill.
>   2. The only notifiers that can fail _start() are the i915 and Nouveau
>      drivers.
>   3. The only notifiers that utilize _end() are the SGI UV GRU driver
>      and KVM.
>   4. The GRU driver will never coincide with the i195/Nouveau drivers.
>   5. An imbalanced kvm->mmu_notifier_count only causes soft lockup in the
>      _guest_, and the guest is already doomed due to being an OOM victim.
> 
> Fix the bug now to play nice with future usage, e.g. KVM has a potential
> use case for blocking memslot updates in KVM while an invalidation is
> in-progress, and failure to unblock would result in said updates being
> blocked indefinitely and hanging.
> 
> Found by inspection.  Verified by adding a second notifier in KVM that
> periodically returns -EAGAIN on non-blockable ranges, triggering OOM,
> and observing that KVM exits with an elevated notifier count.
> 
> Fixes: 93065ac753e4 ("mm, oom: distinguish blockable mode for mmu notifiers")
> Suggested-by: Jason Gunthorpe <jgg@xxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Cc: David Rientjes <rientjes@xxxxxxxxxx>
> Cc: Ben Gardon <bgardon@xxxxxxxxxx>
> Cc: Michal Hocko <mhocko@xxxxxxxx>
> Cc: "Jérôme Glisse" <jglisse@xxxxxxxxxx>
> Cc: Andrea Arcangeli <aarcange@xxxxxxxxxx>
> Cc: Johannes Weiner <hannes@xxxxxxxxxxx>
> Cc: Dimitri Sivanich <dimitri.sivanich@xxxxxxx>
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> 
> v2: Reimplemented as suggested by Jason.  Only functional change relative
>     to Jason's suggestion is to check invalidate_range_end before calling to
>     avoid a NULL pointer dereference.  I also added more comments, hopefully
>     they're helpful...
> 
> v1: https://lkml.kernel.org/r/20210310213117.1444147-1-seanjc@xxxxxxxxxx

Looks fine, thanks. I think you need some commit message remark to
discourage backporting, the added WARN_ON will trigger on older
kernels that have many more things implementing
invalidate_range_end(). It should not be backported to anything that
has more invalidate_range_ends()'s than today's kernel.

Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxx>

Jason