Satyam Sharma wrote:
>
>
> On 7/9/07, Andi Kleen <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
