From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Subject: Re: [PATCH v4 1/6] kvm: add device control API Date: Thu, 25 Apr 2013 11:51:08 -0500 Message-ID: <1366908668.30341.1@snotra> References: <1364954273-18196-1-git-send-email-scottwood@freescale.com> <1365811727-24431-1-git-send-email-scottwood@freescale.com> <1365811727-24431-2-git-send-email-scottwood@freescale.com> <20130425094324.GY12401@redhat.com> <5A22B05F-C1C5-439F-8AC6-D16137A9D086@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; delsp=Yes; format=Flowed Content-Transfer-Encoding: 8BIT Cc: Gleb Natapov , , , To: Alexander Graf Return-path: In-Reply-To: <5A22B05F-C1C5-439F-8AC6-D16137A9D086@suse.de> (from agraf@suse.de on Thu Apr 25 05:47:39 2013) Content-Disposition: inline Sender: kvm-ppc-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On 04/25/2013 05:47:39 AM, Alexander Graf wrote: > > On 25.04.2013, at 11:43, Gleb Natapov wrote: > > >> +void kvm_device_put(struct kvm_device *dev) > >> +{ > >> + if (atomic_dec_and_test(&dev->users)) > >> + dev->ops->destroy(dev); > >> +} > >> + > >> +static int kvm_device_release(struct inode *inode, struct file > *filp) > >> +{ > >> + struct kvm_device *dev = filp->private_data; > >> + struct kvm *kvm = dev->kvm; > >> + > >> + kvm_device_put(dev); > >> + kvm_put_kvm(kvm); > > We may put kvm only if users goes to zero, otherwise kvm can be > > freed while something holds a reference to a device. Why not make > > kvm_device_put() do it? > > Nice catch. I'll change the patch so it does the kvm_put_kvm inside > kvm_device_put's destroy branch. No, please don't. The KVM reference being "put" here is associated with the file descriptor, not with the MPIC object. If you make that change I think you'll have circular references and thus a memory leak, because the vcpus can hold a reference to the MPIC object. -Scott