linux-kernel - Re: [PATCH 2/4] sched: Account per task_group nr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 6 Nov 2017 17:24:36 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Kirill Tkhai <ktkhai@...tuozzo.com>
Cc:     mingo@...hat.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/4] sched: Account per task_group nr_iowait

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 :-(