From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 2/2] KVM: Convert read-only users of vm_list to RCU Date: Thu, 10 Feb 2011 14:56:03 +0200 Message-ID: <4D53E063.1040004@redhat.com> References: <4D512EF7.8040409@siemens.com> <4D512F3B.1080107@siemens.com> <4D53BB02.20206@redhat.com> <4D53CCAB.8040204@siemens.com> <4D53DB54.90605@redhat.com> <4D53DDD3.5020704@siemens.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , kvm , Zachary Amsden To: Jan Kiszka Return-path: Received: from mx1.redhat.com ([209.132.183.28]:31547 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750766Ab1BJM4J (ORCPT ); Thu, 10 Feb 2011 07:56:09 -0500 In-Reply-To: <4D53DDD3.5020704@siemens.com> Sender: kvm-owner@vger.kernel.org List-ID: On 02/10/2011 02:45 PM, Jan Kiszka wrote: > >>> > >>> There is no list_move_tail_rcu(). > >> > >> ...specifically not for this one. > > > > Well, we can add one if needed (and if possible). > > I can have a look, at least at the lower hanging fruits. Please keep rcu->parent in the loop. > > > >>> > >>> Why check kvm->deleted? it's in the process of being torn down anyway, > >>> it doesn't matter if mmu_shrink or kvm_destroy_vm pulls the trigger. > >> > >> kvm_destroy_vm removes a vm from the list while mmu_shrink is running. > >> Then mmu_shrink's list_move_tail will re-add that vm to the list tail > >> again (unless already the removal in move_tail produces a crash). > > > > It's too subtle. Communication across threads with a variable needs > > memory barriers (even though they're nops on x86) and documentation. > > The barriers are provided by this spin lock we acquire for testing are > modifying deleted. Right. I'm not thrilled with adding ->deleted though. > > > > btw, not even sure if it's legal: you have a mutating call within an rcu > > read critical section for the same object. If synchronize_rcu() were > > called there, would it ever terminate? > > Why not? kvm_destroy_vm is not preventing blocking mmu_shrink to acquire > the kvm_lock where we then find the vm deleted and release both kvm_lock > and the rcu read "lock" afterwards. synchronize_rcu() waits until all currently running rcu read-side critical sections are completed. But we are in the middle of one, which isn't going to complete until it synchronize_rcu() returns. > > > > (not that synchronize_rcu() is a good thing there, better do it with > > call_rcu()). > > What's the benefit? The downside is a bit more complexity as you need an > additional callback handler. synchronize_rcu() can be very slow (its a systemwide operation), and mmu_shrink() can be called often on a loaded system. -- error compiling committee.c: too many arguments to function