[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3 30/47] xen/sched: add support for multiple vcpus per sched unit where missing



On 25.09.2019 14:40, Jürgen Groß  wrote:
> On 24.09.19 17:22, Jan Beulich wrote:
>> On 24.09.2019 17:09, Jürgen Groß wrote:
>>> On 24.09.19 17:00, Jan Beulich wrote:
>>>> On 24.09.2019 16:41, Jürgen Groß wrote:
>>>>> for_each_sched_unit_vcpu() for an idle
>>>>> unit might end premature when one of the vcpus is running in another
>>>>> unit (idle_vcpu->sched_unit != idle_unit).
>>>>
>>>> Oh, that (v)->sched_unit == (i) in the construct is clearly unexpected.
>>>> Is this really still needed by the end of the series? I realize that
>>>> _some_ check is needed, but could this perhaps be arranged in a way
>>>> that you'd still hit all vCPU-s when using it on an idle unit, no
>>>> matter whether they're in use as a substitute in a "real" unit?
>>>
>>> I could do that by having another linked list in struct vcpu. This way
>>> I can avoid it.
>>
>> Oh, no, not another list just for this purpose. I was rather thinking
>> of e.g. a comparison of IDs.
> 
> That would result either in something like:
> 
> (v)->vcpu_id < (u)->unit_id + (u)->res->cpupool->granularity
> 
> requiring to make struct sched_resource public as keyhandler.c needs
> for_each_sched_unit_vcpu() plus being quite expensive,

I agree this is not a good option.

> or:
> 
> !(u)->next_in_list || (v)->vcpu_id < (u)->next_in_list->unit_id
> 
> which seems to be more expensive as the current variant, too.

It's not this much more expensive, and it eliminates unexpected
(as I would call it) behavior, so I think I'd go this route. Let's
see if others, in particular Dario or George, have an opinion
either way.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.