From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:38137)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <guangrong.xiao@linux.intel.com>) id 1ZmXft-0002bD-Kc
	for qemu-devel@nongnu.org; Wed, 14 Oct 2015 21:49:46 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <guangrong.xiao@linux.intel.com>) id 1ZmXfq-0002oh-FE
	for qemu-devel@nongnu.org; Wed, 14 Oct 2015 21:49:45 -0400
Received: from mga03.intel.com ([134.134.136.65]:38408)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <guangrong.xiao@linux.intel.com>) id 1ZmXfq-0002ob-96
	for qemu-devel@nongnu.org; Wed, 14 Oct 2015 21:49:42 -0400
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>
	<20151014170655.GW1260@thinpad.lan.raisama.net>
From: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Message-ID: <561F04BD.2050303@linux.intel.com>
Date: Thu, 15 Oct 2015 09:43:25 +0800
MIME-Version: 1.0
In-Reply-To: <20151014170655.GW1260@thinpad.lan.raisama.net>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v3 27/32] nvdimm: support
	DSM_CMD_IMPLEMENTED function
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Eduardo Habkost <ehabkost@redhat.com>
Cc: kvm@vger.kernel.org, mst@redhat.com, gleb@kernel.org, Stefan Hajnoczi <stefanha@gmail.com>, mtosatti@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, pbonzini@redhat.com, imammedo@redhat.com, dan.j.williams@intel.com, rth@twiddle.net



On 10/15/2015 01:06 AM, Eduardo Habkost wrote:
> 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().
>

Ah... I got it, thank you for pointing it out.