From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-19.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 76871C4320A for ; Fri, 20 Aug 2021 09:27:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5856060F45 for ; Fri, 20 Aug 2021 09:27:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236768AbhHTJ20 (ORCPT ); Fri, 20 Aug 2021 05:28:26 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:43355 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235063AbhHTJ2Y (ORCPT ); Fri, 20 Aug 2021 05:28:24 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1629451666; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=/u+8okw9NC3TUEzgwTlSaEbJlbpO/sr31TcVmCIG3eM=; b=DBHx2SlI27WlW9ojjfoOSM6wjAYkCL+Tu/so9T7tu/IqAXpYn4N+t4HojW6N+vEYNR9n0Z KdCda0WKDC1V6DmU5qJxGEKafDnbIMs/KzXpsdU2WNQjfASwKrfWidtbr8ZMhPP94AqdAo Pm3GCT5FC7HISpupfkcxi46BQtJ80Io= Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-424-zWsxf_OmMEKnPMtuUqaOmQ-1; Fri, 20 Aug 2021 05:27:45 -0400 X-MC-Unique: zWsxf_OmMEKnPMtuUqaOmQ-1 Received: by mail-wr1-f72.google.com with SMTP id n10-20020a5d660a0000b02901551ef5616eso2659057wru.20 for ; Fri, 20 Aug 2021 02:27:45 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:organization :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=/u+8okw9NC3TUEzgwTlSaEbJlbpO/sr31TcVmCIG3eM=; b=ATKOZAGtvyfE4b895OBw2No7YRWNVfzT6qecuGRJVu3GiKbrKeRyP81lK2sOnPSgEA AQP0i5v3qMesJmdXNvImrzq5hJu5G1FFevH9U5JaVRdXepzyV/U+RFXdQPE5mcdkOiuw 9xxHrNjnHL+knpzLcFH21WHjgvEDk0G1lyn7PTJ+lRA0EREFEo3uokjmRC4GtWPPawhr 1XqEHwDUICc7TucPekazdBANssQk/F9/WITtfJtMJ0RgMY3sCtbr0yAq2uNBvAN/Pgjm VJUcawnZdG3TWNnJMrrptT6UpUPTsief7EpibdpMRhxs4VNvD2B2Cr4zFswx1Z8UXse+ toaQ== X-Gm-Message-State: AOAM531Wwb3E16e5/bFrGeSdlMqyhCRGNudSwxAZ38r7VPraH3neRRaZ QcWF1d5jaL/UumNeWbRQDItA4Y/Lh2QsjcC3pOYDadxgJ8UnlXLNp3pu/4ZsaSp/0eznNvpqcMc 4SZMW6slWuarth1Xf/Zd5nLlK X-Received: by 2002:a7b:c7c3:: with SMTP id z3mr2877136wmk.96.1629451664226; Fri, 20 Aug 2021 02:27:44 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwrv/MO6I4VJiNFhu/exeX4GinBGO52T7kneA302ioQqlyE7/IysNBUL0jrYV0CaHBwzkxm4w== X-Received: by 2002:a7b:c7c3:: with SMTP id z3mr2877101wmk.96.1629451663998; Fri, 20 Aug 2021 02:27:43 -0700 (PDT) Received: from ?IPv6:2003:d8:2f0a:7f00:fad7:3bc9:69d:31f? (p200300d82f0a7f00fad73bc9069d031f.dip0.t-ipconnect.de. [2003:d8:2f0a:7f00:fad7:3bc9:69d:31f]) by smtp.gmail.com with ESMTPSA id c7sm4478795wmq.13.2021.08.20.02.27.42 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 20 Aug 2021 02:27:43 -0700 (PDT) Subject: Re: [PATCH v4 06/13] KVM: Move WARN on invalid memslot index to update_memslots() To: "Maciej S. Szmigiero" Cc: Sean Christopherson , Wanpeng Li , Jim Mattson , Igor Mammedov , Marc Zyngier , James Morse , Julien Thierry , Suzuki K Poulose , Huacai Chen , Aleksandar Markovic , Paul Mackerras , Christian Borntraeger , Janosch Frank , Cornelia Huck , Claudio Imbrenda , Joerg Roedel , kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Paolo Bonzini , Vitaly Kuznetsov References: <8db0f1d1901768b5de1417caa425e62d1118e5e8.1628871413.git.maciej.szmigiero@oracle.com> <957c6b3d-9621-a5a5-418c-f61f87a32ee0@redhat.com> From: David Hildenbrand Organization: Red Hat Message-ID: <633bf50b-5de4-1e76-736c-067d10bf92b3@redhat.com> Date: Fri, 20 Aug 2021 11:27:42 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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" >>> >>> 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 >>> --- >>>   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