From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:33839)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <udeshpan@redhat.com>) id 1QuIRb-0006SG-AB
	for qemu-devel@nongnu.org; Fri, 19 Aug 2011 02:20:40 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <udeshpan@redhat.com>) id 1QuIRa-0007E9-4s
	for qemu-devel@nongnu.org; Fri, 19 Aug 2011 02:20:39 -0400
Received: from mx1.redhat.com ([209.132.183.28]:10720)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <udeshpan@redhat.com>) id 1QuIRZ-0007Du-U1
	for qemu-devel@nongnu.org; Fri, 19 Aug 2011 02:20:38 -0400
Message-ID: <4E4E00B1.9050009@redhat.com>
Date: Fri, 19 Aug 2011 02:20:33 -0400
From: Umesh Deshpande <udeshpan@redhat.com>
MIME-Version: 1.0
References: <cover.1313552764.git.udeshpan@redhat.com>
	<8f99d56f3a48b6255cf70425bc435d8f231f5352.1313552764.git.udeshpan@redhat.com>
	<4E4B5FA8.4020006@redhat.com>
In-Reply-To: <4E4B5FA8.4020006@redhat.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [RFC PATCH v4 2/5] ramlist mutex
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: Paolo Bonzini <pbonzini@redhat.com>
Cc: mtosatti@redhat.com, qemu-devel@nongnu.org, kvm@vger.kernel.org, quintela@redhat.com

On 08/17/2011 02:28 AM, Paolo Bonzini wrote:
> On 08/16/2011 08:56 PM, Umesh Deshpande wrote:
>> @@ -3001,8 +3016,10 @@ void qemu_ram_free_from_ptr(ram_addr_t addr)
>>
>>       QLIST_FOREACH(block,&ram_list.blocks, next) {
>>           if (addr == block->offset) {
>> +            qemu_mutex_lock_ramlist();
>>               QLIST_REMOVE(block, next);
>>               QLIST_REMOVE(block, next_mru);
>> +            qemu_mutex_unlock_ramlist();
>>               qemu_free(block);
>>               return;
>>           }
>> @@ -3015,8 +3032,10 @@ void qemu_ram_free(ram_addr_t addr)
>>
>>       QLIST_FOREACH(block,&ram_list.blocks, next) {
>>           if (addr == block->offset) {
>> +            qemu_mutex_lock_ramlist();
>>               QLIST_REMOVE(block, next);
>>               QLIST_REMOVE(block, next_mru);
>> +            qemu_mutex_unlock_ramlist();
>>               if (block->flags&  RAM_PREALLOC_MASK) {
>>                   ;
>>               } else if (mem_path) {
>
> You must protect the whole QLIST_FOREACH.  Otherwise looks good.
Or, is it okay to convert all the ramblock list traversals in exec.c 
(under iothread) to mru traversals, and probably it makes sense as the 
original list was also maintained in the mru order, whereas the sequence 
of blocks doesn't matter for the migration code. This way we don't have 
to acquire the mutex for block list traversals.

- Umesh