From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 17/20] SMP: Implement on_cpu() Date: Tue, 10 Jul 2007 09:03:09 +0300 Message-ID: <4693211D.4040406@qumranet.com> References: <11838956891287-git-send-email-avi@qumranet.com> <11838956893094-git-send-email-avi@qumranet.com> <4691D9C1.4050309@qumranet.com> <20070709071640.GB10864@one.firstfloor.org> <46920270.3080309@qumranet.com> <46921BE9.4040801@qumranet.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: KVM , Andrew Morton , Andi Kleen , linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Satyam Sharma Return-path: In-Reply-To: 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 Satyam Sharma wrote: > > > On 7/9/07, Andi Kleen wrote: >> [...] >> on_each_cpu() was imho always a mistake. It would have been better >> to just fix smp_call_function() directly > > I'm not sure what you mean by "fix" here, but if you're proposing > that we change smp_call_function() semantics to _include_ the > current CPU (and just run the given function locally also along > with the others -- and hence get rid of on_each_cpu) then I'm sorry > but I'll have to *violently* disagree with that. Please remember that > the current CPU _must_ be treated specially in a whole *lot* of > usage scenarios ... I imagine that by "fix" Andi means also updating all callers. Otherwise he would just have said "break". > > On 7/9/07, Andi Kleen wrote: >> > I think it would be better to fix smp_call_function_single to just >> > handle this case transparently. There aren't that many callers yet >> > because it is >> > fairly new. > > Take the same example here -- let's say we want to send a > "for (;;) ;" kind of function to a specified CPU. Now let's say > by the time we've called smp_call_function_single() on that > target CPU, we're preempted out and then get rescheduled > on the target CPU itself. There, we begin executing the > smp_call_function_single() (as modified by Avi here with your > proposed changed semantics) and notice that we've landed > on the target CPU itself, execute the suicidal function > _locally_ *in current thread* itself, and ... well, I hope you > get the picture. So you disable preemption before calling smp_call_function_single(). > > So my opinion is to go with the get_cpu() / put_cpu() wrapper > Avi is proposing here and keep smp_call_function{_single} > semantics unchanged. [ Also please remember that for > *correctness*, preemption needs to be disabled by the > _caller_ of smp_call_function{_single} functions, doing so > inside them is insufficient. ] That's not correct. kvm has two places where you can call the new smp_call_function_single() (or on_cpu()) without disabling preemption. There are also a couple of existing places that don't need to disable preemption with the new semantics (see mtrr_save_state(), do_cpuid(), _rdmsr_on_cpu(), all in arch/i386 for examples). In fact I think more places can take advantage of the new semantics than not. -- 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/