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

Re: [Xen-devel] [PATCH RFC V2 42/45] xen/sched: add fall back to idle vcpu when scheduling item



>>> On 17.05.19 at 09:48, <jgross@xxxxxxxx> wrote:
> On 17/05/2019 08:57, Jan Beulich wrote:
>>>>> On 17.05.19 at 07:13, <jgross@xxxxxxxx> wrote:
>>> On 16/05/2019 16:41, Jan Beulich wrote:
>>>>>>> On 16.05.19 at 15:51, <jgross@xxxxxxxx> wrote:
>>>>> As with core scheduling we can be sure the other thread is active
>>>>> (otherwise we would schedule the idle item) and hoping for saving power
>>>>> by using mwait is moot.
>>>>
>>>> Saving power may be indirect, by the CPU re-arranging
>>>> resource assignment between threads when one goes idle.
>>>> I have no idea whether they do this when entering C1, or
>>>> only when entering deeper C states.
>>>
>>> SDM Vol. 3 chapter 8.10.1 "HLT instruction":
>>>
>>> "Here shared resources that were being used by the halted logical
>>> processor become available to active logical processors, allowing them
>>> to execute at greater efficiency."
>> 
>> To be honest, this is to broad/generic a statement to fully
>> trust it, judging from other areas of the SDM. And then, as
>> per above, what about the socket granularity case? Putting
>> entirely idle cores to sleep is surely worthwhile?
> 
> Yes, I assume it is. OTOH this might affect context switches badly
> as the reaction time for the coordinated switch will rise. Maybe a
> good reason for another sub-option?

While I agree that fine grained control is useful, I'm seeing an
increasing risk of there going to be too many controls to actually
be certain in the end that all possible combinations work
correctly.

>>>> And anyway - I'm still none the wiser as to the v->is_urgent
>>>> relationship.
>>>
>>> With v->is_urgent set today's idle loop will drop into default_idle().
>>> I can remove this sentence in case it is just confusing.
>> 
>> I'd prefer if the connection would become more obvious. One
>> needs to go from ->is_urgent via ->urgent_count to
>> sched_has_urgent_vcpu() to find where the described
>> behavior really lives.
>> 
>> What's worse though: This won't work as intended on AMD
>> at all. I don't think it's correct to fall back to default_idle() in
>> this case. Instead sched_has_urgent_vcpu() returning true
>> should amount to the same effect as max_cstate being set
>> to 1. There's
>> (a) no reason not to use MWAIT on Intel CPUs in this case,
>> if MWAIT can enter C1, and
>> (b) a strong need to use MWAIT on (at least) AMD Fam17,
>> or else it won't be C1 that gets entered.
>> I'll see about making a patch in due course.
> 
> Thanks. Would you mind doing it in a way that the caller can specify
> max_cstate? This would remove the need to call sched_has_urgent_vcpu()
> deep down the idle handling and I could re-use it for my purpose.

Hmm, to be honest I'm not fancying giving a parameter to
default_idle(), pm_idle(), and friends. Conceptually it is not
the business of the callers to control the C states to be used.

What about the opposite: You simply mark idle (v)CPUs in
question as "urgent", thus achieving the intended effect as
well.

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®.