[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [RFC PATCH v1 2/6] sched: track time spent in hypervisor tasks
Hello Jürgen, Jürgen Groß writes: > On 12.06.20 13:30, Volodymyr Babchuk wrote: >> On Fri, 2020-06-12 at 06:43 +0200, Jürgen Groß wrote: >>> On 12.06.20 02:22, Volodymyr Babchuk wrote: [...] >>>> + delta = NOW() - v->hyp_entry_time; >>>> + atomic_add(delta, &v->sched_unit->hyp_time); >>>> + >>>> +#ifndef NDEBUG >>>> + v->in_hyp_task = false; >>>> +#endif >>>> +} >>>> + >>>> /* >>>> * Do the actual movement of an unit from old to new CPU. Locks for >>>> *both* >>>> * CPUs needs to have been taken already when calling this! >>>> @@ -2615,6 +2646,7 @@ static void schedule(void) >>>> SCHED_STAT_CRANK(sched_run); >>>> + vcpu_end_hyp_task(current); >>>> rcu_read_lock(&sched_res_rculock); >>>> lock = pcpu_schedule_lock_irq(cpu); >>>> diff --git a/xen/common/softirq.c b/xen/common/softirq.c >>>> index 063e93cbe3..03a29384d1 100644 >>>> --- a/xen/common/softirq.c >>>> +++ b/xen/common/softirq.c >>>> @@ -71,7 +71,9 @@ void process_pending_softirqs(void) >>>> void do_softirq(void) >>>> { >>>> ASSERT_NOT_IN_ATOMIC(); >>>> + vcpu_begin_hyp_task(current); >>>> __do_softirq(0); >>>> + vcpu_end_hyp_task(current); >>> >>> This won't work for scheduling. current will either have changed, >>> or in x86 case __do_softirq() might just not return. You need to >>> handle that case explicitly in schedule() (you did that for the >>> old vcpu, but for the case schedule() is returning you need to >>> call vcpu_begin_hyp_task(current) there). >>> >> Well, this is one of questions, I wanted to discuss. I certainly >> need >> to call vcpu_begin_hyp_task(current) after context switch. But what it >> is the right place? If my understaning is right, code on x86 platform >> will never reach this point. Or I'm wrong there? > > No, this is correct. > > You can add the call to context_switch() just after set_current() has > been called. Looks like I'm missing something there. If I get this right, code you mentioned is executed right before leaving hypervisor. So, as I see this, functions are called in the following way (on x86): 1. do_softirq() calls vcpu_begin_hyp_task() and then executes __do_softirq() 2. __do_softirq() does different jobs and eventually calls schedule() 3. schedule() calls vcpu_end_hyp_task() and makes scheduling decision which leads to call to context_switch() 4. On end context_switch() we will exit hypervisor and enter VM. At least, this is how I understand nextd->arch.ctxt_switch->tail(next); call. So, no need to call vcpu_begin_hyp_task() in context_switch() for x86. On ARM, this is different story. There, I am calling vcpu_begin_hyp_task() after set_current() because ARM code will eventually return to do_softirq() and there will be called corresponding vcpu_end_hyp_task(). I have put bunch of ASSERTs to ensure that vcpu_begin_hyp_task() or vcpu_end_hyp_task() are not called twice and that vcpu_end_hyp_task() is called after vcpu_begin_hyp_task(). Those asserts are not failing, so I assume that I did all this in the right way :) -- Volodymyr Babchuk at EPAM
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |