[PATCH 2/2] arm64: insn: move AARCH64_INSN_SIZE into <asm/insn.h>

Will Deacon will at kernel.org
Mon Jun 21 03:59:13 PDT 2021


On Mon, Jun 21, 2021 at 09:08:30AM +0100, Mark Rutland wrote:
> On Fri, Jun 18, 2021 at 09:53:31AM -0700, Nathan Chancellor wrote:
> > On Fri, Jun 18, 2021 at 04:18:35PM +0100, Mark Rutland wrote:
> > > On Thu, Jun 17, 2021 at 06:25:27PM -0700, Nathan Chancellor wrote:
> > > > Hi Mark,
> > > 
> > > Hi Nathan,
> > > 
> > > > On Wed, Jun 09, 2021 at 11:23:01AM +0100, Mark Rutland wrote:
> > > > > For histroical reasons, we define AARCH64_INSN_SIZE in
> > > > > <asm/alternative-macros.h>, but it would make more sense to do so in
> > > > > <asm/insn.h>. Let's move it into <asm/insn.h>, and add the necessary
> > > > > include directives for this.
> > > 
> > > > I bisected a CONFIG_LTO_CLANG_THIN=y build failure that our CI reported
> > > > to this patch:
> > > > 
> > > > https://builds.tuxbuild.com/1u4Fpx2FQkkgkyPxWtq0Ke4YFCQ/build.log
> > > 
> > > Thanks for reporting this; the lopg is really helpful!
> > > 
> > > > I have not had a whole ton of time to look into this (dealing with a
> > > > million fires it seems :^) but it is not immediately obvious to me why
> > > > this fails because include/linux/build_bug.h is included within
> > > > arch/arm64/include/asm/insn.h.
> > > 
> > > The problem is that with LTO, we patch READ_ONCE(), and <asm/rwonce.h>
> > > includes <asm/insn.h>, creating a circular include chain:
> > > 
> > > 	<linux/build_bug.h>
> > > 	<linux/compiler.h>
> > > 	<asm/rwonce.h>
> > > 	<asm/alternative-macros.h>
> > > 	<asm/insn.h>
> > > 	<linux/build-bug.h>
> > > 
> > > ... and so when <asm/insn.h> includes <linux/build_bug.h>, none of the
> > > BUILD_BUG* definitions have happened yet.
> > 
> > Aha, that would certainly explain it. I figured something like this
> > would be the root cause but figuring out header dependencies is not my
> > cup of tea.
> > 
> > > Will, are you happy to take the fixup patch below, or would you prefer
> > > to drop this patch for now?
> 
> > >  arch/arm64/include/asm/alternative-macros.h | 2 +-
> > >  arch/arm64/include/asm/insn.h               | 5 +----
> > 
> > Looks like arch/arm64/include/asm/insn-def.h is missing from this patch?
> > 
> > If I add one with just the two deleted lines plus a header guard, the
> > build passes.
> > 
> > Tested-by: Nathan Chancellor <nathan at kernel.org>
> 
> Whoops; that should have been as below.

I've queued this locally on top of for-next/insn and will push out later
on (once my LTO build has finished).

Will



More information about the linux-arm-kernel mailing list