From mboxrd@z Thu Jan  1 00:00:00 1970
From: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Subject: Re: [PATCH v3 27/32] nvdimm: support DSM_CMD_IMPLEMENTED function
Date: Thu, 15 Oct 2015 09:43:25 +0800
Message-ID: <561F04BD.2050303@linux.intel.com>
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>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
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: Eduardo Habkost <ehabkost@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mga02.intel.com ([134.134.136.20]:25319 "EHLO mga02.intel.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1753430AbbJOBtj (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 14 Oct 2015 21:49:39 -0400
In-Reply-To: <20151014170655.GW1260@thinpad.lan.raisama.net>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>



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.