On Fri, May 15, 2015 at 2:42 PM, Dario Faggioli
wrote:
> On Wed, 2015-05-13 at 06:01 +0100, wei.l...@citrix.com wrote:
>
>> = Prognosis =
>>
>> The states are: none -> fair -> ok -> good -> done
>>
>> none - nothing yet
>> fair - still working on it, patches are prototypes or RFC
>> ok - patches
Dario,
>> Without the processor assignment
>> here the vcpu might go on being assigned to a processor it no longer
>> is allowed to run on.
>>
> Ok.
>
>> In that case, function runq_candidate may only
>> get called for the vcpu's old processor, and runq_candidate will no
>> longer let a vcpu run o
t can, I'd look for
>> > ways of preventing that to happen before getting here... It seems easier
>> > and more correct to do that, rather than trying to figure out what to do
>> > here if the mask is empty. :-O
>>
>> Yes, we need to go through the code and make sure that we understand
>> what our assumptions are, so that people can't crash Xen by doing
>> irrational things like making the hard affinity not intersect with the
>> cpupool cpus.
>>
> True.
>
> Something like that can be figured out and either forbidden or, in
> general, addressed in other places, rather than requiring us to care
> here. In fact, this seems to me to be what's happening already (see
> below).
I don't think there's any way the mask can be empty after the
cpumask_and with the cpu_hard_affinity and the VCPU2ONLINE. In
schedule.c:vcpu_set_hard_affinity, if the intersection of the new hard
mask and VCPU2ONLINE is empty, the function returns -EINVAL. I took
into account all the discussion here and modified the function for v3.
Thank you!
Justin Weaver
University of Hawaii at Manoa
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Dario,
I'm working on soft affinity while you review v2 of the credit 2 hard
affinity patch (no rush, of course).
On Tue, Jan 13, 2015 at 3:06 AM, Dario Faggioli
wrote:
> On Sat, 2014-11-29 at 14:33 -1000, Justin T. Weaver wrote:
>> when deciding which run queue to assign each vcpu to.
>>
>> The
On Mon, Feb 2, 2015 at 4:24 AM, Dario Faggioli
wrote:
> On Sat, 2015-01-31 at 20:51 -1000, Justin Weaver wrote:
>> On Mon, Jan 19, 2015 at 9:21 PM, Justin Weaver wrote:
>> > On Mon, Jan 12, 2015 at 8:05 AM, Dario Faggioli
>
>> For example...
>> I start a guest w
On Mon, Jan 19, 2015 at 9:21 PM, Justin Weaver wrote:
> On Mon, Jan 12, 2015 at 8:05 AM, Dario Faggioli
> wrote:
>>> if ( __vcpu_on_runq(svc) )
>>> +on_runq = 1;
>>> +
>>> +/* If the runqs are different, move svc to t
nges. I noticed some odd behavior where it would migrate a vcpu
away from a runqueue that only had the one vcpu running on it. Maybe
it hasn't been tested much because of the single run queue issue which
your patches fix? Anyway, I will examine this section further and see
what I can come up with.
>> @@ -1338,11 +1458,17 @@ retry:
>> {
>> __update_svc_load(ops, pull_svc, 0, now);
>> }
>> -
>> +
>>
> Avoid things like these. I appreciate that this is an actual
> improvement, but let's not mix coding style fixes with new features. :-)
Got it, I will take this out.
>> @@ -1399,8 +1531,12 @@ csched2_vcpu_migrate(
>>
>> trqd = RQD(ops, new_cpu);
>>
>> -if ( trqd != svc->rqd )
>> -migrate(ops, svc, trqd, NOW());
>> +/*
>> + * Call migrate even if svc->rqd == trqd; there may have been an
>> + * affinity change that requires a call to runq_tickle for a new
>> + * processor within the same run queue.
>> + */
>> +migrate(ops, svc, trqd, NOW());
>> }
>>
> As said above, I don't think I see the reason for this. Affinity
> changes, e.g., due to calls to vcpu_set_affinity() in schedule.c, forces
> the vcpu through a sleep wakeup cycle (it calls vcpu_sleep_nosync()
> direcly, while vcpu_wake() is called inside vcpu_migrate()).
>
> So, looks like what you are after (i.e., runq_tickle being called)
> should happen already, isn't it? Are there other reasons you need it
> for?
Like I said above, I will look at this again. My VMs were getting
stuck after certain hard affinity changes. I'll roll back some of
these changes and test it out again.
>> static int
>> @@ -1610,6 +1746,13 @@ runq_candidate(struct csched2_runqueue_data *rqd,
>> {
>> struct csched2_vcpu * svc = list_entry(iter, struct csched2_vcpu,
>> runq_elem);
>>
>> +/*
>> + * If vcpu is not allowed to run on this processor due to
>> + * hard affinity, continue to the next vcpu on the queue.
>> + */
>> +if ( !cpumask_test_cpu(cpu, svc->vcpu->cpu_hard_affinity) )
>> +continue;
>> +
>>
> Looks ok. The comment explains a bit too much the "what", which one can
> easily see from the code. Comments (most of the time :-) ) should be
> about the "why" things are done in a certain way.
>
> Here, the relevant piece of information is that "Only consider vcpus
> allowed to run on this processor", so I'd just say so. The fact that you
> are continuing scanning the runque is pretty evident. :-D
Understood, I'll fix the comment here.
Thank you for the feedback! I agree that the hard affinity piece needs
to be solid before the soft affinity. I'm going to hold off on
replying to your feedback on my second patch until I get this one
fixed. I hope that's OK.
Justin Weaver
University of Hawaii at Manoa
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
elopment-report/
I will take a look at these and try to run additional workloads. If
you have any other ideas about workloads I should try, please let me
know.
Thank you!
Justin Weaver
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
