2022-08-24 06:26:02

by Shang XiaoJing

Subject: [PATCH -next] sched: Add update_current_exec_runtime helper

Wrap repeated code in helper function update_current_exec_runtime for
update the exec time of the current.

Signed-off-by: Shang XiaoJing <[email protected]>
---
kernel/sched/deadline.c | 6 +-----
kernel/sched/rt.c | 6 +-----
kernel/sched/sched.h | 10 ++++++++++
kernel/sched/stop_task.c | 6 +-----
4 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index daa9a7fb5917..d116d2b9d2f9 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -1333,11 +1333,7 @@ static void update_curr_dl(struct rq *rq)

trace_sched_stat_runtime(curr, delta_exec, 0);

- curr->se.sum_exec_runtime += delta_exec;
- account_group_exec_runtime(curr, delta_exec);
-
- curr->se.exec_start = now;
- cgroup_account_cputime(curr, delta_exec);
+ update_current_exec_runtime(curr, now, delta_exec);

if (dl_entity_is_special(dl_se))
return;
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index cd07c9367bc2..27e694cf3e37 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -1062,11 +1062,7 @@ static void update_curr_rt(struct rq *rq)

trace_sched_stat_runtime(curr, delta_exec, 0);

- curr->se.sum_exec_runtime += delta_exec;
- account_group_exec_runtime(curr, delta_exec);
-
- curr->se.exec_start = now;
- cgroup_account_cputime(curr, delta_exec);
+ update_current_exec_runtime(curr, now, delta_exec);

if (!rt_bandwidth_enabled())
return;
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index ddcfc7837595..7ad65f79cb59 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -3163,4 +3163,14 @@ extern int sched_dynamic_mode(const char *str);
extern void sched_dynamic_update(int mode);
#endif

+static inline void update_current_exec_runtime(struct task_struct *curr,
+ u64 now, u64 delta_exec)
+{
+ curr->se.sum_exec_runtime += delta_exec;
+ account_group_exec_runtime(curr, delta_exec);
+
+ curr->se.exec_start = now;
+ cgroup_account_cputime(curr, delta_exec);
+}
+
#endif /* _KERNEL_SCHED_SCHED_H */
diff --git a/kernel/sched/stop_task.c b/kernel/sched/stop_task.c
index d04073a93eb4..027068779126 100644
--- a/kernel/sched/stop_task.c
+++ b/kernel/sched/stop_task.c
@@ -80,11 +80,7 @@ static void put_prev_task_stop(struct rq *rq, struct task_struct *prev)
schedstat_set(curr->stats.exec_max,
max(curr->stats.exec_max, delta_exec));

- curr->se.sum_exec_runtime += delta_exec;
- account_group_exec_runtime(curr, delta_exec);
-
- curr->se.exec_start = rq_clock_task(rq);
- cgroup_account_cputime(curr, delta_exec);
+ update_current_exec_runtime(curr, rq_clock_task(rq), delta_exec);
}

/*
--
2.17.1

2022-08-24 06:45:05

by Peter Zijlstra

Subject: Re: [PATCH -next] sched: Add update_current_exec_runtime helper

On Wed, Aug 24, 2022 at 02:53:26PM +0800, Shang XiaoJing wrote:

In general I like, however:

> diff --git a/kernel/sched/stop_task.c b/kernel/sched/stop_task.c
> index d04073a93eb4..027068779126 100644
> --- a/kernel/sched/stop_task.c
> +++ b/kernel/sched/stop_task.c
> @@ -80,11 +80,7 @@ static void put_prev_task_stop(struct rq *rq, struct task_struct *prev)
> schedstat_set(curr->stats.exec_max,
> max(curr->stats.exec_max, delta_exec));
>
> - curr->se.sum_exec_runtime += delta_exec;
> - account_group_exec_runtime(curr, delta_exec);
> -
> - curr->se.exec_start = rq_clock_task(rq);
> - cgroup_account_cputime(curr, delta_exec);
> + update_current_exec_runtime(curr, rq_clock_task(rq), delta_exec);
> }

This already has a rq_clock_task() invocation; please fix it to call it
once -- pre-existing issue, but if we're cleaning up we should clean up,
right :-)

2022-08-24 08:04:53

by Shang XiaoJing

Subject: Re: [PATCH -next] sched: Add update_current_exec_runtime helper

On 2022/8/24 14:38, Peter Zijlstra wrote:
> On Wed, Aug 24, 2022 at 02:53:26PM +0800, Shang XiaoJing wrote:
>
> In general I like, however:
>
>> diff --git a/kernel/sched/stop_task.c b/kernel/sched/stop_task.c
>> index d04073a93eb4..027068779126 100644
>> --- a/kernel/sched/stop_task.c
>> +++ b/kernel/sched/stop_task.c
>> @@ -80,11 +80,7 @@ static void put_prev_task_stop(struct rq *rq, struct task_struct *prev)
>> schedstat_set(curr->stats.exec_max,
>> max(curr->stats.exec_max, delta_exec));
>>
>> - curr->se.sum_exec_runtime += delta_exec;
>> - account_group_exec_runtime(curr, delta_exec);
>> -
>> - curr->se.exec_start = rq_clock_task(rq);
>> - cgroup_account_cputime(curr, delta_exec);
>> + update_current_exec_runtime(curr, rq_clock_task(rq), delta_exec);
>> }
> This already has a rq_clock_task() invocation; please fix it to call it
> once -- pre-existing issue, but if we're cleaning up we should clean up,
> right :-)

Right, the redundant invocation of rq_clock_task() will be removed in v2.


Thanks :-) ,

Shang XiaoJing