All of lore.kernel.org
 help / color / mirror / Atom feed
From: Roman Kagan <rkagan@virtuozzo.com>
To: <linux-kernel@vger.kernel.org>
Cc: "Denis V. Lunev" <den@openvz.org>,
	Roman Kagan <rkagan@virtuozzo.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	<x86@kernel.org>, Andy Lutomirski <luto@kernel.org>,
	Borislav Petkov <bp@suse.de>, Paolo Bonzini <pbonzini@redhat.com>,
	<stable@vger.kernel.org>
Subject: [PATCH] x86:pvclock: add missing barriers
Date: Wed, 8 Jun 2016 21:11:39 +0300	[thread overview]
Message-ID: <1465409499-23166-1-git-send-email-rkagan@virtuozzo.com> (raw)

Gradual removal of excessive barriers in pvclock reading functions
(commits 502dfeff239e8313bfbe906ca0a1a6827ac8481b,
a3eb97bd80134ba07864ca00747466c02118aca1) ended up removing too much:
although rdtsc is now orderd WRT other loads, there's no protection
against the compiler reordering the loads of ->version with the loads of
other fields.

E.g. on my system gcc-5.3.1 generates code which loads ->system_time and
->flags outside of the ->version test loop.

(Re)introduce the compiler barriers around accesses to the contents of
pvclock.  While at this, make the function a bit more compact by
removing unnecessary local variables.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@suse.de>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: stable@vger.kernel.org
---
 arch/x86/include/asm/pvclock.h | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/arch/x86/include/asm/pvclock.h b/arch/x86/include/asm/pvclock.h
index fdcc040..65c4de2 100644
--- a/arch/x86/include/asm/pvclock.h
+++ b/arch/x86/include/asm/pvclock.h
@@ -80,18 +80,11 @@ static __always_inline
 unsigned __pvclock_read_cycles(const struct pvclock_vcpu_time_info *src,
 			       cycle_t *cycles, u8 *flags)
 {
-	unsigned version;
-	cycle_t ret, offset;
-	u8 ret_flags;
-
-	version = src->version;
-
-	offset = pvclock_get_nsec_offset(src);
-	ret = src->system_time + offset;
-	ret_flags = src->flags;
-
-	*cycles = ret;
-	*flags = ret_flags;
+	unsigned version = src->version;
+	barrier();
+	*cycles = src->system_time + pvclock_get_nsec_offset(src);
+	*flags = src->flags;
+	barrier();
 	return version;
 }
 
-- 
2.5.5

             reply	other threads:[~2016-06-08 19:44 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-08 18:11 Roman Kagan [this message]
2016-06-08 19:45 ` [PATCH] x86:pvclock: add missing barriers Borislav Petkov
2016-06-08 21:01   ` Roman Kagan
2016-06-09 10:29     ` Roman Kagan
2016-06-12 10:02   ` Minfei Huang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1465409499-23166-1-git-send-email-rkagan@virtuozzo.com \
    --to=rkagan@virtuozzo.com \
    --cc=bp@suse.de \
    --cc=den@openvz.org \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=stable@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.