From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: irqdevice INTR example Date: Mon, 16 Apr 2007 08:46:40 +0300 Message-ID: <46230DC0.8090504@qumranet.com> References: <461D7702.BA47.005A.0@novell.com> <461DE791.1040707@qumranet.com> <461DE5C9.BA47.005A.0@novell.com> <461E2AD5.7070905@qumranet.com> <461DFF1C.BA47.005A.0@novell.com> <461E3EDB.3080002@qumranet.com> <461E1F73.BA47.005A.0@novell.com> <4620E56A.7040207@qumranet.com> <46226FBC.BA47.005A.0@novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org To: Gregory Haskins Return-path: In-reply-to: <46226FBC.BA47.005A.0-Et1tbQHTxzrQT0dZR+AlfA@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org Errors-To: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: kvm.vger.kernel.org Gregory Haskins wrote: > Out of curiosity, did you review my kick_process() version or the newer smp_call_function_single() based version of the patch? > > The former. >> >>> @@ -1866,6 +1874,19 @@ static int kvm_vcpu_ioctl_run(struct kvm_vcpu *vcpu, >>> >> struct kvm_run *kvm_run) >> >>> kvm_arch_ops->decache_regs(vcpu); >>> } >>> >>> + /* >>> + * Make sure the current task is accurately recorded. Most of the >>> + * time, it will be so we first check if its necessary without taking >>> + * the lock. If there is a mismatch, we must aquire the lock and >>> + * check again to eliminate races >>> + */ >>> + if (unlikely(vcpu->irq.task != current)) { >>> + spin_lock(&vcpu->irq.lock); >>> + if (vcpu->irq.task != current) >>> + vcpu->irq.task = current; >>> + spin_unlock(&vcpu->irq.lock); >>> + } >>> + >>> >>> >> Isn't this equivalent to >> >> vcpu->irq.task = current >> > > Yeah, good point. I originally had a printk in there to report when it changes, thus the conditional. I didn't collapse it when I took the printk out. I suppose you could argue that updating the variable indiscriminately will unnecessarily dirty a cache-line, but I doubt it would matter in the grand scheme of things. > > The vcpu cachelines are already dirty. Please repost the patchset (one patch per message) so I can see it together. Thanks. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. ------------------------------------------------------------------------- This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/