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-next>] [day] [month] [year] [list]
Message-Id: <20181011003336.168941-1-edumazet@google.com>
Date:   Wed, 10 Oct 2018 17:33:36 -0700
From:   Eric Dumazet <edumazet@...gle.com>
To:     linux-kernel <linux-kernel@...r.kernel.org>
Cc:     Eric Dumazet <edumazet@...gle.com>,
        Eric Dumazet <eric.dumazet@...il.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>
Subject: [PATCH] x86/tsc: use real seqcount_latch in cyc2ns_read_begin()

While looking at native_sched_clock() disassembly I had
the surprise to see the compiler (gcc 7.3 here) had
optimized out the loop, meaning the code is broken.

Using the documented and approved API not only fixes the bug,
it also makes the code more readable.

Replacing five this_cpu_read() by one this_cpu_ptr() makes
the generated code smaller.

Fixes: 59eaef78bfea ("x86/tsc: Remodel cyc2ns to use seqcount_latch()")
Signed-off-by: Eric Dumazet <edumazet@...gle.com>
Cc: Peter Zijlstra (Intel) <peterz@...radead.org>
Cc: Thomas Gleixner <tglx@...utronix.de>
Cc: Ingo Molnar <mingo@...hat.com>
Cc: "H. Peter Anvin" <hpa@...or.com>
---
 arch/x86/kernel/tsc.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index b52bd2b6cdb443ba0c89d78aaa52b02b82a10b6e..b039ae0f358a4f78cc830c069ad90e4fb108489e 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -60,19 +60,16 @@ static DEFINE_PER_CPU_ALIGNED(struct cyc2ns, cyc2ns);
 
 void cyc2ns_read_begin(struct cyc2ns_data *data)
 {
-	int seq, idx;
+	const struct cyc2ns *c2n;
+	int seq;
 
 	preempt_disable_notrace();
 
+	c2n = this_cpu_ptr(&cyc2ns);
 	do {
-		seq = this_cpu_read(cyc2ns.seq.sequence);
-		idx = seq & 1;
-
-		data->cyc2ns_offset = this_cpu_read(cyc2ns.data[idx].cyc2ns_offset);
-		data->cyc2ns_mul    = this_cpu_read(cyc2ns.data[idx].cyc2ns_mul);
-		data->cyc2ns_shift  = this_cpu_read(cyc2ns.data[idx].cyc2ns_shift);
-
-	} while (unlikely(seq != this_cpu_read(cyc2ns.seq.sequence)));
+		seq = raw_read_seqcount_latch(&c2n->seq);
+		*data = c2n->data[seq & 1];
+	} while (read_seqcount_retry(&c2n->seq, seq));
 }
 
 void cyc2ns_read_end(void)
-- 
2.19.0.605.g01d371f741-goog

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