Re: [Xen-devel] [PATCH v5 11/28] xsplice: Implement support for applying/reverting/replacing patches.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.04.16 at 05:05, <konrad.wilk@xxxxxxxxxx> wrote:
>> >> > +        /* All CPUs are waiting, now signal to disable IRQs. */
>> >> > +        xsplice_work.ready = 1;
>> >> > +        smp_wmb();
>> >> > +
>> >> > +        atomic_inc(&xsplice_work.irq_semaphore);
>> >> > +        if ( !xsplice_do_wait(&xsplice_work.irq_semaphore, timeout, 
>> >> > total_cpus,
>> >> > +                              "Timed out on IRQ semaphore") )
>> >> 
>> >> I'd prefer if the common parts of that message moved into
>> >> xsplice_do_wait() - no reason to have more string literal space
>> >> occupied than really needed. Also, looking back, the respective
>> >> function parameter could do with a more descriptive name.
>> >> 
>> >> And then - does it make sense to wait the perhaps full 30ms
>> >> on this second step? Rendezvoused CPUs should react rather
>> >> quickly to the signal to disable interrupts.
>> > 
>> > We don't reset the timeout - the timeout is for both calls
>> > to xsplice_do_wait.
>> 
>> I understand that's the way it's right now, but that's what I'm putting
>> under question. Rendezvousing CPUs is quite a bit more at risk of
>> taking some time compared to having already rendezvoused CPUs
>> disable their IRQs.
> 
> Yes. I could expand the timeout, but maybe have some reset (add more
> timeout) once CPUs  come along?

Expand the timeout? Add more timeout? I don't understand. My
point was about shortening the timeout on the second step.

Jan


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