From mboxrd@z Thu Jan  1 00:00:00 1970
From: Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp>
Subject: Re: [PATCH 2/6] qemu-kvm: Modify and introduce wrapper functions
 to access phys_ram_dirty.
Date: Wed, 17 Mar 2010 07:50:48 +0900
Message-ID: <4BA00B48.5040108@lab.ntt.co.jp>
References: <1268736839-27371-1-git-send-email-tamura.yoshiaki@lab.ntt.co.jp> <1268736839-27371-3-git-send-email-tamura.yoshiaki@lab.ntt.co.jp> <4B9F7D78.5090201@redhat.com> <4B9F8904.2040704@codemonkey.ws>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: Avi Kivity <avi@redhat.com>, kvm@vger.kernel.org,
	qemu-devel@nongnu.org, ohmura.kei@lab.ntt.co.jp
To: Anthony Liguori <anthony@codemonkey.ws>
Return-path: <kvm-owner@vger.kernel.org>
Received: from tama50.ecl.ntt.co.jp ([129.60.39.147]:61928 "EHLO
	tama50.ecl.ntt.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1750794Ab0CPWvK (ORCPT <rfc822;kvm@vger.kernel.org>);
	Tue, 16 Mar 2010 18:51:10 -0400
In-Reply-To: <4B9F8904.2040704@codemonkey.ws>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Anthony Liguori wrote:
> On 03/16/2010 07:45 AM, Avi Kivity wrote:
>> On 03/16/2010 12:53 PM, Yoshiaki Tamura wrote:
>>> Modifies wrapper functions for byte-based phys_ram_dirty bitmap to
>>> bit-based phys_ram_dirty bitmap, and adds more wrapper functions to
>>> prevent
>>> direct access to the phys_ram_dirty bitmap.
>>
>>> +
>>> +static inline int cpu_physical_memory_get_dirty_flags(ram_addr_t addr)
>>> +{
>>> + unsigned long mask;
>>> + int index = (addr>> TARGET_PAGE_BITS) / HOST_LONG_BITS;
>>> + int offset = (addr>> TARGET_PAGE_BITS)& (HOST_LONG_BITS - 1);
>>> + int ret = 0;
>>> +
>>> + mask = 1UL<< offset;
>>> + if (phys_ram_vga_dirty[index]& mask)
>>> + ret |= VGA_DIRTY_FLAG;
>>> + if (phys_ram_code_dirty[index]& mask)
>>> + ret |= CODE_DIRTY_FLAG;
>>> + if (phys_ram_migration_dirty[index]& mask)
>>> + ret |= MIGRATION_DIRTY_FLAG;
>>> +
>>> + return ret;
>>> }
>>>
>>> static inline int cpu_physical_memory_get_dirty(ram_addr_t addr,
>>> int dirty_flags)
>>> {
>>> - return phys_ram_dirty[addr>> TARGET_PAGE_BITS]& dirty_flags;
>>> + return cpu_physical_memory_get_dirty_flags(addr)& dirty_flags;
>>> }
>>
>> This turns one cacheline access into three. If the dirty bitmaps were
>> in an array, you could do
>>
>> return dirty_bitmaps[dirty_index][addr >> (TARGET_PAGE_BITS +
>> BITS_IN_LONG)] & mask;
>>
>> with one cacheline access.
>
> As far as I can tell, we only ever call with a single flag so your
> suggestion makes sense.
>
> I'd suggest introducing these functions before splitting the bitmap up.
> It makes review a bit easier.

Thanks for your advise.
I'll post the wrapper functions for existing byte-based phys_ram_dirty first.

Yoshi