Re: [PATCH v3] sched/core: Skip rq->avg_idle update without a valid idle_stamp

[Zhan Xusheng]

The diff of 4b603f1551a73 is its own evidence, which may be quicker to
look at than the description:

  -	if (rq->idle_stamp) {
  -		u64 delta = rq_clock(rq) - rq->idle_stamp;
  +void update_rq_avg_idle(struct rq *rq)
  +{
  +	u64 delta = rq_clock(rq) - rq->idle_stamp;

The trigger moved on purpose, from the wakeup to idle switch-out.  The
guard came off with it, and nothing about that fails to compile.

Two things about it that the changelog leaves narrower than they are.

rq->idle_stamp has exactly one setter, fair.c:14563, and it sits below the
ttwu_pending bail-out at 14555.  So the reachable set is not the three
examples but any path into idle that misses that one line, and since
update_rq_avg_idle() zeroes the field on the way out, every such entry
starts from zero again.

The sample is not merely invalid, it saturates on the first hit.
update_avg() adds diff/8, so a rq_clock() of ~1e12 on a few minutes of
uptime moves avg_idle by ~1.25e11, against a clamp of
2*max_idle_balance_cost in the 1e4..1e5 range.  Both places that consult
avg_idle to decide whether an idle interval was long enough to be worth
balancing, fair.c:14504 and 14584, then stop holding anything back.  It
only ever adds newidle balancing, never removes it, which fits latency
reports rather than wrong results.

I also went looking for a stale stamp rather than a zero one and did not
find one: 14674 clears it when newidle actually pulled a task, so no idle
follows, and the other write is sched_init().  The field is either zero or
this idle period's rq_clock, which is what the restored guard splits on.

Reviewed-by: Zhan Xusheng <zhanxusheng@xxxxxxxxxx>

Thanks,
Zhan Xusheng