From: Dario Faggioli <dario.faggioli@citrix.com>
To: Tianyang Chen <tiche@seas.upenn.edu>, xen-devel@lists.xenproject.org
Cc: george.dunlap@citrix.com, Dagaen Golomb <dgolomb@seas.upenn.edu>,
Meng Xu <mengxu@cis.upenn.edu>
Subject: Re: [PATCH v5][RFC]xen: sched: convert RTDS from time to event driven model
Date: Thu, 25 Feb 2016 18:51:23 +0100 [thread overview]
Message-ID: <1456422683.2959.46.camel@citrix.com> (raw)
In-Reply-To: <56CF39F3.5040807@seas.upenn.edu>
[-- Attachment #1.1: Type: text/plain, Size: 3824 bytes --]
On Thu, 2016-02-25 at 12:29 -0500, Tianyang Chen wrote:
> On 2/25/2016 5:34 AM, Dario Faggioli wrote:
> > >
> > Which one ASSERT() fails?
> >
> The replq_insert() fails because it's already on the replenishment
> queue
> when rt_vcpu_wake() is trying to insert a vcpu again.
>
> (XEN) Xen call trace:
> (XEN) [<ffff82d08012a003>] sched_rt.c#rt_vcpu_wake+0xf0/0x17f
> (XEN) [<ffff82d08012be0c>] vcpu_wake+0x213/0x3d4
> (XEN) [<ffff82d08012c327>] vcpu_unblock+0x4b/0x4d
> (XEN) [<ffff82d080169cea>] vcpu_kick+0x20/0x6f
> (XEN) [<ffff82d080169d65>] vcpu_mark_events_pending+0x2c/0x2f
> (XEN) [<ffff82d08010762a>]
> event_2l.c#evtchn_2l_set_pending+0xa9/0xb9
> (XEN) [<ffff82d08010822a>] evtchn_send+0x158/0x183
> (XEN) [<ffff82d0801096fc>] do_event_channel_op+0xe21/0x147d
> (XEN) [<ffff82d0802439e2>] lstar_enter+0xe2/0x13c
> (XEN)
> (XEN)
> (XEN) ****************************************
> (XEN) Panic on CPU 0:
> (XEN) Assertion '!__vcpu_on_replq(svc)' failed at sched_rt.c:527
> (XEN) ****************************************
>
Ok, makes sense.
> > > And like you
> > > said, mostly spurious sleep
> > > happens when a vcpu is running and it could happen in other
> > > cases,
> > > although rare.
> > >
> > I think I said already there's no such thing as "spurious sleep".
> > Or at
> > least, I can't think of anything that I would define a spurious
> > sleep,
> > if you do, please, explain what situation you're referring to.
> >
> I meant to say spurious wakeup...
>
If, for spurious wakeup, we refer to wakeups happening when the vcpu is
either running or runnable (and hence in the runqueue already), which I
do, we don't even get to call __replq_insert() in those cases. I mean,
we leave rt_vcpu_wake() before that, don't we?
> If rt_vcpu_sleep() removes vcpus from
> replenishment queue, it's perfectly safe for rt_vcpu_wake() to
> insert
> them back.
>
It's safe against sleeps, not against blockings. That's the point I'm
trying to make.
> So, I'm suspecting it's the spurious wakeup that's causing
> troubles because vcpus are not removed prior to rt_vcpu_wake().
> However,
> the two RETURNs at the beginning of rt_vcpu_wake() should catch that
> shouldn't it?
>
Exactly!
> > In any case, one way of dealing with vcpus blocking/offlining/etc
> > could
> > be to, in context_saved(), in case we are not adding the vcpu back
> > to
> > the runq, cancel its replenishment event with __replq_remove().
> >
> > (This may make it possible to avoid doing it in rt_vcpu_sleep()
> > too,
> > but you'll need to check and test.)
> >
> > Can you give this a try.
> That makes sense. Doing it in context_saved() kinda implies that if
> a
> vcpu is sleeping and taken off, its replenishment event should be
> removed. On the other hand, the logic is the same as removing it in
> rt_vcpu_sleep() but just at different times.
>
It is, but, if done properly, it catches more cases, or at least that's
what I'm after.
> Well, I have tried it and
> the check still needs to be there in rt_vcpu_wake(). I will send the
> next version so it's easier to look at.
>
If you're still seeing assertion failures, perhaps context_saved() is
not the right place where to do that... I'll think more about this...
Anyway, yes, let's see the code. Please, also send again, as a follow
up email, the assertion failure log you get if you remove the check.
Thanks and Regards,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
prev parent reply other threads:[~2016-02-25 17:51 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-09 4:33 [PATCH v5][RFC]xen: sched: convert RTDS from time to event driven model Tianyang Chen
2016-02-16 3:55 ` Meng Xu
2016-02-18 1:55 ` Tianyang Chen
2016-02-24 15:23 ` Tianyang Chen
2016-02-25 2:02 ` Dario Faggioli
2016-02-25 6:15 ` Tianyang Chen
2016-02-25 10:34 ` Dario Faggioli
2016-02-25 17:29 ` Tianyang Chen
2016-02-25 17:51 ` Dario Faggioli [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1456422683.2959.46.camel@citrix.com \
--to=dario.faggioli@citrix.com \
--cc=dgolomb@seas.upenn.edu \
--cc=george.dunlap@citrix.com \
--cc=mengxu@cis.upenn.edu \
--cc=tiche@seas.upenn.edu \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).