From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57570) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZPndt-00068Z-8V for qemu-devel@nongnu.org; Thu, 13 Aug 2015 04:13:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZPndq-000127-0S for qemu-devel@nongnu.org; Thu, 13 Aug 2015 04:13:41 -0400 Received: from mail-wi0-x234.google.com ([2a00:1450:400c:c05::234]:33679) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZPndp-00011z-OJ for qemu-devel@nongnu.org; Thu, 13 Aug 2015 04:13:37 -0400 Received: by wijp15 with SMTP id p15so248132655wij.0 for ; Thu, 13 Aug 2015 01:13:37 -0700 (PDT) Sender: Paolo Bonzini References: <1439397664-70734-1-git-send-email-pbonzini@redhat.com> <1439397664-70734-9-git-send-email-pbonzini@redhat.com> <20150812203734.GA17706@flamenco> From: Paolo Bonzini Message-ID: <55CC51AC.7030503@redhat.com> Date: Thu, 13 Aug 2015 10:13:32 +0200 MIME-Version: 1.0 In-Reply-To: <20150812203734.GA17706@flamenco> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 08/10] tcg: add memory barriers in page_find_alloc accesses List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Emilio G. Cota" Cc: mttcg@greensocs.com, qemu-devel@nongnu.org, fred.konrad@greensocs.com On 12/08/2015 22:37, Emilio G. Cota wrote: > > page_find is reading the radix tree outside all locks, so it has to > > use the RCU primitives. It does not need RCU critical sections > > because the PageDescs are never removed, so there is never a need > > to wait for the end of code sections that use a PageDesc. > > Note that rcu_find_alloc might end up writing to the tree, see below. Yes, but in that case it's always called with the mmap_lock held, see patch 7. page_find_alloc is only called by tb_alloc_page (called by tb_link_page which takes mmap_lock), or by page_set_flags (called with mmap_lock held by linux-user/mmap.c). > BTW the fact that there are no removals makes the use of RCU unnecessary. It only makes it not use the RCU synchronization primitives. You still need the memory barriers. > I argue however that it is better to call page_find/_alloc with a mutex held, > since otherwise we'd have to add per-PageDesc locks (it's very common to > call page_find and then update the PageDesc). The fields are protected by either the mmap_lock (e.g. the flags, see page_unprotect and tb_alloc_page) or the tb_lock (e.g. the tb lists). The code is complicated and could definitely use more documentation, especially for struct PageDesc, but it seems correct to me apart from the lock inversion fixed in patch 10. Paolo