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

["Jan Beulich" <JBeulich@suse.com>]

>>> On 09.04.16 at 16:42, <konrad@kernel.org> wrote:
> On Thu, Apr 07, 2016 at 09:38:53AM -0600, Jan Beulich wrote:
>> >>> On 07.04.16 at 05:05, <konrad.wilk@oracle.com> 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.
> 
> By how much?

1ms would seem more than enough to me at a first glance.

> The clock (timeout) starts ticking the moment the schedule_work
> is called - to quiten all the CPUs. Adding an acceleration once
> they have passed the #1 stage is modifying the semantics of the
> timeout ("well, it was 30ms, but once it goes over phase #1 it
> is shorten by half (or is it a quarter?").
> 
> Should we expose both timeouts to the sysctl so that the user can
> customize the acceleration ratio?
> 
> Perhaps we can fiddle with this later?

Sure. It was just a thought, not an immediate requirement.

Jan


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