Re: [PATCH 2/4] sched: Account per task_group nr_iowait

From: Peter Zijlstra
Date: Mon Nov 06 2017 - 11:24:44 EST


On Mon, Nov 06, 2017 at 07:12:58PM +0300, Kirill Tkhai wrote:

> >> + atomic_inc(&tg->stat[rq->cpu].nr_iowait);
> >
> > You're joking right, more atomic ops on the fast paths..
>
> There should be a synchronization... It's modified under rq->lock everywhere, except try_to_wakeup().
> Would it be better to use one more rq->lock at try_to_wakeup() instead of atomic?

No, of course not. We spend a lot of time getting of that rq->lock
there.

The better option is to not care about iowait, since its a complete
garbage number to begin with -- read that commit I pointed you to.

But if you do manage to convince me iowait is a sane thing to export
(and its not); then you should not use atomics -- nor is there any need
to. Since all you want to export is \Sum nr_iowait, you can inc/dec to
pure cpu local variables and the sum will make it all work.

The extant iowait crap cannot do this because it thinks per-cpu IO-wait
is a thing -- its not, its a random number at best, but its ABI so we
can't fix :-(