From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: [RFC PATCH v3 3/4] lock to protect memslots Date: Mon, 15 Aug 2011 00:26:48 -0700 Message-ID: <20110815072648.GA2916@amt.cnet> References: <4E440131.6020200@redhat.com> <4E44CC1E.202@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org, Umesh Deshpande , quintela@redhat.com To: Paolo Bonzini Return-path: Received: from mx1.redhat.com ([209.132.183.28]:9694 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752614Ab1HOH05 (ORCPT ); Mon, 15 Aug 2011 03:26:57 -0400 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p7F7Qvip028651 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 15 Aug 2011 03:26:57 -0400 Content-Disposition: inline In-Reply-To: <4E44CC1E.202@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Fri, Aug 12, 2011 at 08:45:50AM +0200, Paolo Bonzini wrote: > On 08/11/2011 06:20 PM, Paolo Bonzini wrote: > >> > >>+ qemu_mutex_lock_ramlist(); > >> QLIST_REMOVE(block, next); > >> QLIST_INSERT_HEAD(&ram_list.blocks, block, next); > >>+ qemu_mutex_unlock_ramlist(); > > > >Theoretically qemu_get_ram_ptr should be protected. The problem is not > >just accessing the ramlist, it is accessing the data underneath it > >before anyone frees it. Luckily we can set aside that problem for now, > >because qemu_ram_free_from_ptr is only used by device assignment and > >device assignment makes VMs unmigratable. > > Hmm, rethinking about it, all the loops in exec.c should be > protected from the mutex. That's not too good because > qemu_get_ram_ptr is a hot path for TCG. Right. > Perhaps you can also avoid > the mutex entirely, and just disable the above optimization for > most-recently-used-block while migration is running. It's not a > complete solution, but it could be good enough until we have RAM > hot-plug/hot-unplug. Actually the previous patchset does not traverse the ramlist without qemu_mutex locked, which is safe versus the most-recently-used-block optimization. So, agreed that the new ramlist lock is not necessary for now.