From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1425795AbcFHVBg (ORCPT ); Wed, 8 Jun 2016 17:01:36 -0400 Received: from mail-am1on0118.outbound.protection.outlook.com ([157.56.112.118]:27409 "EHLO emea01-am1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1162241AbcFHVBY (ORCPT ); Wed, 8 Jun 2016 17:01:24 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=rkagan@virtuozzo.com; Date: Thu, 9 Jun 2016 00:01:13 +0300 From: Roman Kagan To: Borislav Petkov CC: Minfei Huang , , "Denis V. Lunev" , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , , Andy Lutomirski , Paolo Bonzini Subject: Re: [PATCH] x86:pvclock: add missing barriers Message-ID: <20160608210112.GA6735@rkaganip> Mail-Followup-To: Roman Kagan , Borislav Petkov , Minfei Huang , linux-kernel@vger.kernel.org, "Denis V. Lunev" , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , x86@kernel.org, Andy Lutomirski , Paolo Bonzini References: <1465409499-23166-1-git-send-email-rkagan@virtuozzo.com> <20160608194509.GE4094@pd.tnic> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20160608194509.GE4094@pd.tnic> User-Agent: Mutt/1.6.1 (2016-04-27) X-Originating-IP: [2a02:2168:e14:d800:4ad2:24ff:fec3:31e0] X-ClientProxiedBy: VI1PR06CA0033.eurprd06.prod.outlook.com (10.162.116.171) To AM5PR0801MB1250.eurprd08.prod.outlook.com (10.167.216.137) X-MS-Office365-Filtering-Correlation-Id: dcf53cdf-76a9-439e-e658-08d38fe00a63 X-Microsoft-Exchange-Diagnostics: 1;AM5PR0801MB1250;2:zsVnpVjJnPeircQEkWbb7XUu96S535a9e0JaEu0VrwrdeQW3HmpbNkcFpzteOvYma1fEV0Coqysl8rLDfsdYst25iCed36UJ/k+0voht0jEbt7adM2VyLOqU8Wt+ibBwGm1Jw6f0rhprloaXAf+AwWigjTkPgs64xVI6a3KdWUrb0HZ1j9aHdw9Qo1E0EOpT;3:pWMJAqRhykcmW3eMz7cuEaQelQIhY5aUpTN0eZ/49LMKSLwW96a19D6DjXY59/EG3UarK3+NNDYi5eooDiJJepIzTvUy5pFUs0YBSpc8N49OoiyY9FEcvnk0r4f8zFwF X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:AM5PR0801MB1250; X-Microsoft-Exchange-Diagnostics: 1;AM5PR0801MB1250;25:DEyyUnFXgJPIDPSAUGDjBxHxtBCytVz/iC0rPwvno6BFXp9qHULi3YjbouOAuHddVOwUMi1UsVSUi7VyAS3mvTdHlZx6pLphSMSlR+UYRW6j9QJVGCNIqQsqFmuxi23OaYM68GPSmH3zEEfi+9xxyGyBCvKttKox7d2H4z9IML95amqhtsYIFM7ttUzajH21tuyDBPt+2FbS0fcJ7m1NBEQ+c+s5sfTP38XOgph/Wrwts+cXyVXUWkcsv6jUF8Y8fWc5KQoMdQDydYUN7rG9F2CQZ0bpfIp5JaTIbvo6HJDx+q1F8TLGfOdGljY6tCHJDltNQZ2v/Fs1D4L+BH6vuBGCL40mz3lLxO+vJqMySNXNgfuGRGTG1orcGEs6GQffdeICoAf4/6nD3CVIl6OCSldRhZ34/efqNkwZQjvoYusNcrP5IBN9qq24rxDJUDDyJihoE+g7wt42bijnESxa58O2lY9AvhZmM5lq+cI9YHzqs+Of5kSGdjf1Ck9FthDoBsBcvwoHmm7PUbIcBH2JZMvgex5wkESUuKqQwo3vmJ2XuZ6EtqFDD/uYIn3RG/l8IfB/5mZ/07z+Xz6qjaIfEYfj8bRSxc+EwcXq/PN9IJ+JC/JdT0PvBEwbozDDn31p3saz82qqSJIghcqCahfmtxtY1SMCyCxU3hRXDLQA5fDDrkgwLib27fWC5UikEAtKrJJsiEFGNolg+4Y/+1fYyUpQtbEJDvTh+tWwfmGATeE4eMFa9El2E5Hkocso49FO6rPjytD1fo5dtpWH7zGYpzHok232Y4SjkNP31aQRr1o= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(9452136761055)(42068640409301); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040130)(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001)(6041072)(6043046);SRVR:AM5PR0801MB1250;BCL:0;PCL:0;RULEID:;SRVR:AM5PR0801MB1250; X-Microsoft-Exchange-Diagnostics: 1;AM5PR0801MB1250;4:lzHSxf35vDbg0hwh4k6ITKq1BOsO03+843NtrrDxeUgLYzyWeJPC98qsBpKDpGN4Duq2KEgqu50h18G0u4AvukrSVGQIqJAy5MjxX+k3E9s88BDAraP5oX0DKcU/5AAOSIbpOIAFudzZO1GahpxCkHB2MmG43fHIWjNaLtcxTAbWcUVjT+egY0gAZqEEScS3vRLEjeSUsCftVSlG0tHfshp8jPeIkA52n+Y5yI6IAsW/O87xt9hq8e8nUhxuZ/xVKr3wU64F2kB8spCWotJ/swJxD3ZL1uVFGJTMN0lT3F9mkP69MFvVyW/mvBrMMqC/QUWMyRSeKJl0hnWv95lpaIi/Ev5qEzbvBELFRBka+P4UArk5q/JK/kMbcUPeUOoEKRrjUMgAXSN8D4mJT6fOTBa34cCcpBMmd5VPjd0R9EjBSkC+uUQQJO/IdzwF0YxINBEMI0CzlGOyYAxiom4tPLM8UOpRULaTaVR51J47ir4wRjXpcirsvgwwEaNeW30Q X-Forefront-PRVS: 0967749BC1 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6009001)(189002)(51914003)(24454002)(199003)(1076002)(42186005)(68736007)(325944008)(97756001)(23726003)(9686002)(50466002)(105586002)(81166006)(575784001)(106356001)(86362001)(2906002)(6116002)(81156014)(2950100001)(8676002)(5004730100002)(19580395003)(97736004)(83506001)(19580405001)(4326007)(586003)(5008740100001)(54356999)(15975445007)(50986999)(76176999)(110136002)(101416001)(77096005)(46406003)(92566002)(4001350100001)(33716001)(33656002)(47776003)(189998001)(3826002);DIR:OUT;SFP:1102;SCL:1;SRVR:AM5PR0801MB1250;H:rkaganip;FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1;AM5PR0801MB1250;23:kn7KN9WWBni0urC43zIMy23FlxboikCnFmThHtp?= =?us-ascii?Q?PSQ3iK1OvtGYqtBms6WO+IlXp4bcbPYRpHvfgtkjWQ4EckflA45a4BgHuOau?= =?us-ascii?Q?j+LzGV6EviYjUxrJ0jqmqt4nlpSIXWXJwC6WpgDA8zFrKWRir4zzjmgpFGNZ?= =?us-ascii?Q?mcbRdJpjfOum6Zu50IoeALMF79+GBWl8sAeU0gFVrMwuNTL0X6zVlUoWi5Uu?= =?us-ascii?Q?A7gEU0b4MxjZ0x+VeGNOZfgiSzp+jwvcm7YilOWNMXoKCMynRmvX3xca/4sd?= =?us-ascii?Q?Ryh0WfN0Bevo/70xKYlWXiZi+lbHumAJpoqhN/KrV6TT96Wh8XgV8E3vpA0y?= =?us-ascii?Q?fELabNmcNSAGN40W+ECakqH7CG2hv6Si/hbLqTCa14CKhkfq52NVfNdEtDwE?= =?us-ascii?Q?f9RK4JiiRWTqDKiJjZraPLFu9j82KxUkXDfw1ujst1VtotV4SWLnV1vq00jG?= =?us-ascii?Q?/r2RygT5buzGQ/OdN0JOCQ1nF4xx/sTNjQBgXwyr1lMtCor8p5MgdriWDcIL?= =?us-ascii?Q?x1SJ4hRbbLsuNQiT9FvaNQnw5G4+xrgLPW+B3GDaCBeQeiAt0zpdYVbISRJP?= =?us-ascii?Q?jN7Qq4YkDgCfXsrWPNhJYynA1od+4YUXpyrUFPQu8qWhBZZJAF4cGDCVrH94?= =?us-ascii?Q?+vW2DzHj4f7v4c7KTzV2qQsOVVD+ForgYY5xnR9Xi2Tj9B0LPGSfkCW0xrzq?= =?us-ascii?Q?0chgrFno32nsA7Gedw2rsq0sum77ReO5uDpuKwkrM8H+ti5jOrs1CdUHyQ0N?= =?us-ascii?Q?Qb9fmePyAHsvJ49TKYuxwT3n0bAl/4m7BKK+jpg/5dUZ4C7Rrk/8UkH68rOQ?= =?us-ascii?Q?nAHNBc00dpR4EFPvCd83+rqlx6TT4PfoKRFtSgSpIU/mWJyAHYRaWMfRdRkr?= =?us-ascii?Q?wcGDkGyh4a2W3JgpRyg3fGQEQXFJfJghdhExrx8jOsxkn0U/9kOidMVneoTX?= =?us-ascii?Q?Or0C0Tgj9D3BceB6BWSgi6mFER4LbKJqSrxpGh9bb0aSDyLFlEOC0mUDOG3K?= =?us-ascii?Q?L6ISriZnoD2JrUNQghQ1tQBXSkUqJzM//PJuaTZnul1KkqWyAJgrjqeqC6VW?= =?us-ascii?Q?8Mc/IE45As94S7HPwPhJePmtXifikqr83ZE8HGSDI6QBwtt51JZIrCPDvxlO?= =?us-ascii?Q?wWDLFXkU5oe7OkpTlf+rGSSFbojH8pfRqL4rPaj2nqprsUpwrnESPM4+kve2?= =?us-ascii?Q?FqCvp5gAzYfypRgx+7Mx1za33QIfMMTlGkrE9?= X-Microsoft-Exchange-Diagnostics: 1;AM5PR0801MB1250;5:j1VaHXWtoCS2Znqp8bX568shBpwCMCMCmhovS3QClAADI8vqRSdhpx2ltGXhHjXEYea1/dKWx7eol+SrsMS04MoJ6pG+nm2BdG+YdfDh1Bl/ZvLCvEfIgmpeN8Y2nAZN9OjM2YJ/f/HiukBWKWbQMQ==;24:hpxfuzg7gahM9wPF1fiIdoOPspRBZ6Lu5jx23H6pyu0xQSfFVAaBkWaP0mr91fKspQBvw6KVVsVKZdvP/oxqZqFCaLb/Es4MSVqFt+EwKwg=;7:sQxyb9+0Ju9oo0ckl1DcsNjRDirM542QrgdgG5V9JmWSR9Joa2KpGRoK48qS4QEy7QB5No+I+9M6qMa2aaz+rmVtp0xZmY3lFHzuSczOTN9IcE13aHTVAXMn5rLk2LHiBA0nUTQmhUHTadEzrhon207AT91dxVQ2rmx6bdAYlypctNKA4NHrlK74+L2+8xqvpjlBKfp7VmIDFJhTIMx71ykJMJoOZcEo6kSjqwG1KSM=;20:Bu9bNfIRSTC9VS+VoLD/x+yD2rMVl6xDZCkyJ8Igs26cLcsmKTxFpilxOiWJ27Q2lyJKvfO1VIDigOke9to3xtYBh1gK8asKK/1aRukCoLFWK4CLUt5RUnZfetzlowA2bU62AOmbFhzGp8D+4/sqUFI9Ia/1ALXwU3n+1QaqP04= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: virtuozzo.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Jun 2016 21:01:18.9137 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM5PR0801MB1250 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jun 08, 2016 at 09:45:09PM +0200, Borislav Petkov wrote: > On Wed, Jun 08, 2016 at 09:11:39PM +0300, Roman Kagan wrote: > > 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 > > Cc: Thomas Gleixner > > Cc: Ingo Molnar > > Cc: "H. Peter Anvin" > > Cc: x86@kernel.org > > Cc: Andy Lutomirski > > Cc: Borislav Petkov > > Cc: Paolo Bonzini > > 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; > > I have a similar patchset in my mbox starting here: > > https://lkml.kernel.org/r/1464329832-4638-1-git-send-email-mnghuan@gmail.com > > Care to take a look? Just did, thanks for the link. The difference is whether you want the reader to see consistent view of the pvclock data (as in my patch) or also the most up to date one (as in Minfei Huang's patch) at the cost of extra lfence instructions (on my machine this is 30% slowdown). I'm not sure if the latter is really necessary. If it is, then the lfence or mfence in rdtsc_ordered() becomes excessive. Perhaps we'd have to revert to rdtsc_barrier() to surround pvclock data access, and plain rdtsc() instead of rdtsc_ordered(). Roman.