Messages in this thread | | | Subject | Re: [PATCH v4 06/13] KVM: Move WARN on invalid memslot index to update_memslots() | From | David Hildenbrand <> | Date | Fri, 20 Aug 2021 11:27:42 +0200 |
| |
On 18.08.21 23:43, Maciej S. Szmigiero wrote: > On 18.08.2021 16:35, David Hildenbrand wrote: >> On 13.08.21 21:33, Maciej S. Szmigiero wrote: >>> From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com> >>> >>> Since kvm_memslot_move_forward() can theoretically return a negative >>> memslot index even when kvm_memslot_move_backward() returned a positive one >>> (and so did not WARN) let's just move the warning to the common code. >>> >>> Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com> >>> --- >>> virt/kvm/kvm_main.c | 6 ++++-- >>> 1 file changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c >>> index 03ef42d2e421..7000efff1425 100644 >>> --- a/virt/kvm/kvm_main.c >>> +++ b/virt/kvm/kvm_main.c >>> @@ -1293,8 +1293,7 @@ static inline int kvm_memslot_move_backward(struct kvm_memslots *slots, >>> struct kvm_memory_slot *mslots = slots->memslots; >>> int i; >>> - if (WARN_ON_ONCE(slots->id_to_index[memslot->id] == -1) || >>> - WARN_ON_ONCE(!slots->used_slots)) >>> + if (slots->id_to_index[memslot->id] == -1 || !slots->used_slots) >>> return -1; >>> /* >>> @@ -1398,6 +1397,9 @@ static void update_memslots(struct kvm_memslots *slots, >>> i = kvm_memslot_move_backward(slots, memslot); >>> i = kvm_memslot_move_forward(slots, memslot, i); >>> + if (WARN_ON_ONCE(i < 0)) >>> + return; >>> + >>> /* >>> * Copy the memslot to its new position in memslots and update >>> * its index accordingly. >>> >> >> >> Note that WARN_ON_* is frowned upon, because it can result in crashes with panic_on_warn enabled, which is what some distributions do enable. >> >> We tend to work around that by using pr_warn()/pr_warn_once(), avoiding eventually crashing the system when there is a way to continue. >> > > This patch uses WARN_ON_ONCE because: > 1) It was used in the old code and the patch merely moves the check > from kvm_memslot_move_backward() to its caller, > > 2) This chunk of code is wholly replaced by patch 11 from this series > anyway ("Keep memslots in tree-based structures instead of array-based ones").
Okay, that makes sense then, thanks!
-- Thanks,
David / dhildenb
| |