linux-kernel - Re: [PATCH 3/5] timer: Change idle/iowait accounting semantics

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20131104162253.GG9299@localhost.localdomain>
Date:	Mon, 4 Nov 2013 17:22:55 +0100
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Andreas Mohr <andi@...as.de>
Cc:	Oleg Nesterov <oleg@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Fernando Luis Vazquez Cao <fernando_b1@....ntt.co.jp>,
	Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...nel.org>
Subject: Re: [PATCH 3/5] timer: Change idle/iowait accounting semantics

On Sun, Oct 20, 2013 at 09:34:56AM +0200, Andreas Mohr wrote:
> Hi,
> 
> just wanted to report that this capricious open-coded (ok, lone-coded :)
> converter:
> 
> +static inline ktime_t us_to_ktime(u64 us)
> +{
> +	static const ktime_t ktime_zero = { .tv64 = 0 };
> +
> +	return ktime_add_us(ktime_zero, us);
> +}
> 
> triggered a virtual red flag in my processing.
> 
> We obviously seem to be compensating for a domain mismatch here:
> 
> +	iowait = get_cpu_iowait_time_us(cpu, NULL);
> +
> +	if (ktime_compare(idle, us_to_ktime(iowait)) > 0)
> +		idle = ktime_sub_us(idle, iowait);
> +
> 
> And sure enough:
> get_cpu_iowait_time_us() does a final to-us conversion
> only right before result is returned,
> and it's located right within the same build unit (tick-sched.c).
> (and then we go ahead and do a from-us round trip :-P)
> 
> So, in case we want to standardize on ktime_t domain
> for these parts of purely *in-kernel* time handling,
> how about adding a _unit-local_ helper for providing a ktime-only result
> and convert get_cpu_iowait_time_us() into a simple to-us *external user code*
> one-line wrapper for it?
> (and include updates to all other places which would benefit from this change)

Agreed. I admit I worked around this little part because that code is actually
not expected to be merged but was rather posted to raise some questions and may be
answer. But what you're suggesting is definetly the right way.


> And preferably launch such patch as a preparatory patch for this
> subsequent 3/5 patch, within the series?

Yep. Well, I hope we can find another way to fix the initial issue. But if that part of the code
is reused, I'll take care of fixing that.

Thanks!

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/