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=-5.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham 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 05343CA9EA0 for ; Mon, 4 Nov 2019 12:21:58 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id D339621E6F for ; Mon, 4 Nov 2019 12:21:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728619AbfKDMV4 (ORCPT ); Mon, 4 Nov 2019 07:21:56 -0500 Received: from mx1.redhat.com ([209.132.183.28]:56156 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726100AbfKDMV4 (ORCPT ); Mon, 4 Nov 2019 07:21:56 -0500 Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id BD83D3DD47 for ; Mon, 4 Nov 2019 12:21:55 +0000 (UTC) Received: by mail-wm1-f70.google.com with SMTP id z5so6073259wma.5 for ; Mon, 04 Nov 2019 04:21:55 -0800 (PST) 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:openpgp:message-id :date:user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=SnAmxt67QITd4idD27zmFwQoIn//FDiJRgfk5w70PuY=; b=diGRn4xFazAxJb6Fvtq8N6qN7F9+qYpjRUYF32eSU7f4uTvHLYYLjtqAC8iyxN6s1x 8a7O7nVgc9kG/zQWFyAYRMzdl9eaA0x02eyPqUzhN7u8YqZTydlOrrkqt0eYc5warXTB Sqj7FtM5P/ytZeBE6h50QIFpNo6WLE1fKP1bikMh9hSWOZdlkgo3OGx+u0e4ifQQXGLu uBdJOGMRBBRADNlL9Sozc06bcCN0V39+3jdnIrbEREjR+ayhTtklrc7xlRLGP2pqzTY6 as7o9zahLxQ158OtNiXYz0eeU04yEe18KiOjk5VN1DVo4DRKa0rZxQcGDggH4Rk8RRAZ IFlg== X-Gm-Message-State: APjAAAUUDTwfsLJC5PeBzFrY/Cgz9Vo9bTl+3oiDfeaObZC3BRNBk0Px xmhqABQNhdoKWqKTqdDwWv8oNlz3whgs7U9l4m8Qv3Ps8RFBGCfyoPkMXR+6qsK6pn4HacNymov Rm+wRe1jAjcPndyIxpnPz6xtI X-Received: by 2002:a5d:4612:: with SMTP id t18mr21810270wrq.255.1572870114414; Mon, 04 Nov 2019 04:21:54 -0800 (PST) X-Google-Smtp-Source: APXvYqxPxprR782mRFbAmu9nXhQC8TRKYPA9z8jPOZPgx02pyqcnrKyzrWeITE3wZWZoKj7oiPfd5w== X-Received: by 2002:a5d:4612:: with SMTP id t18mr21810251wrq.255.1572870114117; Mon, 04 Nov 2019 04:21:54 -0800 (PST) Received: from ?IPv6:2001:b07:6468:f312:4051:461:136e:3f74? ([2001:b07:6468:f312:4051:461:136e:3f74]) by smtp.gmail.com with ESMTPSA id i13sm9382368wrp.12.2019.11.04.04.21.52 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 04 Nov 2019 04:21:53 -0800 (PST) Subject: Re: [PATCH 2/2] KVM: Fix rcu splat if vm creation fails To: Wanpeng Li Cc: LKML , kvm , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Sean Christopherson , Vitaly Kuznetsov , Wanpeng Li , Jim Mattson , Joerg Roedel References: <1572848879-21011-1-git-send-email-wanpengli@tencent.com> <1572848879-21011-2-git-send-email-wanpengli@tencent.com> From: Paolo Bonzini Openpgp: preference=signencrypt Message-ID: Date: Mon, 4 Nov 2019 13:21:52 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 04/11/19 13:16, Wanpeng Li wrote: >> I don't understand this one, hasn't >> >> WARN_ON_ONCE(!refcount_dec_and_test(&kvm->users_count)); >> >> decreased the conut already? With your patch the refcount would then >> underflow. > > r = kvm_arch_init_vm(kvm, type); > if (r) > goto out_err_no_arch_destroy_vm; > > out_err_no_disable: > kvm_arch_destroy_vm(kvm); > WARN_ON_ONCE(!refcount_dec_and_test(&kvm->users_count)); > out_err_no_arch_destroy_vm: > > So, if kvm_arch_init_vm() fails, we will not execute > WARN_ON_ONCE(!refcount_dec_and_test(&kvm->users_count)); Uuh of course. But I'd rather do the opposite: move the refcount_set earlier so that refcount_dec_and_test can be moved after no_arch_destroy_vm. Moving the refcount_set is not strictly necessary, but avoids the introduction of yet another label. diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index e22ff63e5b1a..e7a07132cd7f 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -650,6 +650,7 @@ static struct kvm *kvm_create_vm(unsigned long type) if (init_srcu_struct(&kvm->irq_srcu)) goto out_err_no_irq_srcu; + refcount_set(&kvm->users_count, 1); for (i = 0; i < KVM_ADDRESS_SPACE_NUM; i++) { struct kvm_memslots *slots = kvm_alloc_memslots(); @@ -667,7 +668,6 @@ static struct kvm *kvm_create_vm(unsigned long type) goto out_err_no_arch_destroy_vm; } - refcount_set(&kvm->users_count, 1); r = kvm_arch_init_vm(kvm, type); if (r) goto out_err_no_arch_destroy_vm; @@ -696,8 +696,8 @@ static struct kvm *kvm_create_vm(unsigned long type) hardware_disable_all(); out_err_no_disable: kvm_arch_destroy_vm(kvm); - WARN_ON_ONCE(!refcount_dec_and_test(&kvm->users_count)); out_err_no_arch_destroy_vm: + WARN_ON_ONCE(!refcount_dec_and_test(&kvm->users_count)); for (i = 0; i < KVM_NR_BUSES; i++) kfree(kvm_get_bus(kvm, i)); for (i = 0; i < KVM_ADDRESS_SPACE_NUM; i++)