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

[Konrad Rzeszutek Wilk <konrad@xxxxxxxxxx>]

On Thu, Apr 07, 2016 at 09:38:53AM -0600, Jan Beulich wrote:
> >>> 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.

By how much?

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?
> 
> Jan
> 

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