lkml.org 
[lkml]   [2023]   [Sep]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [External] Re: [PATCH 0/2] Fix nohz_full vs rt bandwidth
From


On 2023/9/8 Phil Auld wrote:
> On Fri, Sep 08, 2023 at 10:57:26AM +0800 Hao Jia wrote:
>>
>>
>> On 2023/9/7 Phil Auld wrote:
>>> Hi Hao,
>>>
>>> On Wed, Sep 06, 2023 at 02:45:39PM +0800 Hao Jia wrote:
>>>>
>>>> Friendly ping...
>>>>
>>>> On 2023/8/21 Hao Jia wrote:
>>>>> Since the commit 88c56cfeaec4 ("sched/fair: Block nohz tick_stop
>>>>> when cfs bandwidth in use") was merged, it handles conflicts between
>>>>> NOHZ full and cfs_bandwidth well, and the scheduler feature HZ_BW
>>>>> allows us to choose which one to prefer.
>>>>>
>>>>> This conflict also exists between NOHZ full and rt_bandwidth,
>>>>> these two patches try to handle it in a similar way.
>>>>>
>>>
>>> Are you actually hitting this in the real world?
>>>
>>> We, for example, no longer enable RT_GROUP_SCHED so this is a non-issue
>>> for our use cases. I'd recommend considering that. (Does it even
>>> work with cgroup2?)
>>>
>>
>> Yes, it has always been there. Regardless of whether RT_GROUP_SCHED is
>> enabled or not, rt bandwidth is always enabled. If RT_GROUP_SCHED is not
>> enabled, all rt tasks in the system are a group, and rt_runtime is 950000,
>> and rt_period is 1000000.So rt bandwidth is always enabled by default.
>
> Sure, there is that. But I think Daniel is actively trying to remove it.
>

Thank you for your reply. Maybe I'm missing something. Can you give me
some links to discussions about it?

> Also I'm not sure you answered my question. Are you actually hitting this
> in the real world? I'd be tempted to think this is a mis-configuration or
> mis-use of RT. Plus you can disable that throttling and use stalld to catch
> cases where the rt task goes out of control.
>

> Are you actually hitting this in the real world?

I tested on my machine using default settings (rt_runtime is 950000, and
rt_period is 1000000.). The rt task is supposed to be throttled after
running for 0.95 seconds, but due to the influence of NO_HZ_FULL, it may
be throttled after running for about 1.4 seconds. This will only cause
the rt_bandwidth throttle to be delayed, but no warning will be triggered.


> Plus you can disable that throttling and use stalld to catch cases
where the rt task goes out of control.

IIRC, if we disable rt_bandwidth. The rt task is always running, which
may cause cfs task starvation and hung_task warnning. This may be the
reason why rt_bandwidth is enabled by default (rt_runtime is 950000, and
rt_period is 1000000).


Thanks,
Hao

> I'm not totally against doing this (for what my vote counts...), I just
> wonder if it's really needed. It seem it may be over-engineering something
> that is soon to be a non-problem.
>
>
> Cheers,
> Phil
>
>>
>>
>> Thanks,
>> Hao
>>
>>> In some ways what you have is a simplification of code, but it also
>>> obfuscates the stop_tick conditions by hiding them all in the class
>>> specific functions. It was easier to see why the tick didn't stop
>>> looking at the original code.
>>>
>>> It would be better to do this only if it is really needed, in my opinion.
>>>
>>>
>>> Cheers,
>>> Phil
>>>
>>>>> patch1: Extracts a can_stop_tick() callback function for each
>>>>> sched_class from sched_can_stop_tick(), it will make things clearer
>>>>> and also convenient to handle the conflict between NOHZ full
>>>>> and rt_bandwidth.
>>>>>
>>>>> patch2: If the HZ_BW scheduler feature is enabled, and the RT task
>>>>> to be run is constrained by rt_bandwidth runtime. Then it will
>>>>> prevent NO_HZ full from stopping tick.
>>>>>
>>>>> Hao Jia (2):
>>>>> sched/core: Introduce sched_class::can_stop_tick()
>>>>> sched/rt: Block nohz tick_stop when rt bandwidth in use
>>>>>
>>>>> kernel/sched/core.c | 67 +++++--------------------------
>>>>> kernel/sched/deadline.c | 16 ++++++++
>>>>> kernel/sched/fair.c | 56 +++++++++++++++++++++++---
>>>>> kernel/sched/rt.c | 89 ++++++++++++++++++++++++++++++++++++++++-
>>>>> kernel/sched/sched.h | 5 ++-
>>>>> 5 files changed, 168 insertions(+), 65 deletions(-)
>>>>>
>>>>
>>>
>>
>

\
 
 \ /
  Last update: 2023-09-11 05:39    [W:0.127 / U:0.516 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site