Re: [PATCH] bit_spinlock: Include <asm/processor.h>

From: Al Viro <viro@zeniv.linux.org.uk>
To: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Cc: Vineet Gupta <vgupta@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Dan Carpenter <error27@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-snps-arc@lists.infradead.org, linux-kernel@vger.kernel.org,
	kernel-janitors@vger.kernel.org
Subject: Re: [PATCH] bit_spinlock: Include <asm/processor.h>
Date: Tue, 10 Jan 2023 21:23:35 +0000	[thread overview]
Message-ID: <Y73XV1SRtcpJQ1Vq@ZenIV> (raw)
In-Reply-To: <f4aa7f0d-50a9-b5e0-e980-1e8e779b48bc@wanadoo.fr>

On Tue, Jan 10, 2023 at 07:08:33PM +0100, Christophe JAILLET wrote:
> Le 10/01/2023 à 08:19, Vineet Gupta a écrit :
> > 
> > On 1/8/23 11:04, Christophe JAILLET wrote:
> > > In an attempt to simplify some includes in <include/dcache.h>, it
> > > appeared, when compiling fs/ecryptfs/dentry.c, that
> > > <linux/bit_spinlock.h>
> > > was relying on other includes to get the definition of cpu_relax().
> > > (see [1])
> > > 
> > > It broke on arc.
> > 
> > It its just ARC that broke, maybe we can do something there ?
> 
> Hi,
> 
> It is all what build-bots have spotted so far :)
> 
> I don't think that "fixing" it in ARC is the right approach, unless I missed
> something.
> 
> <linux/bit_spinlock.h> does use cpu_relax(), so it should include what is
> need for that, and not rely on other black magic.

Umm...  That's not obvious - it only uses cpu_relax() in macros, so missing
include would not cause problems if all actual users of those macros happen
to pull the needed header by other means.

Said that, we have

1) defined directly in asm/processor.h, using nothing but the stuff provided by
compiler.h:
	alpha, arc, csky, loongarch, m68k, microblaze, nios2,
	openrisc, parisc, s390, sh, xtensa
2) same, using something in headers pulled by asm/processor.h itself:
	ia64 (needs asm/intrinsic.h)
	hexagon (needs asm/hexagon_vm.h)
	um (needs arch/um/include/linux/time-internal.h)
3) same, but defined in something pulled by asm/processor.h rather than
in asm/processor.h itself; asm/vdso/processor.h is the common location -
those are the cases when we share the same definition for kernel and
vdso builds
	sparc (asm/processor_32.h or asm/processor_64.h)
	arm (asm/vdso/processor.h)
	arm64 (asm/vdso/processor.h)
	powerpc (asm/vdso/processor.h)
	x86 (asm/vdso/processor.h)
	riscv (asm/vdso/processor.h; needs several headers included there -
jump_label.h, etc.)
	mips (asm/vdso/processor.h, needs asm/barrier.h, pulled from asm/processor.h
by way of linux/atomic.h -> asm/atomic.h -> asm/barrier.h)

So asm/processor.h is sufficient for working cpu_relax() and if some
arch-independent code wants cpu_relax() it should pull either
asm/processor.h or linux/processor.h

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc