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
