2020-07-29 11:07:33

by Stephen Rothwell

[permalink] [raw]
Subject: linux-next: build failure after merge of the printk tree

Hi all,

After merging the printk tree, today's linux-next build (powerpc
allyesconfig) failed like this:

In file included from include/linux/printk.h:10,
from include/linux/kernel.h:15,
from include/asm-generic/bug.h:20,
from arch/powerpc/include/asm/bug.h:109,
from include/linux/bug.h:5,
from arch/powerpc/include/asm/cmpxchg.h:8,
from arch/powerpc/include/asm/atomic.h:11,
from include/linux/atomic.h:7,
from include/asm-generic/qspinlock_types.h:19,
from arch/powerpc/include/asm/spinlock_types.h:10,
from include/linux/spinlock_types.h:13,
from include/linux/genalloc.h:32,
from drivers/soc/fsl/qe/qe_common.c:16:
include/linux/ratelimit_types.h:16:2: error: unknown type name 'raw_spinlock_t'
16 | raw_spinlock_t lock; /* protect the state */
| ^~~~~~~~~~~~~~
In file included from include/linux/wait.h:9,
from include/linux/pid.h:6,
from include/linux/sched.h:14,
from include/linux/ratelimit.h:6,
from include/linux/dev_printk.h:16,
from include/linux/device.h:15,
from include/linux/node.h:18,
from include/linux/cpu.h:17,
from include/linux/of_device.h:5,
from drivers/soc/fsl/qe/qe_common.c:19:
include/linux/ratelimit.h: In function 'ratelimit_state_init':
include/linux/ratelimit.h:14:21: error: passing argument 1 of '__raw_spin_lock_init' from incompatible pointer type [-Werror=incompatible-pointer-types]
14 | raw_spin_lock_init(&rs->lock);
include/linux/spinlock.h:103:24: note: in definition of macro 'raw_spin_lock_init'
103 | __raw_spin_lock_init((lock), #lock, &__key, LD_WAIT_SPIN); \
| ^~~~
include/linux/spinlock.h:96:52: note: expected 'raw_spinlock_t *' {aka 'struct raw_spinlock *'} but argument is of type 'int *'
96 | extern void __raw_spin_lock_init(raw_spinlock_t *lock, const char *name,
| ~~~~~~~~~~~~~~~~^~~~
In file included from arch/powerpc/include/asm/cmpxchg.h:8,
from arch/powerpc/include/asm/atomic.h:11,
from include/linux/atomic.h:7,
from include/asm-generic/qspinlock_types.h:19,
from arch/powerpc/include/asm/spinlock_types.h:10,
from include/linux/spinlock_types.h:13,
from include/linux/ratelimit_types.h:7,
from include/linux/printk.h:10,
from include/linux/kernel.h:15,
from include/asm-generic/bug.h:20,
from arch/powerpc/include/asm/bug.h:109,
from drivers/block/drbd/drbd_interval.c:2:
include/linux/bug.h:34:47: warning: 'struct bug_entry' declared inside parameter list will not be visible outside of this definition or declaration
34 | static inline int is_warning_bug(const struct bug_entry *bug)
| ^~~~~~~~~
include/linux/bug.h: In function 'is_warning_bug':
include/linux/bug.h:36:12: error: dereferencing pointer to incomplete type 'const struct bug_entry'
36 | return bug->flags & BUGFLAG_WARNING;
| ^~

And another similar.

Caused by commit

b4a461e72bcb ("printk: Make linux/printk.h self-contained")

This is becoming a bit of a whack-a-mole :-(

I have reverted that commit for today.
--
Cheers,
Stephen Rothwell


Attachments:
(No filename) (499.00 B)
OpenPGP digital signature

2020-07-29 11:51:02

by Herbert Xu

[permalink] [raw]
Subject: Re: linux-next: build failure after merge of the printk tree

On Wed, Jul 29, 2020 at 09:03:11PM +1000, Stephen Rothwell wrote:
>
> After merging the printk tree, today's linux-next build (powerpc
> allyesconfig) failed like this:

Hi Stephen:

This loop was introduced recently by the powerpc tree with

https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?h=next&id=aa65ff6b18e0366db1790609956a4ac7308c5668

powerpc/64s: Implement queued spinlocks and rwlocks

However the loop itself goes back further and in fact someone has
already tried to work around it by adding ifdefs on CONFIG_PARAVIRT
in asm-generic/qspinlock_types.h. I'll try to fix this properly.

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2020-07-29 12:29:11

by Herbert Xu

[permalink] [raw]
Subject: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop

This miniseries breaks a header loop involving qspinlock_types.h.
The issue is that qspinlock_types.h includes atomic.h, which then
eventually includes kernel.h which could lead back to the original
file via spinlock_types.h.

The first patch moves ATOMIC_INIT into linux/types.h while the second
patch actuallys breaks the loop by no longer including atomic.h
in qspinlock_types.h.

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2020-07-29 12:34:00

by Herbert Xu

[permalink] [raw]
Subject: [PATCH 2/2] locking/qspinlock: Do not include atomic.h from qspinlock_types.h

This patch breaks a header loop involving qspinlock_types.h.
The issue is that qspinlock_types.h includes atomic.h, which then
eventually includes kernel.h which could lead back to the original
file via spinlock_types.h.

As ATOMIC_INIT is now defined by linux/types.h, there is no longer
any need to include atomic.h from qspinlock_types.h. This also
allows the CONFIG_PARAVIRT hack to be removed since it was trying
to prevent exactly this loop.

Signed-off-by: Herbert Xu <[email protected]>

diff --git a/include/asm-generic/qspinlock.h b/include/asm-generic/qspinlock.h
index 38ca14e79a86..4fe7fd0fe834 100644
--- a/include/asm-generic/qspinlock.h
+++ b/include/asm-generic/qspinlock.h
@@ -11,6 +11,7 @@
#define __ASM_GENERIC_QSPINLOCK_H

#include <asm-generic/qspinlock_types.h>
+#include <linux/atomic.h>

#ifndef queued_spin_is_locked
/**
diff --git a/include/asm-generic/qspinlock_types.h b/include/asm-generic/qspinlock_types.h
index 56d1309d32f8..2fd1fb89ec36 100644
--- a/include/asm-generic/qspinlock_types.h
+++ b/include/asm-generic/qspinlock_types.h
@@ -9,15 +9,7 @@
#ifndef __ASM_GENERIC_QSPINLOCK_TYPES_H
#define __ASM_GENERIC_QSPINLOCK_TYPES_H

-/*
- * Including atomic.h with PARAVIRT on will cause compilation errors because
- * of recursive header file incluson via paravirt_types.h. So don't include
- * it if PARAVIRT is on.
- */
-#ifndef CONFIG_PARAVIRT
#include <linux/types.h>
-#include <linux/atomic.h>
-#endif

typedef struct qspinlock {
union {
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2020-07-29 12:34:20

by Herbert Xu

[permalink] [raw]
Subject: [PATCH 1/2] locking/atomic: Move ATOMIC_INIT into linux/types.h

This patch moves ATOMIC_INIT from asm/atomic.h into linux/types.h.
This allows users of atomic_t to use ATOMIC_INIT without having to
include atomic.h as that way may lead to header loops.

Signed-off-by: Herbert Xu <[email protected]>

diff --git a/arch/alpha/include/asm/atomic.h b/arch/alpha/include/asm/atomic.h
index 2144530d1428..e2093994fd0d 100644
--- a/arch/alpha/include/asm/atomic.h
+++ b/arch/alpha/include/asm/atomic.h
@@ -24,7 +24,6 @@
#define __atomic_acquire_fence()
#define __atomic_post_full_fence()

-#define ATOMIC_INIT(i) { (i) }
#define ATOMIC64_INIT(i) { (i) }

#define atomic_read(v) READ_ONCE((v)->counter)
diff --git a/arch/arc/include/asm/atomic.h b/arch/arc/include/asm/atomic.h
index 7298ce84762e..c614857eb209 100644
--- a/arch/arc/include/asm/atomic.h
+++ b/arch/arc/include/asm/atomic.h
@@ -14,8 +14,6 @@
#include <asm/barrier.h>
#include <asm/smp.h>

-#define ATOMIC_INIT(i) { (i) }
-
#ifndef CONFIG_ARC_PLAT_EZNPS

#define atomic_read(v) READ_ONCE((v)->counter)
diff --git a/arch/arm/include/asm/atomic.h b/arch/arm/include/asm/atomic.h
index 75bb2c543e59..455eb19a5ac1 100644
--- a/arch/arm/include/asm/atomic.h
+++ b/arch/arm/include/asm/atomic.h
@@ -15,8 +15,6 @@
#include <asm/barrier.h>
#include <asm/cmpxchg.h>

-#define ATOMIC_INIT(i) { (i) }
-
#ifdef __KERNEL__

/*
diff --git a/arch/arm64/include/asm/atomic.h b/arch/arm64/include/asm/atomic.h
index a08890da696c..015ddffaf6ca 100644
--- a/arch/arm64/include/asm/atomic.h
+++ b/arch/arm64/include/asm/atomic.h
@@ -99,8 +99,6 @@ static inline long arch_atomic64_dec_if_positive(atomic64_t *v)
return __lse_ll_sc_body(atomic64_dec_if_positive, v);
}

-#define ATOMIC_INIT(i) { (i) }
-
#define arch_atomic_read(v) __READ_ONCE((v)->counter)
#define arch_atomic_set(v, i) __WRITE_ONCE(((v)->counter), (i))

diff --git a/arch/h8300/include/asm/atomic.h b/arch/h8300/include/asm/atomic.h
index c6b6a06231b2..a990d151f163 100644
--- a/arch/h8300/include/asm/atomic.h
+++ b/arch/h8300/include/asm/atomic.h
@@ -12,8 +12,6 @@
* resource counting etc..
*/

-#define ATOMIC_INIT(i) { (i) }
-
#define atomic_read(v) READ_ONCE((v)->counter)
#define atomic_set(v, i) WRITE_ONCE(((v)->counter), (i))

diff --git a/arch/hexagon/include/asm/atomic.h b/arch/hexagon/include/asm/atomic.h
index 0231d69c8bf2..4ab895d7111f 100644
--- a/arch/hexagon/include/asm/atomic.h
+++ b/arch/hexagon/include/asm/atomic.h
@@ -12,8 +12,6 @@
#include <asm/cmpxchg.h>
#include <asm/barrier.h>

-#define ATOMIC_INIT(i) { (i) }
-
/* Normal writes in our arch don't clear lock reservations */

static inline void atomic_set(atomic_t *v, int new)
diff --git a/arch/ia64/include/asm/atomic.h b/arch/ia64/include/asm/atomic.h
index 50440f3ddc43..f267d956458f 100644
--- a/arch/ia64/include/asm/atomic.h
+++ b/arch/ia64/include/asm/atomic.h
@@ -19,7 +19,6 @@
#include <asm/barrier.h>


-#define ATOMIC_INIT(i) { (i) }
#define ATOMIC64_INIT(i) { (i) }

#define atomic_read(v) READ_ONCE((v)->counter)
diff --git a/arch/m68k/include/asm/atomic.h b/arch/m68k/include/asm/atomic.h
index 47228b0d4163..756c5cc58f94 100644
--- a/arch/m68k/include/asm/atomic.h
+++ b/arch/m68k/include/asm/atomic.h
@@ -16,8 +16,6 @@
* We do not have SMP m68k systems, so we don't have to deal with that.
*/

-#define ATOMIC_INIT(i) { (i) }
-
#define atomic_read(v) READ_ONCE((v)->counter)
#define atomic_set(v, i) WRITE_ONCE(((v)->counter), (i))

diff --git a/arch/mips/include/asm/atomic.h b/arch/mips/include/asm/atomic.h
index e5ac88392d1f..f904084fcb1f 100644
--- a/arch/mips/include/asm/atomic.h
+++ b/arch/mips/include/asm/atomic.h
@@ -45,7 +45,6 @@ static __always_inline type pfx##_xchg(pfx##_t *v, type n) \
return xchg(&v->counter, n); \
}

-#define ATOMIC_INIT(i) { (i) }
ATOMIC_OPS(atomic, int)

#ifdef CONFIG_64BIT
diff --git a/arch/parisc/include/asm/atomic.h b/arch/parisc/include/asm/atomic.h
index 118953d41763..f960e2f32b1b 100644
--- a/arch/parisc/include/asm/atomic.h
+++ b/arch/parisc/include/asm/atomic.h
@@ -136,8 +136,6 @@ ATOMIC_OPS(xor, ^=)
#undef ATOMIC_OP_RETURN
#undef ATOMIC_OP

-#define ATOMIC_INIT(i) { (i) }
-
#ifdef CONFIG_64BIT

#define ATOMIC64_INIT(i) { (i) }
diff --git a/arch/powerpc/include/asm/atomic.h b/arch/powerpc/include/asm/atomic.h
index f6a3d145ffb7..8a55eb8cc97b 100644
--- a/arch/powerpc/include/asm/atomic.h
+++ b/arch/powerpc/include/asm/atomic.h
@@ -11,8 +11,6 @@
#include <asm/cmpxchg.h>
#include <asm/barrier.h>

-#define ATOMIC_INIT(i) { (i) }
-
/*
* Since *_return_relaxed and {cmp}xchg_relaxed are implemented with
* a "bne-" instruction at the end, so an isync is enough as a acquire barrier
diff --git a/arch/riscv/include/asm/atomic.h b/arch/riscv/include/asm/atomic.h
index 96f95c9ebd97..400a8c8b6de7 100644
--- a/arch/riscv/include/asm/atomic.h
+++ b/arch/riscv/include/asm/atomic.h
@@ -19,8 +19,6 @@
#include <asm/cmpxchg.h>
#include <asm/barrier.h>

-#define ATOMIC_INIT(i) { (i) }
-
#define __atomic_acquire_fence() \
__asm__ __volatile__(RISCV_ACQUIRE_BARRIER "" ::: "memory")

diff --git a/arch/s390/include/asm/atomic.h b/arch/s390/include/asm/atomic.h
index 491ad53a0d4e..cae473a7b6f7 100644
--- a/arch/s390/include/asm/atomic.h
+++ b/arch/s390/include/asm/atomic.h
@@ -15,8 +15,6 @@
#include <asm/barrier.h>
#include <asm/cmpxchg.h>

-#define ATOMIC_INIT(i) { (i) }
-
static inline int atomic_read(const atomic_t *v)
{
int c;
diff --git a/arch/sh/include/asm/atomic.h b/arch/sh/include/asm/atomic.h
index f37b95a80232..7c2a8a703b9a 100644
--- a/arch/sh/include/asm/atomic.h
+++ b/arch/sh/include/asm/atomic.h
@@ -19,8 +19,6 @@
#include <asm/cmpxchg.h>
#include <asm/barrier.h>

-#define ATOMIC_INIT(i) { (i) }
-
#define atomic_read(v) READ_ONCE((v)->counter)
#define atomic_set(v,i) WRITE_ONCE((v)->counter, (i))

diff --git a/arch/sparc/include/asm/atomic_32.h b/arch/sparc/include/asm/atomic_32.h
index 94c930f0bc62..efad5532f169 100644
--- a/arch/sparc/include/asm/atomic_32.h
+++ b/arch/sparc/include/asm/atomic_32.h
@@ -18,8 +18,6 @@
#include <asm/barrier.h>
#include <asm-generic/atomic64.h>

-#define ATOMIC_INIT(i) { (i) }
-
int atomic_add_return(int, atomic_t *);
int atomic_fetch_add(int, atomic_t *);
int atomic_fetch_and(int, atomic_t *);
diff --git a/arch/sparc/include/asm/atomic_64.h b/arch/sparc/include/asm/atomic_64.h
index b60448397d4f..6b235d3d1d9d 100644
--- a/arch/sparc/include/asm/atomic_64.h
+++ b/arch/sparc/include/asm/atomic_64.h
@@ -12,7 +12,6 @@
#include <asm/cmpxchg.h>
#include <asm/barrier.h>

-#define ATOMIC_INIT(i) { (i) }
#define ATOMIC64_INIT(i) { (i) }

#define atomic_read(v) READ_ONCE((v)->counter)
diff --git a/arch/x86/include/asm/atomic.h b/arch/x86/include/asm/atomic.h
index bf35e476a776..b6cac6e9bb70 100644
--- a/arch/x86/include/asm/atomic.h
+++ b/arch/x86/include/asm/atomic.h
@@ -14,8 +14,6 @@
* resource counting etc..
*/

-#define ATOMIC_INIT(i) { (i) }
-
/**
* arch_atomic_read - read atomic variable
* @v: pointer of type atomic_t
diff --git a/arch/xtensa/include/asm/atomic.h b/arch/xtensa/include/asm/atomic.h
index 3e7c6134ed32..744c2f463845 100644
--- a/arch/xtensa/include/asm/atomic.h
+++ b/arch/xtensa/include/asm/atomic.h
@@ -19,8 +19,6 @@
#include <asm/cmpxchg.h>
#include <asm/barrier.h>

-#define ATOMIC_INIT(i) { (i) }
-
/*
* This Xtensa implementation assumes that the right mechanism
* for exclusion is for locking interrupts to level EXCM_LEVEL.
diff --git a/include/asm-generic/atomic.h b/include/asm-generic/atomic.h
index 286867f593d2..11f96f40f4a7 100644
--- a/include/asm-generic/atomic.h
+++ b/include/asm-generic/atomic.h
@@ -159,8 +159,6 @@ ATOMIC_OP(xor, ^)
* resource counting etc..
*/

-#define ATOMIC_INIT(i) { (i) }
-
/**
* atomic_read - read atomic variable
* @v: pointer of type atomic_t
diff --git a/include/linux/types.h b/include/linux/types.h
index d3021c879179..a147977602b5 100644
--- a/include/linux/types.h
+++ b/include/linux/types.h
@@ -167,6 +167,8 @@ typedef struct {
int counter;
} atomic_t;

+#define ATOMIC_INIT(i) { (i) }
+
#ifdef CONFIG_64BIT
typedef struct {
s64 counter;
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2020-07-29 12:49:25

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop

On Wed, Jul 29, 2020 at 10:28:07PM +1000, Herbert Xu wrote:
> This miniseries breaks a header loop involving qspinlock_types.h.
> The issue is that qspinlock_types.h includes atomic.h, which then
> eventually includes kernel.h which could lead back to the original
> file via spinlock_types.h.

How did you run into this, I haven't seen any build failures due to
this.

> The first patch moves ATOMIC_INIT into linux/types.h while the second
> patch actuallys breaks the loop by no longer including atomic.h
> in qspinlock_types.h.

Anyway, the patches look sane enough, I'll go stick them in
tip/locking/core or somesuch.

2020-07-29 12:52:51

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop

On Wed, Jul 29, 2020 at 02:47:44PM +0200, [email protected] wrote:
>
> How did you run into this, I haven't seen any build failures due to
> this.

I didn't, Stephen ran into it after merging the printk tree together
with the powerpc tree:

https://lore.kernel.org/lkml/[email protected]/

> Anyway, the patches look sane enough, I'll go stick them in
> tip/locking/core or somesuch.

Perhaps add them on top of the other two patches in locking/header?

Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2020-07-29 13:01:09

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop

On Wed, Jul 29, 2020 at 10:51:44PM +1000, Herbert Xu wrote:
> On Wed, Jul 29, 2020 at 02:47:44PM +0200, [email protected] wrote:
> >
> > How did you run into this, I haven't seen any build failures due to
> > this.
>
> I didn't, Stephen ran into it after merging the printk tree together
> with the powerpc tree:
>
> https://lore.kernel.org/lkml/[email protected]/
>
> > Anyway, the patches look sane enough, I'll go stick them in
> > tip/locking/core or somesuch.
>
> Perhaps add them on top of the other two patches in locking/header?

Can do,

2020-07-29 13:32:15

by Sergey Senozhatsky

[permalink] [raw]
Subject: Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop

On (20/07/29 15:00), [email protected] wrote:
> On Wed, Jul 29, 2020 at 10:51:44PM +1000, Herbert Xu wrote:
> > On Wed, Jul 29, 2020 at 02:47:44PM +0200, [email protected] wrote:
> > >
[..]
> > > Anyway, the patches look sane enough, I'll go stick them in
> > > tip/locking/core or somesuch.
> >
> > Perhaps add them on top of the other two patches in locking/header?
>
> Can do,

locking/header would be better

-ss

2020-07-29 13:36:36

by Waiman Long

[permalink] [raw]
Subject: Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop

On 7/29/20 8:28 AM, Herbert Xu wrote:
> This miniseries breaks a header loop involving qspinlock_types.h.
> The issue is that qspinlock_types.h includes atomic.h, which then
> eventually includes kernel.h which could lead back to the original
> file via spinlock_types.h.
>
> The first patch moves ATOMIC_INIT into linux/types.h while the second
> patch actuallys breaks the loop by no longer including atomic.h
> in qspinlock_types.h.
>
> Cheers,

This patch series looks good to me. I just wonder if we should also move
ATOMIC64_INIT() to types.h for symmetry purpose. Anyway,

Acked-by: Waiman Long <[email protected]>

2020-07-29 14:32:02

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop

On Wed, Jul 29, 2020 at 10:28:49PM +0900, Sergey Senozhatsky wrote:
> On (20/07/29 15:00), [email protected] wrote:
> > On Wed, Jul 29, 2020 at 10:51:44PM +1000, Herbert Xu wrote:
> > > On Wed, Jul 29, 2020 at 02:47:44PM +0200, [email protected] wrote:
> > > >
> [..]
> > > > Anyway, the patches look sane enough, I'll go stick them in
> > > > tip/locking/core or somesuch.
> > >
> > > Perhaps add them on top of the other two patches in locking/header?
> >
> > Can do,
>
> locking/header would be better

Done.

Subject: [tip: locking/core] locking/atomic: Move ATOMIC_INIT into linux/types.h

The following commit has been merged into the locking/core branch of tip:

Commit-ID: 7ca8cf5347f720b07a0b32a924b768f5710547e7
Gitweb: https://git.kernel.org/tip/7ca8cf5347f720b07a0b32a924b768f5710547e7
Author: Herbert Xu <[email protected]>
AuthorDate: Wed, 29 Jul 2020 22:31:05 +10:00
Committer: Peter Zijlstra <[email protected]>
CommitterDate: Wed, 29 Jul 2020 16:14:18 +02:00

locking/atomic: Move ATOMIC_INIT into linux/types.h

This patch moves ATOMIC_INIT from asm/atomic.h into linux/types.h.
This allows users of atomic_t to use ATOMIC_INIT without having to
include atomic.h as that way may lead to header loops.

Signed-off-by: Herbert Xu <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Acked-by: Waiman Long <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
arch/alpha/include/asm/atomic.h | 1 -
arch/arc/include/asm/atomic.h | 2 --
arch/arm/include/asm/atomic.h | 2 --
arch/arm64/include/asm/atomic.h | 2 --
arch/h8300/include/asm/atomic.h | 2 --
arch/hexagon/include/asm/atomic.h | 2 --
arch/ia64/include/asm/atomic.h | 1 -
arch/m68k/include/asm/atomic.h | 2 --
arch/mips/include/asm/atomic.h | 1 -
arch/parisc/include/asm/atomic.h | 2 --
arch/powerpc/include/asm/atomic.h | 2 --
arch/riscv/include/asm/atomic.h | 2 --
arch/s390/include/asm/atomic.h | 2 --
arch/sh/include/asm/atomic.h | 2 --
arch/sparc/include/asm/atomic_32.h | 2 --
arch/sparc/include/asm/atomic_64.h | 1 -
arch/x86/include/asm/atomic.h | 2 --
arch/xtensa/include/asm/atomic.h | 2 --
include/asm-generic/atomic.h | 2 --
include/linux/types.h | 2 ++
20 files changed, 2 insertions(+), 34 deletions(-)

diff --git a/arch/alpha/include/asm/atomic.h b/arch/alpha/include/asm/atomic.h
index 2144530..e209399 100644
--- a/arch/alpha/include/asm/atomic.h
+++ b/arch/alpha/include/asm/atomic.h
@@ -24,7 +24,6 @@
#define __atomic_acquire_fence()
#define __atomic_post_full_fence()

-#define ATOMIC_INIT(i) { (i) }
#define ATOMIC64_INIT(i) { (i) }

#define atomic_read(v) READ_ONCE((v)->counter)
diff --git a/arch/arc/include/asm/atomic.h b/arch/arc/include/asm/atomic.h
index 7298ce8..c614857 100644
--- a/arch/arc/include/asm/atomic.h
+++ b/arch/arc/include/asm/atomic.h
@@ -14,8 +14,6 @@
#include <asm/barrier.h>
#include <asm/smp.h>

-#define ATOMIC_INIT(i) { (i) }
-
#ifndef CONFIG_ARC_PLAT_EZNPS

#define atomic_read(v) READ_ONCE((v)->counter)
diff --git a/arch/arm/include/asm/atomic.h b/arch/arm/include/asm/atomic.h
index 75bb2c5..455eb19 100644
--- a/arch/arm/include/asm/atomic.h
+++ b/arch/arm/include/asm/atomic.h
@@ -15,8 +15,6 @@
#include <asm/barrier.h>
#include <asm/cmpxchg.h>

-#define ATOMIC_INIT(i) { (i) }
-
#ifdef __KERNEL__

/*
diff --git a/arch/arm64/include/asm/atomic.h b/arch/arm64/include/asm/atomic.h
index a08890d..015ddff 100644
--- a/arch/arm64/include/asm/atomic.h
+++ b/arch/arm64/include/asm/atomic.h
@@ -99,8 +99,6 @@ static inline long arch_atomic64_dec_if_positive(atomic64_t *v)
return __lse_ll_sc_body(atomic64_dec_if_positive, v);
}

-#define ATOMIC_INIT(i) { (i) }
-
#define arch_atomic_read(v) __READ_ONCE((v)->counter)
#define arch_atomic_set(v, i) __WRITE_ONCE(((v)->counter), (i))

diff --git a/arch/h8300/include/asm/atomic.h b/arch/h8300/include/asm/atomic.h
index c6b6a06..a990d15 100644
--- a/arch/h8300/include/asm/atomic.h
+++ b/arch/h8300/include/asm/atomic.h
@@ -12,8 +12,6 @@
* resource counting etc..
*/

-#define ATOMIC_INIT(i) { (i) }
-
#define atomic_read(v) READ_ONCE((v)->counter)
#define atomic_set(v, i) WRITE_ONCE(((v)->counter), (i))

diff --git a/arch/hexagon/include/asm/atomic.h b/arch/hexagon/include/asm/atomic.h
index 0231d69..4ab895d 100644
--- a/arch/hexagon/include/asm/atomic.h
+++ b/arch/hexagon/include/asm/atomic.h
@@ -12,8 +12,6 @@
#include <asm/cmpxchg.h>
#include <asm/barrier.h>

-#define ATOMIC_INIT(i) { (i) }
-
/* Normal writes in our arch don't clear lock reservations */

static inline void atomic_set(atomic_t *v, int new)
diff --git a/arch/ia64/include/asm/atomic.h b/arch/ia64/include/asm/atomic.h
index 50440f3..f267d95 100644
--- a/arch/ia64/include/asm/atomic.h
+++ b/arch/ia64/include/asm/atomic.h
@@ -19,7 +19,6 @@
#include <asm/barrier.h>


-#define ATOMIC_INIT(i) { (i) }
#define ATOMIC64_INIT(i) { (i) }

#define atomic_read(v) READ_ONCE((v)->counter)
diff --git a/arch/m68k/include/asm/atomic.h b/arch/m68k/include/asm/atomic.h
index 47228b0..756c5cc 100644
--- a/arch/m68k/include/asm/atomic.h
+++ b/arch/m68k/include/asm/atomic.h
@@ -16,8 +16,6 @@
* We do not have SMP m68k systems, so we don't have to deal with that.
*/

-#define ATOMIC_INIT(i) { (i) }
-
#define atomic_read(v) READ_ONCE((v)->counter)
#define atomic_set(v, i) WRITE_ONCE(((v)->counter), (i))

diff --git a/arch/mips/include/asm/atomic.h b/arch/mips/include/asm/atomic.h
index e5ac883..f904084 100644
--- a/arch/mips/include/asm/atomic.h
+++ b/arch/mips/include/asm/atomic.h
@@ -45,7 +45,6 @@ static __always_inline type pfx##_xchg(pfx##_t *v, type n) \
return xchg(&v->counter, n); \
}

-#define ATOMIC_INIT(i) { (i) }
ATOMIC_OPS(atomic, int)

#ifdef CONFIG_64BIT
diff --git a/arch/parisc/include/asm/atomic.h b/arch/parisc/include/asm/atomic.h
index 118953d..f960e2f 100644
--- a/arch/parisc/include/asm/atomic.h
+++ b/arch/parisc/include/asm/atomic.h
@@ -136,8 +136,6 @@ ATOMIC_OPS(xor, ^=)
#undef ATOMIC_OP_RETURN
#undef ATOMIC_OP

-#define ATOMIC_INIT(i) { (i) }
-
#ifdef CONFIG_64BIT

#define ATOMIC64_INIT(i) { (i) }
diff --git a/arch/powerpc/include/asm/atomic.h b/arch/powerpc/include/asm/atomic.h
index 498785f..0311c3c 100644
--- a/arch/powerpc/include/asm/atomic.h
+++ b/arch/powerpc/include/asm/atomic.h
@@ -11,8 +11,6 @@
#include <asm/cmpxchg.h>
#include <asm/barrier.h>

-#define ATOMIC_INIT(i) { (i) }
-
/*
* Since *_return_relaxed and {cmp}xchg_relaxed are implemented with
* a "bne-" instruction at the end, so an isync is enough as a acquire barrier
diff --git a/arch/riscv/include/asm/atomic.h b/arch/riscv/include/asm/atomic.h
index 96f95c9..400a8c8 100644
--- a/arch/riscv/include/asm/atomic.h
+++ b/arch/riscv/include/asm/atomic.h
@@ -19,8 +19,6 @@
#include <asm/cmpxchg.h>
#include <asm/barrier.h>

-#define ATOMIC_INIT(i) { (i) }
-
#define __atomic_acquire_fence() \
__asm__ __volatile__(RISCV_ACQUIRE_BARRIER "" ::: "memory")

diff --git a/arch/s390/include/asm/atomic.h b/arch/s390/include/asm/atomic.h
index 491ad53..cae473a 100644
--- a/arch/s390/include/asm/atomic.h
+++ b/arch/s390/include/asm/atomic.h
@@ -15,8 +15,6 @@
#include <asm/barrier.h>
#include <asm/cmpxchg.h>

-#define ATOMIC_INIT(i) { (i) }
-
static inline int atomic_read(const atomic_t *v)
{
int c;
diff --git a/arch/sh/include/asm/atomic.h b/arch/sh/include/asm/atomic.h
index f37b95a..7c2a8a7 100644
--- a/arch/sh/include/asm/atomic.h
+++ b/arch/sh/include/asm/atomic.h
@@ -19,8 +19,6 @@
#include <asm/cmpxchg.h>
#include <asm/barrier.h>

-#define ATOMIC_INIT(i) { (i) }
-
#define atomic_read(v) READ_ONCE((v)->counter)
#define atomic_set(v,i) WRITE_ONCE((v)->counter, (i))

diff --git a/arch/sparc/include/asm/atomic_32.h b/arch/sparc/include/asm/atomic_32.h
index 94c930f..efad553 100644
--- a/arch/sparc/include/asm/atomic_32.h
+++ b/arch/sparc/include/asm/atomic_32.h
@@ -18,8 +18,6 @@
#include <asm/barrier.h>
#include <asm-generic/atomic64.h>

-#define ATOMIC_INIT(i) { (i) }
-
int atomic_add_return(int, atomic_t *);
int atomic_fetch_add(int, atomic_t *);
int atomic_fetch_and(int, atomic_t *);
diff --git a/arch/sparc/include/asm/atomic_64.h b/arch/sparc/include/asm/atomic_64.h
index b604483..6b235d3 100644
--- a/arch/sparc/include/asm/atomic_64.h
+++ b/arch/sparc/include/asm/atomic_64.h
@@ -12,7 +12,6 @@
#include <asm/cmpxchg.h>
#include <asm/barrier.h>

-#define ATOMIC_INIT(i) { (i) }
#define ATOMIC64_INIT(i) { (i) }

#define atomic_read(v) READ_ONCE((v)->counter)
diff --git a/arch/x86/include/asm/atomic.h b/arch/x86/include/asm/atomic.h
index bf35e47..b6cac6e 100644
--- a/arch/x86/include/asm/atomic.h
+++ b/arch/x86/include/asm/atomic.h
@@ -14,8 +14,6 @@
* resource counting etc..
*/

-#define ATOMIC_INIT(i) { (i) }
-
/**
* arch_atomic_read - read atomic variable
* @v: pointer of type atomic_t
diff --git a/arch/xtensa/include/asm/atomic.h b/arch/xtensa/include/asm/atomic.h
index 3e7c613..744c2f4 100644
--- a/arch/xtensa/include/asm/atomic.h
+++ b/arch/xtensa/include/asm/atomic.h
@@ -19,8 +19,6 @@
#include <asm/cmpxchg.h>
#include <asm/barrier.h>

-#define ATOMIC_INIT(i) { (i) }
-
/*
* This Xtensa implementation assumes that the right mechanism
* for exclusion is for locking interrupts to level EXCM_LEVEL.
diff --git a/include/asm-generic/atomic.h b/include/asm-generic/atomic.h
index 286867f..11f96f4 100644
--- a/include/asm-generic/atomic.h
+++ b/include/asm-generic/atomic.h
@@ -159,8 +159,6 @@ ATOMIC_OP(xor, ^)
* resource counting etc..
*/

-#define ATOMIC_INIT(i) { (i) }
-
/**
* atomic_read - read atomic variable
* @v: pointer of type atomic_t
diff --git a/include/linux/types.h b/include/linux/types.h
index d3021c8..a147977 100644
--- a/include/linux/types.h
+++ b/include/linux/types.h
@@ -167,6 +167,8 @@ typedef struct {
int counter;
} atomic_t;

+#define ATOMIC_INIT(i) { (i) }
+
#ifdef CONFIG_64BIT
typedef struct {
s64 counter;

Subject: [tip: locking/core] locking/qspinlock: Do not include atomic.h from qspinlock_types.h

The following commit has been merged into the locking/core branch of tip:

Commit-ID: 459e39538e612b8dd130d34b93c9bfc89ecc836c
Gitweb: https://git.kernel.org/tip/459e39538e612b8dd130d34b93c9bfc89ecc836c
Author: Herbert Xu <[email protected]>
AuthorDate: Wed, 29 Jul 2020 22:33:16 +10:00
Committer: Peter Zijlstra <[email protected]>
CommitterDate: Wed, 29 Jul 2020 16:14:19 +02:00

locking/qspinlock: Do not include atomic.h from qspinlock_types.h

This patch breaks a header loop involving qspinlock_types.h.
The issue is that qspinlock_types.h includes atomic.h, which then
eventually includes kernel.h which could lead back to the original
file via spinlock_types.h.

As ATOMIC_INIT is now defined by linux/types.h, there is no longer
any need to include atomic.h from qspinlock_types.h. This also
allows the CONFIG_PARAVIRT hack to be removed since it was trying
to prevent exactly this loop.

Signed-off-by: Herbert Xu <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Acked-by: Waiman Long <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
include/asm-generic/qspinlock.h | 1 +
include/asm-generic/qspinlock_types.h | 8 --------
2 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/include/asm-generic/qspinlock.h b/include/asm-generic/qspinlock.h
index fde943d..2b26cd7 100644
--- a/include/asm-generic/qspinlock.h
+++ b/include/asm-generic/qspinlock.h
@@ -11,6 +11,7 @@
#define __ASM_GENERIC_QSPINLOCK_H

#include <asm-generic/qspinlock_types.h>
+#include <linux/atomic.h>

/**
* queued_spin_is_locked - is the spinlock locked?
diff --git a/include/asm-generic/qspinlock_types.h b/include/asm-generic/qspinlock_types.h
index 56d1309..2fd1fb8 100644
--- a/include/asm-generic/qspinlock_types.h
+++ b/include/asm-generic/qspinlock_types.h
@@ -9,15 +9,7 @@
#ifndef __ASM_GENERIC_QSPINLOCK_TYPES_H
#define __ASM_GENERIC_QSPINLOCK_TYPES_H

-/*
- * Including atomic.h with PARAVIRT on will cause compilation errors because
- * of recursive header file incluson via paravirt_types.h. So don't include
- * it if PARAVIRT is on.
- */
-#ifndef CONFIG_PARAVIRT
#include <linux/types.h>
-#include <linux/atomic.h>
-#endif

typedef struct qspinlock {
union {

2020-07-29 15:06:48

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop

On Wed, Jul 29, 2020 at 4:35 PM Waiman Long <[email protected]> wrote:
> On 7/29/20 8:28 AM, Herbert Xu wrote:

...

> This patch series looks good to me. I just wonder if we should also move
> ATOMIC64_INIT() to types.h for symmetry purpose. Anyway,

Same question here.

--
With Best Regards,
Andy Shevchenko

2020-07-29 16:44:33

by Sergey Senozhatsky

[permalink] [raw]
Subject: Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop

On (20/07/29 16:28), [email protected] wrote:
> On Wed, Jul 29, 2020 at 10:28:49PM +0900, Sergey Senozhatsky wrote:
> > On (20/07/29 15:00), [email protected] wrote:
> > > On Wed, Jul 29, 2020 at 10:51:44PM +1000, Herbert Xu wrote:
> > > > On Wed, Jul 29, 2020 at 02:47:44PM +0200, [email protected] wrote:
> > > > >
> > [..]
> > > > > Anyway, the patches look sane enough, I'll go stick them in
> > > > > tip/locking/core or somesuch.
> > > >
> > > > Perhaps add them on top of the other two patches in locking/header?
> > >
> > > Can do,
> >
> > locking/header would be better
>
> Done.

Thanks a lot!
I'll run some cross-compilation tests.

-ss

2020-07-29 17:53:15

by Sergey Senozhatsky

[permalink] [raw]
Subject: Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop

On (20/07/29 22:28), Herbert Xu wrote:
> This miniseries breaks a header loop involving qspinlock_types.h.
> The issue is that qspinlock_types.h includes atomic.h, which then
> eventually includes kernel.h which could lead back to the original
> file via spinlock_types.h.
>
> The first patch moves ATOMIC_INIT into linux/types.h while the second
> patch actuallys breaks the loop by no longer including atomic.h
> in qspinlock_types.h.

Thanks for staying on this.

I pulled locking/header, run some cross-compilation tests locally
(powerpc, arm, arm64, x86) and pushed printk/for-next. Let's see.

-ss

2020-07-30 01:03:26

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop

On Wed, Jul 29, 2020 at 06:04:57PM +0300, Andy Shevchenko wrote:
> On Wed, Jul 29, 2020 at 4:35 PM Waiman Long <[email protected]> wrote:
> > On 7/29/20 8:28 AM, Herbert Xu wrote:
>
> ...
>
> > This patch series looks good to me. I just wonder if we should also move
> > ATOMIC64_INIT() to types.h for symmetry purpose. Anyway,
>
> Same question here.

Yes I almost started doing it but at least one architecture (arc)
had a custom atomic64_t so I kept it out just to be on the safe
side.

We certainly could do this as a follow-up patch.

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2020-07-30 07:48:09

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop

On Thu, Jul 30, 2020 at 4:00 AM Herbert Xu <[email protected]> wrote:
>
> On Wed, Jul 29, 2020 at 06:04:57PM +0300, Andy Shevchenko wrote:
> > On Wed, Jul 29, 2020 at 4:35 PM Waiman Long <[email protected]> wrote:
> > > On 7/29/20 8:28 AM, Herbert Xu wrote:
> >
> > ...
> >
> > > This patch series looks good to me. I just wonder if we should also move
> > > ATOMIC64_INIT() to types.h for symmetry purpose. Anyway,
> >
> > Same question here.
>
> Yes I almost started doing it but at least one architecture (arc)
> had a custom atomic64_t so I kept it out just to be on the safe
> side.

We may ask Synopsys folks to look at this as well.
Vineet, any ideas if we may unify ATOMIC64_INIT() across the architectures?

> We certainly could do this as a follow-up patch.

--
With Best Regards,
Andy Shevchenko

2020-07-30 07:54:04

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop

On Thu, Jul 30, 2020 at 10:47:16AM +0300, Andy Shevchenko wrote:
>
> We may ask Synopsys folks to look at this as well.
> Vineet, any ideas if we may unify ATOMIC64_INIT() across the architectures?

I don't think there is any technical difficulty. The custom
atomic64_t simply adds an alignment requirement so the initialisor
remains the same.

I just didn't want to add more complexity to the existing patch.
As a follow-up patch it should be quite straightforward.

Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2020-07-30 07:57:05

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop

On Thu, Jul 30, 2020 at 10:51 AM Herbert Xu <[email protected]> wrote:
> On Thu, Jul 30, 2020 at 10:47:16AM +0300, Andy Shevchenko wrote:
> >
> > We may ask Synopsys folks to look at this as well.
> > Vineet, any ideas if we may unify ATOMIC64_INIT() across the architectures?
>
> I don't think there is any technical difficulty. The custom
> atomic64_t simply adds an alignment requirement so the initialisor
> remains the same.
>
> I just didn't want to add more complexity to the existing patch.
> As a follow-up patch it should be quite straightforward.

Ah, I see what you mean. I considered the follow up patch as well, I
thought there were some bigger impediments.

--
With Best Regards,
Andy Shevchenko

2020-08-06 03:38:31

by Vineet Gupta

[permalink] [raw]
Subject: Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop

On 7/30/20 12:50 AM, Herbert Xu wrote:
> On Thu, Jul 30, 2020 at 10:47:16AM +0300, Andy Shevchenko wrote:
>> We may ask Synopsys folks to look at this as well.
>> Vineet, any ideas if we may unify ATOMIC64_INIT() across the architectures?
> I don't think there is any technical difficulty. The custom
> atomic64_t simply adds an alignment requirement so the initialisor
> remains the same.

Exactly so.

FWIW the alignment requirement is because ARC ABI allows 64-bit data to be 32-bit
aligned provided hardware deals fine with 4 byte aligned for the non-atomic
double-load/store LDD/STD instructions. The 64-bit alignement however is required
for atomic double load/store LLOCKD/SCONDD instructions hence the definition of
ARC atomic64_t

-Vineet