[PATCH] compiler.h: Move barrier() back into compiler-*.h

public inbox for linux-next@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH] compiler.h: Move barrier() back into compiler-*.h
       [not found] <202010312104.Dk9VQJYb-lkp@intel.com>
@ 2020-11-01 17:31 ` Arvind Sankar
  2020-11-01 17:38   ` Matthew Wilcox
  0 siblings, 1 reply; 10+ messages in thread
From: Arvind Sankar @ 2020-11-01 17:31 UTC (permalink / raw)
  To: kernel test robot, linux-next, Stephen Rothwell
  Cc: kbuild-all, Linux Memory Management List, Nick Desaulniers,
	Kees Cook, Andrew Morton, stable

Commit
  b9de06783f01 ("compiler.h: fix barrier_data() on clang")
moved the definition of barrier() into compiler.h.

This causes build failures at least on alpha, because there are files
that rely on barrier() being defined via the implicit include of
compiler_types.h.

Revert this portion of the commit to fix.

Link: https://lore.kernel.org/linux-mm/202010312104.Dk9VQJYb-lkp@intel.com/
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
Fixes: b9de06783f01 ("compiler.h: fix barrier_data() on clang")
Cc: <stable@vger.kernel.org>
---
 include/linux/compiler-clang.h | 6 ++++++
 include/linux/compiler-gcc.h   | 5 +++++
 include/linux/compiler.h       | 3 +--
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index dd7233c48bf3..230604e7f057 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -60,6 +60,12 @@
 #define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
 #endif
 
+/* The following are for compatibility with GCC, from compiler-gcc.h,
+ * and may be redefined here because they should not be shared with other
+ * compilers, like ICC.
+ */
+#define barrier() __asm__ __volatile__("" : : : "memory")
+
 #if __has_feature(shadow_call_stack)
 # define __noscs	__attribute__((__no_sanitize__("shadow-call-stack")))
 #endif
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 50912ed00278..a572965c801a 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -15,6 +15,11 @@
 # error Sorry, your version of GCC is too old - please use 4.9 or newer.
 #endif
 
+/* Optimization barrier */
+
+/* The "volatile" is due to gcc bugs */
+#define barrier() __asm__ __volatile__("": : :"memory")
+
 /*
  * This macro obfuscates arithmetic on a variable address so that gcc
  * shouldn't recognize the original var, and make assumptions about it.
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index b8fe0c23cfff..25c803f4222f 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -80,8 +80,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
 
 /* Optimization barrier */
 #ifndef barrier
-/* The "volatile" is due to gcc bugs */
-# define barrier() __asm__ __volatile__("": : :"memory")
+# define barrier() __memory_barrier()
 #endif
 
 #ifndef barrier_data
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] compiler.h: Move barrier() back into compiler-*.h
  2020-11-01 17:31 ` [PATCH] compiler.h: Move barrier() back into compiler-*.h Arvind Sankar
@ 2020-11-01 17:38   ` Matthew Wilcox
  2020-11-01 17:48     ` Randy Dunlap
                       ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Matthew Wilcox @ 2020-11-01 17:38 UTC (permalink / raw)
  To: Arvind Sankar
  Cc: kernel test robot, linux-next, Stephen Rothwell, kbuild-all,
	Linux Memory Management List, Nick Desaulniers, Kees Cook,
	Andrew Morton, stable

On Sun, Nov 01, 2020 at 12:31:05PM -0500, Arvind Sankar wrote:
> Commit
>   b9de06783f01 ("compiler.h: fix barrier_data() on clang")
> moved the definition of barrier() into compiler.h.

That's not a real commit ID.  It only exists in linux-next and
will expire after a few weeks.

The right way to fix a patch in Andrew's tree is to send an email
asking him to apply it as a -fix patch.  As part of Andrew's submission
process, he folds all the -fix patches into the parent patch and it
shows up pristine in Linus' tree.

> This causes build failures at least on alpha, because there are files
> that rely on barrier() being defined via the implicit include of
> compiler_types.h.

That seems like a bug that should be fixed rather than reverting this
part of the patch?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] compiler.h: Move barrier() back into compiler-*.h
  2020-11-01 17:38   ` Matthew Wilcox
@ 2020-11-01 17:48     ` Randy Dunlap
  2020-11-01 17:53       ` Matthew Wilcox
  2020-11-01 19:51     ` Arvind Sankar
  2020-11-01 21:56     ` Stephen Rothwell
  2 siblings, 1 reply; 10+ messages in thread
From: Randy Dunlap @ 2020-11-01 17:48 UTC (permalink / raw)
  To: Matthew Wilcox, Arvind Sankar
  Cc: kernel test robot, linux-next, Stephen Rothwell, kbuild-all,
	Linux Memory Management List, Nick Desaulniers, Kees Cook,
	Andrew Morton, stable

