From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ingo Molnar Subject: Re: [PATCH 16/19] linux/compiler.h: Split into compiler.h and compiler-types.h Date: Tue, 24 Oct 2017 11:52:53 +0200 Message-ID: <20171024095253.b7legaopolk274vh@gmail.com> References: <20171023210408.GA2930@linux.vnet.ibm.com> <1508792849-3115-16-git-send-email-paulmck@linux.vnet.ibm.com> <20171024092512.nwumnxqrpaols55g@gmail.com> <20171024093026.GB17909@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <20171024093026.GB17909@arm.com> Sender: linux-kernel-owner@vger.kernel.org To: Will Deacon Cc: "Paul E. McKenney" , linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org, mark.rutland@arm.com, snitzer@redhat.com, thor.thayer@linux.intel.com, viro@zeniv.linux.org.uk, davem@davemloft.net, shuah@kernel.org, mpe@ellerman.id.au, tj@kernel.org, torvalds@linux-foundation.org List-Id: linux-arch.vger.kernel.org * Will Deacon wrote: > On Tue, Oct 24, 2017 at 11:25:12AM +0200, Ingo Molnar wrote: > > > > * Paul E. McKenney wrote: > > > > > From: Will Deacon > > > > > > linux/compiler.h is included indirectly by linux/types.h via > > > uapi/linux/types.h -> uapi/linux/posix_types.h -> linux/stddef.h > > > -> uapi/linux/stddef.h and is needed to provide a proper definition of > > > offsetof. > > > > > > Unfortunately, compiler.h requires a definition of > > > smp_read_barrier_depends() for defining lockless_dereference and soon > > > for defining READ_ONCE, which means that all > > > users of READ_ONCE will need to include asm/barrier.h to avoid splats > > > such as: > > > > > > In file included from include/uapi/linux/stddef.h:1:0, > > > from include/linux/stddef.h:4, > > > from arch/h8300/kernel/asm-offsets.c:11: > > > include/linux/list.h: In function 'list_empty': > > > >> include/linux/compiler.h:343:2: error: implicit declaration of function 'smp_read_barrier_depends' [-Werror=implicit-function-declaration] > > > smp_read_barrier_depends(); /* Enforce dependency ordering from x */ \ > > > ^ > > > > > > A better alternative is to include asm/barrier.h in linux/compiler.h, > > > but this requires a type definition for "bool" on some architectures > > > (e.g. x86), which is defined later by linux/types.h. Type "bool" is also > > > used directly in linux/compiler.h, so the whole thing is pretty fragile. > > > > > > This patch splits compiler.h in two: compiler-types.h contains type > > > annotations, definitions and the compiler-specific parts, whereas > > > compiler.h #includes compiler-types.h and additionally defines macros > > > such as {READ,WRITE.ACCESS}_ONCE. > > > > > > uapi/linux/stddef.h and linux/linkage.h are then moved over to include > > > linux/compiler-types.h, which fixes the build for h8 and blackfin. > > > > > > Reported-by: kbuild test robot > > > Signed-off-by: Will Deacon > > > Signed-off-by: Paul E. McKenney > > > [ paulmck: Add Will's fixes for issues reported by kbuild test robot. ] > > > --- > > > arch/arm/include/asm/ptrace.h | 3 +- > > > arch/sparc/include/asm/ptrace.h | 1 + > > > arch/um/include/shared/init.h | 2 +- > > > include/linux/compiler-clang.h | 2 +- > > > include/linux/compiler-gcc.h | 2 +- > > > include/linux/compiler-intel.h | 2 +- > > > include/linux/compiler-types.h | 274 ++++++++++++++++++++++++++++++++++++++++ > > > include/linux/compiler.h | 265 +------------------------------------- > > > > The common pattern is to use an underscore (compiler_types.h), not a hyphen > > (compiler-types.h) ... > > I just followed the existing examples of compiler-{clang,gcc,intel}.h! > I can respin if you like, but it will be inconsistent with the existing > compiler* headers and it's not like there aren't other hyphenated header > files in include/linux/ (I count 433). The pattern to follow here is the _types.h header file postfix that denotes data type definitions of a subsystem, and those are consistently _types.h, in 90%+ of the cases: triton:~/tip> find include/ arch/ | grep -e _types.h | wc -l 90 triton:~/tip> find include/ arch/ | grep -e -types.h | wc -l 6 Thanks, Ingo From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f65.google.com ([74.125.82.65]:47065 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751807AbdJXJw5 (ORCPT ); Tue, 24 Oct 2017 05:52:57 -0400 Date: Tue, 24 Oct 2017 11:52:53 +0200 From: Ingo Molnar Subject: Re: [PATCH 16/19] linux/compiler.h: Split into compiler.h and compiler-types.h Message-ID: <20171024095253.b7legaopolk274vh@gmail.com> References: <20171023210408.GA2930@linux.vnet.ibm.com> <1508792849-3115-16-git-send-email-paulmck@linux.vnet.ibm.com> <20171024092512.nwumnxqrpaols55g@gmail.com> <20171024093026.GB17909@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171024093026.GB17909@arm.com> Sender: linux-arch-owner@vger.kernel.org List-ID: To: Will Deacon Cc: "Paul E. McKenney" , linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org, mark.rutland@arm.com, snitzer@redhat.com, thor.thayer@linux.intel.com, viro@zeniv.linux.org.uk, davem@davemloft.net, shuah@kernel.org, mpe@ellerman.id.au, tj@kernel.org, torvalds@linux-foundation.org Message-ID: <20171024095253.LEww3EMbxEvNRGXT28HO5gCSheZQJvhQ_Zu1apqsCx4@z> * Will Deacon wrote: > On Tue, Oct 24, 2017 at 11:25:12AM +0200, Ingo Molnar wrote: > > > > * Paul E. McKenney wrote: > > > > > From: Will Deacon > > > > > > linux/compiler.h is included indirectly by linux/types.h via > > > uapi/linux/types.h -> uapi/linux/posix_types.h -> linux/stddef.h > > > -> uapi/linux/stddef.h and is needed to provide a proper definition of > > > offsetof. > > > > > > Unfortunately, compiler.h requires a definition of > > > smp_read_barrier_depends() for defining lockless_dereference and soon > > > for defining READ_ONCE, which means that all > > > users of READ_ONCE will need to include asm/barrier.h to avoid splats > > > such as: > > > > > > In file included from include/uapi/linux/stddef.h:1:0, > > > from include/linux/stddef.h:4, > > > from arch/h8300/kernel/asm-offsets.c:11: > > > include/linux/list.h: In function 'list_empty': > > > >> include/linux/compiler.h:343:2: error: implicit declaration of function 'smp_read_barrier_depends' [-Werror=implicit-function-declaration] > > > smp_read_barrier_depends(); /* Enforce dependency ordering from x */ \ > > > ^ > > > > > > A better alternative is to include asm/barrier.h in linux/compiler.h, > > > but this requires a type definition for "bool" on some architectures > > > (e.g. x86), which is defined later by linux/types.h. Type "bool" is also > > > used directly in linux/compiler.h, so the whole thing is pretty fragile. > > > > > > This patch splits compiler.h in two: compiler-types.h contains type > > > annotations, definitions and the compiler-specific parts, whereas > > > compiler.h #includes compiler-types.h and additionally defines macros > > > such as {READ,WRITE.ACCESS}_ONCE. > > > > > > uapi/linux/stddef.h and linux/linkage.h are then moved over to include > > > linux/compiler-types.h, which fixes the build for h8 and blackfin. > > > > > > Reported-by: kbuild test robot > > > Signed-off-by: Will Deacon > > > Signed-off-by: Paul E. McKenney > > > [ paulmck: Add Will's fixes for issues reported by kbuild test robot. ] > > > --- > > > arch/arm/include/asm/ptrace.h | 3 +- > > > arch/sparc/include/asm/ptrace.h | 1 + > > > arch/um/include/shared/init.h | 2 +- > > > include/linux/compiler-clang.h | 2 +- > > > include/linux/compiler-gcc.h | 2 +- > > > include/linux/compiler-intel.h | 2 +- > > > include/linux/compiler-types.h | 274 ++++++++++++++++++++++++++++++++++++++++ > > > include/linux/compiler.h | 265 +------------------------------------- > > > > The common pattern is to use an underscore (compiler_types.h), not a hyphen > > (compiler-types.h) ... > > I just followed the existing examples of compiler-{clang,gcc,intel}.h! > I can respin if you like, but it will be inconsistent with the existing > compiler* headers and it's not like there aren't other hyphenated header > files in include/linux/ (I count 433). The pattern to follow here is the _types.h header file postfix that denotes data type definitions of a subsystem, and those are consistently _types.h, in 90%+ of the cases: triton:~/tip> find include/ arch/ | grep -e _types.h | wc -l 90 triton:~/tip> find include/ arch/ | grep -e -types.h | wc -l 6 Thanks, Ingo