Re: [Qemu-devel] [RFC v1 3/3] make address_space_map safe

[liu ping fan]

On Wed, Feb 13, 2013 at 8:12 PM, Vasilis Liaskovitis
<address@hidden> wrote:
> Hi,
>
> I am looking at this old ref/unref patchset for safely removing hot-plugged
> dimms/MemoryRegions. I am not sure if the set is still actively worked on or
> relevant for qemu-master, but I had a small comment below:
>
> On Fri, Nov 09, 2012 at 11:14:30AM +0800, Liu Ping Fan wrote:
>> From: Liu Ping Fan <address@hidden>
>>
>> Signed-off-by: Liu Ping Fan <address@hidden>
>> ---
>>  cpu-common.h      |    8 ++++++--
>>  cputlb.c          |    4 ++--
>>  dma-helpers.c     |    4 +++-
>>  dma.h             |    5 ++++-
>>  exec.c            |   45 +++++++++++++++++++++++++++++++++++++--------
>>  memory.h          |    4 +++-
>>  target-i386/kvm.c |    4 ++--
>>  7 files changed, 57 insertions(+), 17 deletions(-)
>>
> [snip]
>> diff --git a/exec.c b/exec.c
>> index e5f1c0f..e9bd695 100644
>> --- a/exec.c
>> +++ b/exec.c
> [snip]
>> @@ -3822,7 +3837,8 @@ void address_space_unmap(AddressSpace *as, void 
>> *buffer, target_phys_addr_t len,
>>  {
>>      if (buffer != bounce.buffer) {
>>          if (is_write) {
>> -            ram_addr_t addr1 = qemu_ram_addr_from_host_nofail(buffer);
>> +            /* Will release RAM refcnt */
>> +            ram_addr_t addr1 = qemu_ram_addr_from_host_nofail(buffer, true);
>>              while (access_len) {
>>                  unsigned l;
>>                  l = TARGET_PAGE_SIZE;
>
> Since qemu_ram_addr_from_host_nofail(buffer, true) will decrease the reference
> counter for this memoryregion, I think is should be called regardless of
> read/write i.e.  outside of the "if (is_write)" clause. Otherwise references 
> for
> reads are not decreased properly.
>
Yes, you are right. if reference hold map, it need to be released here.

Thanks,
pingfan


> thanks,
>
> - Vasilis