On 11/1/20 9:38 AM, Matthew Wilcox wrote:
> On Sun, Nov 01, 2020 at 12:31:05PM -0500, Arvind Sankar wrote:
>> Commit
>>   b9de06783f01 ("compiler.h: fix barrier_data() on clang")
>> moved the definition of barrier() into compiler.h.
> 
> That's not a real commit ID.  It only exists in linux-next and
> will expire after a few weeks.
> 
> The right way to fix a patch in Andrew's tree is to send an email
> asking him to apply it as a -fix patch.  As part of Andrew's submission
> process, he folds all the -fix patches into the parent patch and it
> shows up pristine in Linus' tree.
> 
>> This causes build failures at least on alpha, because there are files
>> that rely on barrier() being defined via the implicit include of
>> compiler_types.h.
> 
> That seems like a bug that should be fixed rather than reverting this
> part of the patch?
> 

maybe: ?

https://lore.kernel.org/lkml/20201101030159.15858-1-rdunlap@infradead.org/

-- 
~Randy


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] compiler.h: Move barrier() back into compiler-*.h
  2020-11-01 17:48     ` Randy Dunlap
@ 2020-11-01 17:53       ` Matthew Wilcox
  0 siblings, 0 replies; 10+ messages in thread
From: Matthew Wilcox @ 2020-11-01 17:53 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Arvind Sankar, kernel test robot, linux-next, Stephen Rothwell,
	kbuild-all, Linux Memory Management List, Nick Desaulniers,
	Kees Cook, Andrew Morton, stable

On Sun, Nov 01, 2020 at 09:48:30AM -0800, Randy Dunlap wrote:
> maybe: ?
> 
> https://lore.kernel.org/lkml/20201101030159.15858-1-rdunlap@infradead.org/

Yes

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] compiler.h: Move barrier() back into compiler-*.h
  2020-11-01 17:38   ` Matthew Wilcox
  2020-11-01 17:48     ` Randy Dunlap
@ 2020-11-01 19:51     ` Arvind Sankar
  2020-11-01 19:52       ` Matthew Wilcox
  2020-11-01 21:56     ` Stephen Rothwell
  2 siblings, 1 reply; 10+ messages in thread
From: Arvind Sankar @ 2020-11-01 19:51 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Arvind Sankar, kernel test robot, linux-next, Stephen Rothwell,
	kbuild-all, Linux Memory Management List, Nick Desaulniers,
	Kees Cook, Andrew Morton, stable, Masahiro Yamada

On Sun, Nov 01, 2020 at 05:38:35PM +0000, Matthew Wilcox wrote:
> On Sun, Nov 01, 2020 at 12:31:05PM -0500, Arvind Sankar wrote:
> > Commit
> >   b9de06783f01 ("compiler.h: fix barrier_data() on clang")
> > moved the definition of barrier() into compiler.h.
> 
> That's not a real commit ID.  It only exists in linux-next and
> will expire after a few weeks.
> 
> The right way to fix a patch in Andrew's tree is to send an email
> asking him to apply it as a -fix patch.  As part of Andrew's submission
> process, he folds all the -fix patches into the parent patch and it
> shows up pristine in Linus' tree.

Ok. So I still send it as a separate patch and he does the folding, or
should I send a revised patch that replaces the original one?

> 
> > This causes build failures at least on alpha, because there are files
> > that rely on barrier() being defined via the implicit include of
> > compiler_types.h.
> 
> That seems like a bug that should be fixed rather than reverting this
> part of the patch?
> 

I thought about that, but barrier() is used in a large number of places,
so reverting seemed like the safer approach, in case more errors like
this cropped up. I don't know if it's a bug, as it was defined in
compiler_types.h before my original patch, I'm not sure if there's a
reason only compiler_types.h rather than compiler.h is implicitly
included via the Makefile.

Cc'ing Masahiro Yamada in case he has comments on that part.

Thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] compiler.h: Move barrier() back into compiler-*.h
  2020-11-01 19:51     ` Arvind Sankar
@ 2020-11-01 19:52       ` Matthew Wilcox
  2020-11-01 19:59         ` Arvind Sankar
  0 siblings, 1 reply; 10+ messages in thread
From: Matthew Wilcox @ 2020-11-01 19:52 UTC (permalink / raw)
  To: Arvind Sankar
  Cc: kernel test robot, linux-next, Stephen Rothwell, kbuild-all,
	Linux Memory Management List, Nick Desaulniers, Kees Cook,
	Andrew Morton, stable, Masahiro Yamada

On Sun, Nov 01, 2020 at 02:51:10PM -0500, Arvind Sankar wrote:
> Ok. So I still send it as a separate patch and he does the folding, or
> should I send a revised patch that replaces the original one?

I think Randy's patch should be merged instead of this patch.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] compiler.h: Move barrier() back into compiler-*.h
  2020-11-01 19:52       ` Matthew Wilcox
@ 2020-11-01 19:59         ` Arvind Sankar
  2020-11-01 20:40           ` Randy Dunlap
  0 siblings, 1 reply; 10+ messages in thread
From: Arvind Sankar @ 2020-11-01 19:59 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Arvind Sankar, kernel test robot, linux-next, Stephen Rothwell,
	kbuild-all, Linux Memory Management List, Nick Desaulniers,
	Kees Cook, Andrew Morton, stable, Masahiro Yamada

On Sun, Nov 01, 2020 at 07:52:15PM +0000, Matthew Wilcox wrote:
> On Sun, Nov 01, 2020 at 02:51:10PM -0500, Arvind Sankar wrote:
> > Ok. So I still send it as a separate patch and he does the folding, or
> > should I send a revised patch that replaces the original one?
> 
> I think Randy's patch should be merged instead of this patch.

Ok, if that one works then it's better I agree.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] compiler.h: Move barrier() back into compiler-*.h
  2020-11-01 19:59         ` Arvind Sankar
