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]

Date:	Sun, 20 Oct 2013 09:34:56 +0200
From:	Andreas Mohr <andi@...as.de>
To:	Frederic Weisbecker <fweisbec@...il.com>
Cc:	Oleg Nesterov <oleg@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	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

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)

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

Unless I missed some other restrictions which cause us to need to cling
to that manual from-us conversion, for now or forever...

Andreas Mohr
--
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/