[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 01/15] xen: in do_softirq() sample smp_processor_id() once and for all.



On 07/06/17 15:38, Jan Beulich wrote:
>>>> On 01.06.17 at 19:33, <dario.faggioli@xxxxxxxxxx> wrote:
>> In fact, right now, we read it at every iteration of the loop.
>> The reason it's done like this is how context switch was handled
>> on IA64 (see commit ae9bfcdc, "[XEN] Various softirq cleanups" [1]).
>>
>> However:
>> 1) we don't have IA64 any longer, and all the achitectures that
>>    we do support, are ok with sampling once and for all;
>> 2) sampling at every iteration (slightly) affect performance;
>> 3) sampling at every iteration is misleading, as it makes people
>>    believe that it is currently possible that SCHEDULE_SOFTIRQ
>>    moves the execution flow on another CPU (and the comment,
>>    by reinforcing this belief, makes things even worse!).
>>
>> Therefore, let's:
>> - do the sampling once and for all, and remove the comment;
>> - leave an ASSERT() around, so that, if context switching
>>   logic changes (in current or new arches), we will notice.
> 
> I'm not convinced. The comment clearly says "may", and I don't
> think the performance overhead of smp_processor_id() is that
> high. Keeping the code as is allows some future port to do
> context switches like ia64 did, if that's more efficient there.

In English, "may" not only means "has a possibility of not happening",
but also means "has a possibility of happening".  That's false in the
hypervisor code the way it's currently written.  If I said, "Bring a
water pistol to the XenSummit because Jan may burst into flames" I think
you'd say I was implying something false. :-)

I think it would be better to remove it.  There are doubtless *lots* of
places in the code now that implicitly rely on context_switch() never
returning.  The ASSERT() will catch this issue quickly enough if we ever
re-introduce that behavior.

On the other hand, unless there's a noticeable performance overhead,
this isn't a big issue.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.