@ 2020-11-01 20:40           ` Randy Dunlap
  2020-11-01 21:59             ` Stephen Rothwell
  0 siblings, 1 reply; 10+ messages in thread
From: Randy Dunlap @ 2020-11-01 20:40 UTC (permalink / raw)
  To: Arvind Sankar, Matthew Wilcox
  Cc: kernel test robot, linux-next, Stephen Rothwell, kbuild-all,
	Linux Memory Management List, Nick Desaulniers, Kees Cook,
	Andrew Morton, stable, Masahiro Yamada

On 11/1/20 11:59 AM, Arvind Sankar wrote:
> On Sun, Nov 01, 2020 at 07:52:15PM +0000, Matthew Wilcox wrote:
>> On Sun, Nov 01, 2020 at 02:51:10PM -0500, Arvind Sankar wrote:
>>> Ok. So I still send it as a separate patch and he does the folding, or
>>> should I send a revised patch that replaces the original one?
>>
>> I think Randy's patch should be merged instead of this patch.
> 
> Ok, if that one works then it's better I agree.
> 

Do I need to resend it to Andrew?


--
~Randy


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] compiler.h: Move barrier() back into compiler-*.h
  2020-11-01 20:40           ` Randy Dunlap
@ 2020-11-01 21:59             ` Stephen Rothwell
  0 siblings, 0 replies; 10+ messages in thread
From: Stephen Rothwell @ 2020-11-01 21:59 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Arvind Sankar, Matthew Wilcox, kernel test robot, linux-next,
	kbuild-all, Linux Memory Management List, Nick Desaulniers,
	Kees Cook, Andrew Morton, stable, Masahiro Yamada

[-- Attachment #1: Type: text/plain, Size: 725 bytes --]

Hi Randy,

On Sun, 1 Nov 2020 12:40:19 -0800 Randy Dunlap <rdunlap@infradead.org> wrote:
>
> On 11/1/20 11:59 AM, Arvind Sankar wrote:
> > On Sun, Nov 01, 2020 at 07:52:15PM +0000, Matthew Wilcox wrote:  
> >> On Sun, Nov 01, 2020 at 02:51:10PM -0500, Arvind Sankar wrote:  
> >>> Ok. So I still send it as a separate patch and he does the folding, or
> >>> should I send a revised patch that replaces the original one?  
> >>
> >> I think Randy's patch should be merged instead of this patch.  
> > 
> > Ok, if that one works then it's better I agree.
> >   
> 
> Do I need to resend it to Andrew?

Well, his SOB is on the original patch (as is mine) ... so, yes, please.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] compiler.h: Move barrier() back into compiler-*.h
  2020-11-01 17:38   ` Matthew Wilcox
  2020-11-01 17:48     ` Randy Dunlap
  2020-11-01 19:51     ` Arvind Sankar
@ 2020-11-01 21:56     ` Stephen Rothwell
  2 siblings, 0 replies; 10+ messages in thread
From: Stephen Rothwell @ 2020-11-01 21:56 UTC (permalink / raw)
  To: Arvind Sankar
  Cc: Matthew Wilcox, kernel test robot, linux-next, kbuild-all,
	Linux Memory Management List, Nick Desaulniers, Kees Cook,
	Andrew Morton, stable

[-- Attachment #1: Type: text/plain, Size: 511 bytes --]

Hi all,

On Sun, 1 Nov 2020 17:38:35 +0000 Matthew Wilcox <willy@infradead.org> wrote:
>
> On Sun, Nov 01, 2020 at 12:31:05PM -0500, Arvind Sankar wrote:
> > Commit
> >   b9de06783f01 ("compiler.h: fix barrier_data() on clang")
> > moved the definition of barrier() into compiler.h.  
> 
> That's not a real commit ID.  It only exists in linux-next and
> will expire after a few weeks.

Which also means that the Cc: <stable@vger.kernel.org> is also
unnecessary.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-11-01 21:59 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <202010312104.Dk9VQJYb-lkp@intel.com>
2020-11-01 17:31 ` [PATCH] compiler.h: Move barrier() back into compiler-*.h Arvind Sankar
2020-11-01 17:38   ` Matthew Wilcox
2020-11-01 17:48     ` Randy Dunlap
2020-11-01 17:53       ` Matthew Wilcox
2020-11-01 19:51     ` Arvind Sankar
2020-11-01 19:52       ` Matthew Wilcox
2020-11-01 19:59         ` Arvind Sankar
2020-11-01 20:40           ` Randy Dunlap
2020-11-01 21:59             ` Stephen Rothwell
2020-11-01 21:56     ` Stephen Rothwell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox