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:34:28 +0200 Message-ID: <4D53DB54.90605@redhat.com> References: <4D512EF7.8040409@siemens.com> <4D512F3B.1080107@siemens.com> <4D53BB02.20206@redhat.com> <4D53CCAB.8040204@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]:39942 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750946Ab1BJMee (ORCPT ); Thu, 10 Feb 2011 07:34:34 -0500 In-Reply-To: <4D53CCAB.8040204@siemens.com> Sender: kvm-owner@vger.kernel.org List-ID: On 02/10/2011 01:31 PM, Jan Kiszka wrote: > > > >> > >> @@ -3607,10 +3607,14 @@ static int mmu_shrink(struct shrinker *shrink, int nr_to_scan, gfp_t gfp_mask) > >> spin_unlock(&kvm->mmu_lock); > >> srcu_read_unlock(&kvm->srcu, idx); > >> } > >> - if (kvm_freed) > >> - list_move_tail(&kvm_freed->vm_list,&vm_list); > >> + if (kvm_freed) { > >> + raw_spin_lock(&kvm_lock); > >> + if (!kvm->deleted) > >> + list_move_tail(&kvm_freed->vm_list,&vm_list); > > > > There is no list_move_tail_rcu(). > > ...specifically not for this one. Well, we can add one if needed (and if possible). > > > > 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. 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? (not that synchronize_rcu() is a good thing there, better do it with call_rcu()). -- error compiling committee.c: too many arguments to function