From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH] kvm: use the correct RCU API Date: Mon, 19 Apr 2010 12:49:04 +0300 Message-ID: <4BCC2710.8090809@redhat.com> References: <4BCC2543.7050104@cn.fujitsu.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , "Paul E. McKenney" , Ingo Molnar , LKML , kvm@vger.kernel.org To: Lai Jiangshan Return-path: Received: from mx1.redhat.com ([209.132.183.28]:60056 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752277Ab0DSJtW (ORCPT ); Mon, 19 Apr 2010 05:49:22 -0400 In-Reply-To: <4BCC2543.7050104@cn.fujitsu.com> Sender: kvm-owner@vger.kernel.org List-ID: On 04/19/2010 12:41 PM, Lai Jiangshan wrote: > The RCU/SRCU API have already changed for proving RCU usage. > > I got the following dmesg when PROVE_RCU=y because we used incorrect API. > This patch coverts rcu_deference() to srcu_dereference() or family API. > > =================================================== > [ INFO: suspicious rcu_dereference_check() usage. ] > --------------------------------------------------- > arch/x86/kvm/mmu.c:3020 invoked rcu_dereference_check() without protection! > > other info that might help us debug this: > > > rcu_scheduler_active = 1, debug_locks = 0 > 2 locks held by qemu-system-x86/8550: > #0: (&kvm->slots_lock){+.+.+.}, at: [] kvm_set_memory_region+0x29/0x50 [kvm] > #1: (&(&kvm->mmu_lock)->rlock){+.+...}, at: [] kvm_arch_commit_memory_region+0xa6/0xe2 [kvm] > > stack backtrace: > Pid: 8550, comm: qemu-system-x86 Not tainted 2.6.34-rc4-tip-01028-g939eab1 #27 > Call Trace: > [] lockdep_rcu_dereference+0xaa/0xb3 > [] kvm_mmu_calculate_mmu_pages+0x44/0x7d [kvm] > [] kvm_arch_commit_memory_region+0xb7/0xe2 [kvm] > [] __kvm_set_memory_region+0x636/0x6e2 [kvm] > [] kvm_set_memory_region+0x37/0x50 [kvm] > [] vmx_set_tss_addr+0x46/0x5a [kvm_intel] > [] kvm_arch_vm_ioctl+0x17a/0xcf8 [kvm] > [] ? unlock_page+0x27/0x2c > [] ? __do_fault+0x3a9/0x3e1 > [] kvm_vm_ioctl+0x364/0x38d [kvm] > [] ? up_read+0x23/0x3d > [] vfs_ioctl+0x32/0xa6 > [] do_vfs_ioctl+0x495/0x4db > [] ? fget_light+0xc2/0x241 > [] ? do_sys_open+0x104/0x116 > [] ? retint_swapgs+0xe/0x13 > [] sys_ioctl+0x47/0x6a > [] system_call_fastpath+0x16/0x1b > > > > +static inline struct kvm_memslots *kvm_memslots(struct kvm *kvm) > +{ > + return rcu_dereference_check(kvm->memslots, > + srcu_read_lock_held(&kvm->srcu) > + || lockdep_is_held(&kvm->slots_lock)); > +} > + > This open-codes srcu_dereference(). I guess we need an srcu_dereference_check(). Paul? btw, perhaps it is possible not to call rcu_dereference from the write paths. -- error compiling committee.c: too many arguments to function