Re: [Xen-devel] [PATCH v4] xen: sched: convert RTDS from time to event driven model

[Tianyang Chen <tiche@xxxxxxxxxxxxxx>]

On 2/8/2016 6:27 AM, Dario Faggioli wrote:
> On Fri, 2016-02-05 at 23:27 -0500, Tianyang Chen wrote:
> > On 2/5/2016 9:39 AM, Dario Faggioli wrote:
> > > On Wed, 2016-02-03 at 21:23 -0500, Tianyang Chen wrote:
> > > >
> > I see. So I'm just curious what can cause spurious wakeup? Does it
> > only
> > happen to a running vcpu (currently on pcpu), and a vcpu that is on
> > either runq or depletedq?
> Hehe, "that's virt, baby!" :-P
>
> No, seriously, vcpu being already running or already in a queue, is
> indeed what makes me call a wakeup a "spurious" wakeup. You can check
> at the performance counters that we update in those cases, to see when
> they happen most. In my experience, on Credit, I've seen them happening
> mostly when a vcpu is running. For instance:
>
> root@Zhaman:~# xenperf |grep vcpu_wake
> sched: vcpu_wake_running            T=        39
> sched: vcpu_wake_onrunq             T=         0
> sched: vcpu_wake_runnable           T=     59875
> sched: vcpu_wake_not_runnable       T=         0
>
> > > So we realized that this is just a spurious wakeup, and get back to
> > > what we were doing.
> > >
> > > What's wrong with this I just said?
> > The reason why I wanted to add a vcpu back to replq is because it
> > could
> > be taken off from the timer handler. Now, because of the spurious
> > wakeup, I think the timer shouldn't take any vcpus off of replq, in
> > which case wake() should be doing nothing just like other schedulers
> > when it's a spurious wakeup. Also, only sleep() should remove events
> > from replq.
> Err... well, that depends on the how the code ends up looking like, and
> it's start to become difficult to reason on it like this.
>
> Perhaps, considering that it looks to me that we are in agreement on
> all the important aspects, you can draft a new version and we'll reason
> and comment on top of that?
>
> > > Mmm... no, I think we should know/figure out whether a
> > > replenishment is
> > > pending already and we are ok with it, or if we need a new one.
> > Just to make sure, spurious sleep/wakeup are for a vcpu that is on
> > pcpu
> > or any queue right?
> Yes, spurious wakeup is how I'm calling wakeups that needs no action
> from the schedule, because the vcpu is already awake (and either
> running or runnable).
>
> I don't think there is such a thing as spurious sleep... When sleep is
> called, we go to sleep. There are cases where the sleep-->wakeup
> turnaround is really really fast, but I would not call them "spurious
> sleeps", and they should not need much special treatment, not in
> sched_rt.c at least.
>
> Oh, maybe you mean the cases where you wakeup and you find out that
> context_saved() has not run yet (just occurred by looking at the code
> below)? Well, yes... But I wouldn't call those "spurious sleeps"
> either.
>
> > If a real sleep happens, it should be taken off from replq. However,
> > in
> > spurious wakeup (which I assume corresponds to a spurious sleep?),
> > it
> > shouldn't be taken off from replq. Adding back to replq should
> > happen
> > for those vcpus which were taken off because of "real sleep".
> I don't really follow, but I have the feeling that you're making it
> sound more complicated like it is in reality... :-)
So my reasoning is that, when sleep() is called in sched_rt.c, a vcpu 
will need to be taken off from the replenishment event queue. However, a 
vcpu can be put to sleep/not-runnable without even calling sleep(), 
which corresponds to a later "spurious wakeup". Is there any way that 
RTDS can know when this happens? If not, in rt_vcpu_wake(), it needs to 
check, in all situations, if a vcpu is on the replenishment event queue 
or not. If not, add it back. I'm I right?
vcpu running      --> sleep(), taken off from replq  
vcpu on queue     --> sleep(), taken off from replq
vcpu not on queue --> sleep(), taken off from replq


However,

vcpu running/on queque/not on queue --> just not runnable anymore (still 
on replq) --> spurious wakeup (still on replq)
vcpus shouldn't be added back to replq again.

So in all cases, rt_vcpu_wake() always need to check if a vcpu is on the 
replq or not.
Now I think because of the addition of a replenishment queue, spurious 
wake up cannot be simply treated as NOP. Just like V4, the "Out" label 
basically maintains the replenishment queue.
Anyways, I agree it's gonna be easier to reason on v5 where all changes 
are applied.
Thanks,
Tianyang

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