From mboxrd@z Thu Jan  1 00:00:00 1970
From: Eduardo Habkost <ehabkost@redhat.com>
Subject: Re: [PATCH v3 27/32] nvdimm: support DSM_CMD_IMPLEMENTED function
Date: Wed, 14 Oct 2015 14:06:55 -0300
Message-ID: <20151014170655.GW1260@thinpad.lan.raisama.net>
References: <1444535584-18220-1-git-send-email-guangrong.xiao@linux.intel.com>
 <1444535584-18220-28-git-send-email-guangrong.xiao@linux.intel.com>
 <20151014094012.GB14874@stefanha-thinkpad>
 <561E6BC0.4060706@linux.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Stefan Hajnoczi <stefanha@gmail.com>, pbonzini@redhat.com,
	imammedo@redhat.com, gleb@kernel.org, mtosatti@redhat.com,
	stefanha@redhat.com, mst@redhat.com, rth@twiddle.net,
	dan.j.williams@intel.com, kvm@vger.kernel.org,
	qemu-devel@nongnu.org
To: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:48961 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751889AbbJNRG7 (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 14 Oct 2015 13:06:59 -0400
Content-Disposition: inline
In-Reply-To: <561E6BC0.4060706@linux.intel.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Wed, Oct 14, 2015 at 10:50:40PM +0800, Xiao Guangrong wrote:
> On 10/14/2015 05:40 PM, Stefan Hajnoczi wrote:
> >On Sun, Oct 11, 2015 at 11:52:59AM +0800, Xiao Guangrong wrote:
> >>  static void dsm_write(void *opaque, hwaddr addr,
> >>                        uint64_t val, unsigned size)
> >>  {
> >>+    NVDIMMState *state = opaque;
> >>+    MemoryRegion *dsm_ram_mr;
> >>+    dsm_in *in;
> >>+    dsm_out *out;
> >>+    uint32_t revision, function, handle;
> >>+
> >>      if (val != NOTIFY_VALUE) {
> >>          fprintf(stderr, "BUG: unexepected notify value 0x%" PRIx64, val);
> >>      }
> >>+
> >>+    dsm_ram_mr = memory_region_find(&state->mr, state->page_size,
> >>+                                    state->page_size).mr;
> >>+    memory_region_unref(dsm_ram_mr);
> >>+    in = memory_region_get_ram_ptr(dsm_ram_mr);
> >
> >This looks suspicious.  Shouldn't the memory_region_unref(dsm_ram_mr)
> >happen after we're done using it?
> 
> This region is keep-alive during QEMU's running, it is okay.  The same
> style is applied to other codes, for example: line 208 in
> hw/s390x/sclp.c.

In sclp.c (assign_storage()), the memory region is never used after
memory_region_unref() is called. In unassign_storage(), sclp.c owns an
additional reference, grabbed by assign_storage().

-- 
Eduardo