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

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:

quoted

On Fri, Jun 18, 2021 at 04:18:35PM +0100, Mark Rutland wrote:

quoted

On Thu, Jun 17, 2021 at 06:25:27PM -0700, Nathan Chancellor wrote:

quoted

Hi Mark,

Hi Nathan,

quoted

On Wed, Jun 09, 2021 at 11:23:01AM +0100, Mark Rutland wrote:

quoted

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.

quoted

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!

quoted

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.

quoted

Will, are you happy to take the fixup patch below, or would you prefer
to drop this patch for now?

quoted

quoted

 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@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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel