Re: [External] Re: [PATCH 2/7] sched/fair: prepare throttle path for task based throttle

[Chengming Zhou <chengming.zhou@linux.dev>]

On 2025/5/21 17:21, Aaron Lu wrote:
> On Wed, May 21, 2025 at 05:01:58PM +0800, Chengming Zhou wrote:
>> On 2025/5/20 18:41, Aaron Lu wrote:
>>> From: Valentin Schneider <vschneid@redhat.com>
>>>
>>> In current throttle model, when a cfs_rq is throttled, its entity will
>>> be dequeued from cpu's rq, making tasks attached to it not able to run,
>>> thus achiveing the throttle target.
>>>
>>> This has a drawback though: assume a task is a reader of percpu_rwsem
>>> and is waiting. When it gets wakeup, it can not run till its task group's
>>> next period comes, which can be a relatively long time. Waiting writer
>>> will have to wait longer due to this and it also makes further reader
>>> build up and eventually trigger task hung.
>>>
>>> To improve this situation, change the throttle model to task based, i.e.
>>> when a cfs_rq is throttled, record its throttled status but do not remove
>>> it from cpu's rq. Instead, for tasks that belong to this cfs_rq, when
>>> they get picked, add a task work to them so that when they return
>>> to user, they can be dequeued. In this way, tasks throttled will not
>>> hold any kernel resources.
>>>
>>> To avoid breaking bisect, preserve the current throttle behavior by
>>> still dequeuing throttled hierarchy from rq and because of this, no task
>>> can have that throttle task work added yet. The throttle model will
>>> switch to task based in a later patch.
>>>
>>> Suggested-by: Chengming Zhou <chengming.zhou@linux.dev> # tag on pick
>>> Signed-off-by: Valentin Schneider <vschneid@redhat.com>
>>> Signed-off-by: Aaron Lu <ziqianlu@bytedance.com>
>>
>> I'm wondering how about put 02-04 patches together, since it's strange
>> to setup task work in this patch but without changing throttle_cfs_rq(),
> 
> Do you mean 02-05?
> Because the actual change to throttle_cfs_rq() happens in patch5 :)

Ah, right.

> 
>> which makes the reviewing process a bit confused? WDYT?
> 
> Yes, I agree it looks a bit confused.
> 
> The point is to not break bisect while make review easier; if merging
> all task based throttle related patches together, that would be to put
> patch 02-05 together, which seems too big?

Yeah, a big patch but complete, instead of changing a bit on the same
function in each patch. Anyway, it's your call :-)

Thanks!

> 
> Thanks,
> Aaron
>