From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0E71CC43387 for ; Tue, 8 Jan 2019 18:51:05 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id DA31B20827 for ; Tue, 8 Jan 2019 18:51:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729523AbfAHSvD (ORCPT ); Tue, 8 Jan 2019 13:51:03 -0500 Received: from mx1.redhat.com ([209.132.183.28]:42178 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727947AbfAHSvD (ORCPT ); Tue, 8 Jan 2019 13:51:03 -0500 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8A6F589ACA; Tue, 8 Jan 2019 18:51:02 +0000 (UTC) Received: from redhat.com (ovpn-126-25.rdu2.redhat.com [10.10.126.25]) by smtp.corp.redhat.com (Postfix) with SMTP id 2CEA61A7D5; Tue, 8 Jan 2019 18:50:59 +0000 (UTC) Date: Tue, 8 Jan 2019 13:50:58 -0500 From: "Michael S. Tsirkin" To: Nick Desaulniers Cc: Miguel Ojeda , LKML , Jason Wang , Alan Stern , Andrea Parri , Will Deacon , Peter Zijlstra , Boqun Feng , Nicholas Piggin , David Howells , Jade Alglave , Luc Maranget , "Paul E. McKenney" , Akira Yokosawa , Daniel Lustig , linux-arch@vger.kernel.org, netdev@vger.kernel.org, virtualization@lists.linux-foundation.org, Eli Friedman , Joe Perches , Linus Torvalds , Luc Van Oostenryck , linux-sparse@vger.kernel.org, Eric Christopher Subject: Re: [PATCH RFC 1/4] include/linux/compiler*.h: fix OPTIMIZER_HIDE_VAR Message-ID: <20190108134549-mutt-send-email-mst@kernel.org> References: <20190102205715.14054-1-mst@redhat.com> <20190102205715.14054-2-mst@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Tue, 08 Jan 2019 18:51:03 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jan 08, 2019 at 09:44:28AM -0800, Nick Desaulniers wrote: > Thanks for the patch and sorry for the delay; was totally unplugged > for the holidays. > On Wed, Jan 2, 2019 at 12:57 PM Michael S. Tsirkin wrote: > > > > Since commit 815f0ddb346c ("include/linux/compiler*.h: make compiler-*.h > > mutually exclusive") clang no longer reuses the OPTIMIZER_HIDE_VAR macro > > from compiler-gcc - instead it gets the version in > > include/linux/compiler.h. Unfortunately that version doesn't actually > > prevent compiler from optimizing out the variable. > > Good catch. Did you find this via eyeballing the code, a test, or some > other way? eyeballing > > > > Fix up by moving the macro out from compiler-gcc.h to compiler.h. > > Compilers without incline asm support will keep working > > since it's protected by an ifdef. > > > > Also fix up comments to match reality since we are no longer overriding > > any macros. > > > > Build-tested with gcc and clang. > > > > Fixes: 815f0ddb346c ("include/linux/compiler*.h: make compiler-*.h mutually exclusive") > > Cc: Eli Friedman > > Cc: Joe Perches > > Cc: Nick Desaulniers > > Cc: Linus Torvalds > > Signed-off-by: Michael S. Tsirkin > > Also for more context, see: > commit 7829fb09a2b4 ("lib: make memzero_explicit more robust against > dead store elimination") Interesting. That added this text: * while gcc behavior gets along with a normal * barrier(), llvm needs an explicit input variable to be assumed * clobbered. however: #define barrier_data(ptr) __asm__ __volatile__("": :"r"(ptr) :"memory") So no explicit variable is clobbered. Weird isn't it? > > --- > > include/linux/compiler-clang.h | 5 ++--- > > include/linux/compiler-gcc.h | 4 ---- > > include/linux/compiler-intel.h | 4 +--- > > include/linux/compiler.h | 4 +++- > > 4 files changed, 6 insertions(+), 11 deletions(-) > > > > diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h > > index 3e7dafb3ea80..7ddaeb5182e3 100644 > > --- a/include/linux/compiler-clang.h > > +++ b/include/linux/compiler-clang.h > > @@ -3,9 +3,8 @@ > > #error "Please don't include directly, include instead." > > #endif > > > > -/* Some compiler specific definitions are overwritten here > > - * for Clang compiler > > - */ > > +/* Compiler specific definitions for Clang compiler */ > > + > > #define uninitialized_var(x) x = *(&(x)) > > > > /* same as gcc, this was present in clang-2.6 so we can assume it works > > diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h > > index 2010493e1040..72054d9f0eaa 100644 > > --- a/include/linux/compiler-gcc.h > > +++ b/include/linux/compiler-gcc.h > > @@ -58,10 +58,6 @@ > > (typeof(ptr)) (__ptr + (off)); \ > > }) > > > > -/* Make the optimizer believe the variable can be manipulated arbitrarily. */ > > -#define OPTIMIZER_HIDE_VAR(var) \ > > - __asm__ ("" : "=r" (var) : "0" (var)) > > - > > /* > > * A trick to suppress uninitialized variable warning without generating any > > * code > > diff --git a/include/linux/compiler-intel.h b/include/linux/compiler-intel.h > > index 517bd14e1222..b17f3cd18334 100644 > > --- a/include/linux/compiler-intel.h > > +++ b/include/linux/compiler-intel.h > > @@ -5,9 +5,7 @@ > > > > #ifdef __ECC > > > > -/* Some compiler specific definitions are overwritten here > > - * for Intel ECC compiler > > - */ > > +/* Compiler specific definitions for Intel ECC compiler */ > > > > #include > > > > diff --git a/include/linux/compiler.h b/include/linux/compiler.h > > index 06396c1cf127..1ad367b4cd8d 100644 > > --- a/include/linux/compiler.h > > +++ b/include/linux/compiler.h > > @@ -152,7 +152,9 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val, > > #endif > > > > #ifndef OPTIMIZER_HIDE_VAR > > -#define OPTIMIZER_HIDE_VAR(var) barrier() > > +/* Make the optimizer believe the variable can be manipulated arbitrarily. */ > > +#define OPTIMIZER_HIDE_VAR(var) \ > > + __asm__ ("" : "=r" (var) : "0" (var)) > > #endif > > This should be fine, thanks for the cleanup! For now, we're not yet > confident to turn on Clang's integrated assembler for the kernel, but > I'll make sure to revisit this should we, in case Clang is then able > to optimize this out. > + Eric, who might know of a better trick for what we're trying to > accomplish with this macro. > > Reviewed-by: Nick Desaulniers > > + Miguel > Miguel, would you mind taking this into your compiler-attributes tree? > -- > Thanks, > ~Nick Desaulniers From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCH RFC 1/4] include/linux/compiler*.h: fix OPTIMIZER_HIDE_VAR Date: Tue, 8 Jan 2019 13:50:58 -0500 Message-ID: <20190108134549-mutt-send-email-mst@kernel.org> References: <20190102205715.14054-1-mst@redhat.com> <20190102205715.14054-2-mst@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Miguel Ojeda , LKML , Jason Wang , Alan Stern , Andrea Parri , Will Deacon , Peter Zijlstra , Boqun Feng , Nicholas Piggin , David Howells , Jade Alglave , Luc Maranget , "Paul E. McKenney" , Akira Yokosawa , Daniel Lustig , linux-arch@vger.kernel.org, netdev@vger.kernel.org, virtualization@lists.linux-foundation.org, Eli Friedman , Joe Perches , To: Nick Desaulniers Return-path: Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On Tue, Jan 08, 2019 at 09:44:28AM -0800, Nick Desaulniers wrote: > Thanks for the patch and sorry for the delay; was totally unplugged > for the holidays. > On Wed, Jan 2, 2019 at 12:57 PM Michael S. Tsirkin wrote: > > > > Since commit 815f0ddb346c ("include/linux/compiler*.h: make compiler-*.h > > mutually exclusive") clang no longer reuses the OPTIMIZER_HIDE_VAR macro > > from compiler-gcc - instead it gets the version in > > include/linux/compiler.h. Unfortunately that version doesn't actually > > prevent compiler from optimizing out the variable. > > Good catch. Did you find this via eyeballing the code, a test, or some > other way? eyeballing > > > > Fix up by moving the macro out from compiler-gcc.h to compiler.h. > > Compilers without incline asm support will keep working > > since it's protected by an ifdef. > > > > Also fix up comments to match reality since we are no longer overriding > > any macros. > > > > Build-tested with gcc and clang. > > > > Fixes: 815f0ddb346c ("include/linux/compiler*.h: make compiler-*.h mutually exclusive") > > Cc: Eli Friedman > > Cc: Joe Perches > > Cc: Nick Desaulniers > > Cc: Linus Torvalds > > Signed-off-by: Michael S. Tsirkin > > Also for more context, see: > commit 7829fb09a2b4 ("lib: make memzero_explicit more robust against > dead store elimination") Interesting. That added this text: * while gcc behavior gets along with a normal * barrier(), llvm needs an explicit input variable to be assumed * clobbered. however: #define barrier_data(ptr) __asm__ __volatile__("": :"r"(ptr) :"memory") So no explicit variable is clobbered. Weird isn't it? > > --- > > include/linux/compiler-clang.h | 5 ++--- > > include/linux/compiler-gcc.h | 4 ---- > > include/linux/compiler-intel.h | 4 +--- > > include/linux/compiler.h | 4 +++- > > 4 files changed, 6 insertions(+), 11 deletions(-) > > > > diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h > > index 3e7dafb3ea80..7ddaeb5182e3 100644 > > --- a/include/linux/compiler-clang.h > > +++ b/include/linux/compiler-clang.h > > @@ -3,9 +3,8 @@ > > #error "Please don't include directly, include instead." > > #endif > > > > -/* Some compiler specific definitions are overwritten here > > - * for Clang compiler > > - */ > > +/* Compiler specific definitions for Clang compiler */ > > + > > #define uninitialized_var(x) x = *(&(x)) > > > > /* same as gcc, this was present in clang-2.6 so we can assume it works > > diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h > > index 2010493e1040..72054d9f0eaa 100644 > > --- a/include/linux/compiler-gcc.h > > +++ b/include/linux/compiler-gcc.h > > @@ -58,10 +58,6 @@ > > (typeof(ptr)) (__ptr + (off)); \ > > }) > > > > -/* Make the optimizer believe the variable can be manipulated arbitrarily. */ > > -#define OPTIMIZER_HIDE_VAR(var) \ > > - __asm__ ("" : "=r" (var) : "0" (var)) > > - > > /* > > * A trick to suppress uninitialized variable warning without generating any > > * code > > diff --git a/include/linux/compiler-intel.h b/include/linux/compiler-intel.h > > index 517bd14e1222..b17f3cd18334 100644 > > --- a/include/linux/compiler-intel.h > > +++ b/include/linux/compiler-intel.h > > @@ -5,9 +5,7 @@ > > > > #ifdef __ECC > > > > -/* Some compiler specific definitions are overwritten here > > - * for Intel ECC compiler > > - */ > > +/* Compiler specific definitions for Intel ECC compiler */ > > > > #include > > > > diff --git a/include/linux/compiler.h b/include/linux/compiler.h > > index 06396c1cf127..1ad367b4cd8d 100644 > > --- a/include/linux/compiler.h > > +++ b/include/linux/compiler.h > > @@ -152,7 +152,9 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val, > > #endif > > > > #ifndef OPTIMIZER_HIDE_VAR > > -#define OPTIMIZER_HIDE_VAR(var) barrier() > > +/* Make the optimizer believe the variable can be manipulated arbitrarily. */ > > +#define OPTIMIZER_HIDE_VAR(var) \ > > + __asm__ ("" : "=r" (var) : "0" (var)) > > #endif > > This should be fine, thanks for the cleanup! For now, we're not yet > confident to turn on Clang's integrated assembler for the kernel, but > I'll make sure to revisit this should we, in case Clang is then able > to optimize this out. > + Eric, who might know of a better trick for what we're trying to > accomplish with this macro. > > Reviewed-by: Nick Desaulniers > > + Miguel > Miguel, would you mind taking this into your compiler-attributes tree? > -- > Thanks, > ~Nick Desaulniers From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCH RFC 1/4] include/linux/compiler*.h: fix OPTIMIZER_HIDE_VAR Date: Tue, 8 Jan 2019 13:50:58 -0500 Message-ID: <20190108134549-mutt-send-email-mst@kernel.org> References: <20190102205715.14054-1-mst@redhat.com> <20190102205715.14054-2-mst@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Nick Desaulniers Cc: Miguel Ojeda , LKML , Jason Wang , Alan Stern , Andrea Parri , Will Deacon , Peter Zijlstra , Boqun Feng , Nicholas Piggin , David Howells , Jade Alglave , Luc Maranget , "Paul E. McKenney" , Akira Yokosawa , Daniel Lustig , linux-arch@vger.kernel.org, netdev@vger.kernel.org, virtualization@lists.linux-foundation.org, Eli Friedman , Joe Perches List-Id: linux-sparse@vger.kernel.org On Tue, Jan 08, 2019 at 09:44:28AM -0800, Nick Desaulniers wrote: > Thanks for the patch and sorry for the delay; was totally unplugged > for the holidays. > On Wed, Jan 2, 2019 at 12:57 PM Michael S. Tsirkin wrote: > > > > Since commit 815f0ddb346c ("include/linux/compiler*.h: make compiler-*.h > > mutually exclusive") clang no longer reuses the OPTIMIZER_HIDE_VAR macro > > from compiler-gcc - instead it gets the version in > > include/linux/compiler.h. Unfortunately that version doesn't actually > > prevent compiler from optimizing out the variable. > > Good catch. Did you find this via eyeballing the code, a test, or some > other way? eyeballing > > > > Fix up by moving the macro out from compiler-gcc.h to compiler.h. > > Compilers without incline asm support will keep working > > since it's protected by an ifdef. > > > > Also fix up comments to match reality since we are no longer overriding > > any macros. > > > > Build-tested with gcc and clang. > > > > Fixes: 815f0ddb346c ("include/linux/compiler*.h: make compiler-*.h mutually exclusive") > > Cc: Eli Friedman > > Cc: Joe Perches > > Cc: Nick Desaulniers > > Cc: Linus Torvalds > > Signed-off-by: Michael S. Tsirkin > > Also for more context, see: > commit 7829fb09a2b4 ("lib: make memzero_explicit more robust against > dead store elimination") Interesting. That added this text: * while gcc behavior gets along with a normal * barrier(), llvm needs an explicit input variable to be assumed * clobbered. however: #define barrier_data(ptr) __asm__ __volatile__("": :"r"(ptr) :"memory") So no explicit variable is clobbered. Weird isn't it? > > --- > > include/linux/compiler-clang.h | 5 ++--- > > include/linux/compiler-gcc.h | 4 ---- > > include/linux/compiler-intel.h | 4 +--- > > include/linux/compiler.h | 4 +++- > > 4 files changed, 6 insertions(+), 11 deletions(-) > > > > diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h > > index 3e7dafb3ea80..7ddaeb5182e3 100644 > > --- a/include/linux/compiler-clang.h > > +++ b/include/linux/compiler-clang.h > > @@ -3,9 +3,8 @@ > > #error "Please don't include directly, include instead." > > #endif > > > > -/* Some compiler specific definitions are overwritten here > > - * for Clang compiler > > - */ > > +/* Compiler specific definitions for Clang compiler */ > > + > > #define uninitialized_var(x) x = *(&(x)) > > > > /* same as gcc, this was present in clang-2.6 so we can assume it works > > diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h > > index 2010493e1040..72054d9f0eaa 100644 > > --- a/include/linux/compiler-gcc.h > > +++ b/include/linux/compiler-gcc.h > > @@ -58,10 +58,6 @@ > > (typeof(ptr)) (__ptr + (off)); \ > > }) > > > > -/* Make the optimizer believe the variable can be manipulated arbitrarily. */ > > -#define OPTIMIZER_HIDE_VAR(var) \ > > - __asm__ ("" : "=r" (var) : "0" (var)) > > - > > /* > > * A trick to suppress uninitialized variable warning without generating any > > * code > > diff --git a/include/linux/compiler-intel.h b/include/linux/compiler-intel.h > > index 517bd14e1222..b17f3cd18334 100644 > > --- a/include/linux/compiler-intel.h > > +++ b/include/linux/compiler-intel.h > > @@ -5,9 +5,7 @@ > > > > #ifdef __ECC > > > > -/* Some compiler specific definitions are overwritten here > > - * for Intel ECC compiler > > - */ > > +/* Compiler specific definitions for Intel ECC compiler */ > > > > #include > > > > diff --git a/include/linux/compiler.h b/include/linux/compiler.h > > index 06396c1cf127..1ad367b4cd8d 100644 > > --- a/include/linux/compiler.h > > +++ b/include/linux/compiler.h > > @@ -152,7 +152,9 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val, > > #endif > > > > #ifndef OPTIMIZER_HIDE_VAR > > -#define OPTIMIZER_HIDE_VAR(var) barrier() > > +/* Make the optimizer believe the variable can be manipulated arbitrarily. */ > > +#define OPTIMIZER_HIDE_VAR(var) \ > > + __asm__ ("" : "=r" (var) : "0" (var)) > > #endif > > This should be fine, thanks for the cleanup! For now, we're not yet > confident to turn on Clang's integrated assembler for the kernel, but > I'll make sure to revisit this should we, in case Clang is then able > to optimize this out. > + Eric, who might know of a better trick for what we're trying to > accomplish with this macro. > > Reviewed-by: Nick Desaulniers > > + Miguel > Miguel, would you mind taking this into your compiler-attributes tree? > -- > Thanks, > ~Nick Desaulniers