diff mbox series

[RFC,v2,08/12] arm64: cmpxchg: Include build_bug.h instead of bug.h for BUILD_BUG

Message ID 1519657500-15094-9-git-send-email-will.deacon@arm.com
State Accepted
Commit e8a2d040fee54606ff62cc1f22e14ad9b2677f15
Headers show
Series Rewrite asm-generic/bitops/{atomic,lock}.h and use on arm64 | expand

Commit Message

Will Deacon Feb. 26, 2018, 3:04 p.m. UTC
Having asm/cmpxchg.h pull in linux/bug.h is problematic because this
ends up pulling in the atomic bitops which themselves may be built on
top of atomic.h and cmpxchg.h.

Instead, just include build_bug.h for the definition of BUILD_BUG.

Signed-off-by: Will Deacon <will.deacon@arm.com>

---
 arch/arm64/include/asm/cmpxchg.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.1.4

Comments

Mark Rutland Feb. 26, 2018, 3:48 p.m. UTC | #1
On Mon, Feb 26, 2018 at 03:04:56PM +0000, Will Deacon wrote:
> Having asm/cmpxchg.h pull in linux/bug.h is problematic because this

> ends up pulling in the atomic bitops which themselves may be built on

> top of atomic.h and cmpxchg.h.

> 

> Instead, just include build_bug.h for the definition of BUILD_BUG.


We also use VM_BUG_ON(), defined in <linux/mmdebug.h>, which includes
<linux/bug.h>.

... so I think we still have some fragility here, albeit no worse than before.

We also miss includes for:

* <linux/percpu-defs.h> (raw_cpu_ptr)
* <linux/preempt.h> (preempt_disable, preempt_enable)
* <linux/compiler.h> (unreachable)

I'm not sure if those are made worse by this change.

Mark.

> 

> Signed-off-by: Will Deacon <will.deacon@arm.com>

> ---

>  arch/arm64/include/asm/cmpxchg.h | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

> 

> diff --git a/arch/arm64/include/asm/cmpxchg.h b/arch/arm64/include/asm/cmpxchg.h

> index ae852add053d..bc9e07bc6428 100644

> --- a/arch/arm64/include/asm/cmpxchg.h

> +++ b/arch/arm64/include/asm/cmpxchg.h

> @@ -18,7 +18,7 @@

>  #ifndef __ASM_CMPXCHG_H

>  #define __ASM_CMPXCHG_H

>  

> -#include <linux/bug.h>

> +#include <linux/build_bug.h>

>  

>  #include <asm/atomic.h>

>  #include <asm/barrier.h>

> -- 

> 2.1.4

> 

> 

> _______________________________________________

> linux-arm-kernel mailing list

> linux-arm-kernel@lists.infradead.org

> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Will Deacon Feb. 27, 2018, 5:33 p.m. UTC | #2
On Mon, Feb 26, 2018 at 03:48:49PM +0000, Mark Rutland wrote:
> On Mon, Feb 26, 2018 at 03:04:56PM +0000, Will Deacon wrote:

> > Having asm/cmpxchg.h pull in linux/bug.h is problematic because this

> > ends up pulling in the atomic bitops which themselves may be built on

> > top of atomic.h and cmpxchg.h.

> > 

> > Instead, just include build_bug.h for the definition of BUILD_BUG.

> 

> We also use VM_BUG_ON(), defined in <linux/mmdebug.h>, which includes

> <linux/bug.h>.

> 

> ... so I think we still have some fragility here, albeit no worse than before.

> 

> We also miss includes for:

> 

> * <linux/percpu-defs.h> (raw_cpu_ptr)

> * <linux/preempt.h> (preempt_disable, preempt_enable)


Hmm, we can't include this one because it pulls in linux/bitops.h. I've
moved the percpu cmpxchg stuff into asm/percpu.h, but that too is missing
the linux/preempt.h #include, so I've added that as well.

Generally, I think if we want to clean up our #includes then that's better
done as a separate series rather than as a piecemeal effort, which will
likely fail to identify many of the underlying problems.

Will
Mark Rutland Feb. 27, 2018, 5:34 p.m. UTC | #3
On Tue, Feb 27, 2018 at 05:33:23PM +0000, Will Deacon wrote:
> On Mon, Feb 26, 2018 at 03:48:49PM +0000, Mark Rutland wrote:

> > On Mon, Feb 26, 2018 at 03:04:56PM +0000, Will Deacon wrote:

> > > Having asm/cmpxchg.h pull in linux/bug.h is problematic because this

> > > ends up pulling in the atomic bitops which themselves may be built on

> > > top of atomic.h and cmpxchg.h.

> > > 

> > > Instead, just include build_bug.h for the definition of BUILD_BUG.

> > 

> > We also use VM_BUG_ON(), defined in <linux/mmdebug.h>, which includes

> > <linux/bug.h>.

> > 

> > ... so I think we still have some fragility here, albeit no worse than before.

> > 

> > We also miss includes for:

> > 

> > * <linux/percpu-defs.h> (raw_cpu_ptr)

> > * <linux/preempt.h> (preempt_disable, preempt_enable)

> 

> Hmm, we can't include this one because it pulls in linux/bitops.h. I've

> moved the percpu cmpxchg stuff into asm/percpu.h, but that too is missing

> the linux/preempt.h #include, so I've added that as well.

> 

> Generally, I think if we want to clean up our #includes then that's better

> done as a separate series rather than as a piecemeal effort, which will

> likely fail to identify many of the underlying problems.


Sure thing; the above shouldn't hold up this series.

Mark.
diff mbox series

Patch

diff --git a/arch/arm64/include/asm/cmpxchg.h b/arch/arm64/include/asm/cmpxchg.h
index ae852add053d..bc9e07bc6428 100644
--- a/arch/arm64/include/asm/cmpxchg.h
+++ b/arch/arm64/include/asm/cmpxchg.h
@@ -18,7 +18,7 @@ 
 #ifndef __ASM_CMPXCHG_H
 #define __ASM_CMPXCHG_H
 
-#include <linux/bug.h>
+#include <linux/build_bug.h>
 
 #include <asm/atomic.h>
 #include <asm/barrier.h>