From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Xu Subject: Re: [PATCH v2 0/3] KVM: x86: use pic/ioapic destructor when proper Date: Fri, 17 Mar 2017 14:05:11 +0800 Message-ID: <20170317060511.GF17405@pxdev.xzpeter.org> References: <1489564879-6097-1-git-send-email-peterx@redhat.com> <20170315085936.GD5971@pxdev.xzpeter.org> <2cb28e66-b24a-efba-92d1-55ffd39743c6@redhat.com> <20170316060412.GK5971@pxdev.xzpeter.org> <20170316193214.GB16668@potion> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: David Hildenbrand , kvm@vger.kernel.org, Paolo Bonzini To: Radim =?utf-8?B?S3LEjW3DocWZ?= Return-path: Received: from mx1.redhat.com ([209.132.183.28]:35436 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750885AbdCQGMQ (ORCPT ); Fri, 17 Mar 2017 02:12:16 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 969D3C051668 for ; Fri, 17 Mar 2017 06:05:19 +0000 (UTC) Content-Disposition: inline In-Reply-To: <20170316193214.GB16668@potion> Sender: kvm-owner@vger.kernel.org List-ID: On Thu, Mar 16, 2017 at 08:32:14PM +0100, Radim Krčmář wrote: > 2017-03-16 14:04+0800, Peter Xu: > > On Wed, Mar 15, 2017 at 12:15:24PM +0100, David Hildenbrand wrote: > >> Am 15.03.2017 um 09:59 schrieb Peter Xu: > >> > On Wed, Mar 15, 2017 at 09:47:14AM +0100, David Hildenbrand wrote: > >> >> Am 15.03.2017 um 09:01 schrieb Peter Xu: > >> >>> v2: > >> >>> - add one more patch (patch 2 in v2) to check pic/ioapic's > >> >>> existance before destroying them > >> >> > >> >> Patch 2+3 without 1 should work, too, right? > >> > > >> > We may need that? When destroy VM, it's: > >> > > >> > kvm_destroy_vm() > >> > foreach (bus) do kvm_io_bus_destroy() > >> > kfree(bus) <--- [1] > >> > kvm_arch_destroy_vm() > >> > kvm_pic_destroy() > >> > kvm_io_bus_unregister_dev() <--- [2] > >> > > >> > >> Wouldn't the natural way be > >> > >> 1. kvm_arch_destroy_vm() > >> 2. foreach (bus) do kvm_io_bus_destroy() > >> kfree(bus) > >> > >> Then we wouldn't have to deal with suddenly removed buses. But maybe > >> that order is of importance here ... > > > > I'll leave this question for Paolo/Radim, or anyone knows this better > > than me. :-) > > I'd like if kvm_destroy_vm() looked like kvm_create_vm() in reverse and > the current order is closer to that. > > I don't think there is dependency between those two and we can easily > reverse them in kvm_create_vm(), so go ahead if you get better code out > of it in kvm_arch_destroy_vm(). :) Thanks. :) IMHO in all cases we can consider having the first two patches, which should be something nice to have. And if so, I'll prefer patch 3 comparing to reordering of VM init/destroy to avoid any possiblilty of breakage, until one day we really need to touch them (I guess this cleanup series is not a good enough reason :). Thanks, -- peterx