Re: [Xen-devel] [PATCH v8 10/11] viridian: add implementation of synthetic timers

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.03.19 at 16:36, <Paul.Durrant@xxxxxxxxxx> wrote:
>> From: Xen-devel [mailto:xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx] On Behalf Of 
>> Jan Beulich
>> Sent: 18 March 2019 15:21
>> 
>> >>> On 18.03.19 at 15:37, <Paul.Durrant@xxxxxxxxxx> wrote:
>> >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> >> Sent: 18 March 2019 14:24
>> >>
>> >> >>> On 18.03.19 at 12:20, <paul.durrant@xxxxxxxxxx> wrote:
>> >> > +    ASSERT(expiration - now > 0);
>> >> > +
>> >> > +    vs->expiration = expiration;
>> >> > +    timeout = (expiration - now) * 100ull;
>> >> > +
>> >> > +    vs->started = true;
>> >> > +    migrate_timer(&vs->timer, smp_processor_id());
>> >>
>> >> Why is this smp_processor_id() when viridian_time_vcpu_init() uses
>> >> v->processor? How relevant is it in the first place to trace the pCPU
>> >> the vCPU runs on for the timer?
>> >
>> > I was just following suit with other timer code. It seems to be the norm to
>> > migrate to the current pCPU just prior to starting a timer.
>> 
>> But wouldn't v->processor then be more visibly correct (besides
>> likely being cheaper to get at), as to the correlation to the vCPU
>> in question? I can't actually see why "migrate to the current pCPU"
>> would be the norm; I could only see this as an implication from
>> that other code you looked at simply acting on the current vCPU.
>> 
>> Then again I'm having trouble spotting why it would be important
>> for the timer to run on the same CPU the vCPU runs one. By the
>> time the timer fires, the vCPU may have gone elsewhere.
>> 
> 
> I have no problem dropping the migrate call. As I said, I was following 
> prior example e.g. in the VCPUOP_set_singleshot_timer handler and in 
> vcpu_periodic_timer_work(), which do indeed run on current... but then so 
> will start_timer() in the vast majority of invocations (the invocation in 
> viridian_time_vcpu_thaw() being the exception). I'm happy for you to swap it 
> for v->processor on commit unless you want me to send a v9 with the change?

No need to send v9 just for this.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel