Re: [PATCH v5 07/10] sched/irq: add irq utilization tracking
From: Dietmar Eggemann
Date: Thu Jun 07 2018 - 04:29:26 EST
On 06/06/2018 06:06 PM, Vincent Guittot wrote:
Hi Dietmar,
Sorry for the late answer
On 31 May 2018 at 18:54, Dietmar Eggemann <dietmar.eggemann@xxxxxxx> wrote:
On 05/30/2018 08:45 PM, Vincent Guittot wrote:
Hi Dietmar,
On 30 May 2018 at 17:55, Dietmar Eggemann <dietmar.eggemann@xxxxxxx> 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):
To be honest, I prefer to keep the specific sequence above in a
dedicated function instead of adding it in core code.
No big issue.
Furthermore, we end up calling call twice ___update_load_avg instead
of only once. This will set an intermediate and incorrect value in
util_avg and this value can be read in the meantime
Can't buy this argument though because this is true with the current
implementation as well since the 'decay load sum' - 'accrue load sum'
sequence is not atomic.
What about calling update_irq_load_avg(rq, 0) in update_rq_clock_task()
if (irq_delta + steal) eq. 0 and sched_feat(NONTASK_CAPACITY) eq. true
in this #ifdef CONFIG_XXX_TIME_ACCOUNTING block?
Maintaining a irq/steal time signal makes only sense if at least one of
the CONFIG_XXX_TIME_ACCOUNTING is set and NONTASK_CAPACITY is true. The
call to update_irq_load_avg() in update_blocked_averages() isn't guarded
my them.
[...]