Re: [PATCH v5 07/10] sched/irq: add irq utilization tracking

Dietmar Eggemann
2018-05-31T16:55:05Z

On 05/30/2018 08:45 PM, Vincent Guittot wrote:
> Hi Dietmar,
>
> On 30 May 2018 at 17:55, Dietmar Eggemann <dietmar.eggemann@arm.com> wrote:
>> On 05/25/2018 03:12 PM, Vincent Guittot wrote:

[...]

>>> + */
>>> + ret = ___update_load_sum(rq->clock - running, rq->cpu,
>>> &rq->avg_irq,
>>> + 0,
>>> + 0,
>>> + 0);
>>> + ret += ___update_load_sum(rq->clock, rq->cpu, &rq->avg_irq,
>>> + 1,
>>> + 1,
>>> + 1);

Can you not change the function parameter list to the usual
(u64 now, struct rq *rq, int running)?

Something like this (only compile and boot tested):

-- >8 --

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 9894bc7af37e..26ffd585cab8 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -177,8 +177,22 @@ static void update_rq_clock_task(struct rq *rq, s64 delta)
rq->clock_task += delta;

#if defined(CONFIG_IRQ_TIME_ACCOUNTING) || defined(CONFIG_PARAVIRT_TIME_ACCOUNTING)
- if ((irq_delta + steal) && sched_feat(NONTASK_CAPACITY))
- update_irq_load_avg(rq, irq_delta + steal);
+ if ((irq_delta + steal) && sched_feat(NONTASK_CAPACITY)) {
+ /*
+ * We know the time that has been used by interrupt since last
+ * update but we don't when. Let be pessimistic and assume that
+ * interrupt has happened just before the update. This is not
+ * so far from reality because interrupt will most probably
+ * wake up task and trig an update of rq clock during which the
+ * metric si updated.
+ * We start to decay with normal context time and then we add
+ * the interrupt context time.
+ * We can safely remove running from rq->clock because
+ * rq->clock += delta with delta >= running
+ */
+ update_irq_load_avg(rq_clock(rq) - (irq_delta + steal), rq, 0);
+ update_irq_load_avg(rq_clock(rq), rq, 1);
+ }
#endif
}

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 1bb3379c4b71..a245f853c271 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7363,7 +7363,7 @@ static void update_blocked_averages(int cpu)
}
update_rt_rq_load_avg(rq_clock_task(rq), rq, 0);
update_dl_rq_load_avg(rq_clock_task(rq), rq, 0);
- update_irq_load_avg(rq, 0);
+ update_irq_load_avg(rq_clock(rq), rq, 0);
/* Don't need periodic decay once load/util_avg are null */
if (others_rqs_have_blocked(rq))
done = false;
@@ -7434,7 +7434,7 @@ static inline void update_blocked_averages(int cpu)
update_cfs_rq_load_avg(cfs_rq_clock_task(cfs_rq), cfs_rq);
update_rt_rq_load_avg(rq_clock_task(rq), rq, 0);
update_dl_rq_load_avg(rq_clock_task(rq), rq, 0);
- update_irq_load_avg(rq, 0);
+ update_irq_load_avg(rq_clock(rq), rq, 0);
#ifdef CONFIG_NO_HZ_COMMON
rq->last_blocked_load_update_tick = jiffies;
if (!cfs_rq_has_blocked(cfs_rq) && !others_rqs_have_blocked(rq))
diff --git a/kernel/sched/pelt.c b/kernel/sched/pelt.c
index d2e4f2186b13..ae01bb18e28c 100644
--- a/kernel/sched/pelt.c
+++ b/kernel/sched/pelt.c
@@ -365,31 +365,15 @@ int update_dl_rq_load_avg(u64 now, struct rq *rq, int running)
*
*/

-int update_irq_load_avg(struct rq *rq, u64 running)
+int update_irq_load_avg(u64 now, struct rq *rq, int running)
{
- int ret = 0;
- /*
- * We know the time that has been used by interrupt since last update
- * but we don't when. Let be pessimistic and assume that interrupt has
- * happened just before the update. This is not so far from reality
- * because interrupt will most probably wake up task and trig an update
- * of rq clock during which the metric si updated.
- * We start to decay with normal context time and then we add the
- * interrupt context time.
- * We can safely remove running from rq->clock because
- * rq->clock += delta with delta >= running
- */
- ret = ___update_load_sum(rq->clock - running, rq->cpu, &rq->avg_irq,
- 0,
- 0,
- 0);
- ret += ___update_load_sum(rq->clock, rq->cpu, &rq->avg_irq,
- 1,
- 1,
- 1);
-
- if (ret)
+ if (___update_load_sum(now, rq->cpu, &rq->avg_irq,
+ running,
+ running,
+ running)) {
___update_load_avg(&rq->avg_irq, 1, 1);
+ return 1;
+ }

- return ret;
+ return 0;
}
diff --git a/kernel/sched/pelt.h b/kernel/sched/pelt.h
index 0ce9a5a5877a..ebc57301a9a8 100644
--- a/kernel/sched/pelt.h
+++ b/kernel/sched/pelt.h
@@ -5,7 +5,7 @@ int __update_load_avg_se(u64 now, int cpu, struct cfs_rq *cfs_rq, struct sched_e
int __update_load_avg_cfs_rq(u64 now, int cpu, struct cfs_rq *cfs_rq);
int update_rt_rq_load_avg(u64 now, struct rq *rq, int running);
int update_dl_rq_load_avg(u64 now, struct rq *rq, int running);
-int update_irq_load_avg(struct rq *rq, u64 running);
+int update_irq_load_avg(u64 now, struct rq *rq, int running);

/*
* When a task is dequeued, its estimated utilization should not be update if