linux-kernel - Re: [PATCH 16/19] linux/compiler.h: Split into compiler.h and compiler-types.h

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171024095253.b7legaopolk274vh@gmail.com>
Date:   Tue, 24 Oct 2017 11:52:53 +0200
From:   Ingo Molnar <mingo@...nel.org>
To:     Will Deacon <will.deacon@....com>
Cc:     "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
        linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org,
        mark.rutland@....com, snitzer@...hat.com,
        thor.thayer@...ux.intel.com, viro@...iv.linux.org.uk,
        davem@...emloft.net, shuah@...nel.org, mpe@...erman.id.au,
        tj@...nel.org, torvalds@...ux-foundation.org
Subject: Re: [PATCH 16/19] linux/compiler.h: Split into compiler.h and
 compiler-types.h


* Will Deacon <will.deacon@....com> wrote:

> On Tue, Oct 24, 2017 at 11:25:12AM +0200, Ingo Molnar wrote:
> > 
> > * Paul E. McKenney <paulmck@...ux.vnet.ibm.com> wrote:
> > 
> > > From: Will Deacon <will.deacon@....com>
> > > 
> > > 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 <fengguang.wu@...el.com>
> > > Signed-off-by: Will Deacon <will.deacon@....com>
> > > Signed-off-by: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
> > > [ 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