linux-next: build failure after merge of the printk tree

linux-next: build failure after merge of the printk tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 3670 bytes --]

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build failure after merge of the printk tree

[Herbert Xu]

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 <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

[Herbert Xu]

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 <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

[Herbert Xu]

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 <herbert@gondor.apana.org.au>

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 <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

[Herbert Xu]

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 <herbert@gondor.apana.org.au>

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 <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

[peterz]

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.

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

[Herbert Xu]

On Wed, Jul 29, 2020 at 02:47:44PM +0200, peterz@infradead.org 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/20200729210311.425d0e9b@canb.auug.org.au/

> 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 <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

[peterz]

On Wed, Jul 29, 2020 at 10:51:44PM +1000, Herbert Xu wrote:
> On Wed, Jul 29, 2020 at 02:47:44PM +0200, peterz@infradead.org 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/20200729210311.425d0e9b@canb.auug.org.au/
> 
> > 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,

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

[Sergey Senozhatsky]

On (20/07/29 15:00), peterz@infradead.org wrote:
> On Wed, Jul 29, 2020 at 10:51:44PM +1000, Herbert Xu wrote:
> > On Wed, Jul 29, 2020 at 02:47:44PM +0200, peterz@infradead.org 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

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

[Waiman Long]

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 <longman@redhat.com>

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

[peterz]

On Wed, Jul 29, 2020 at 10:28:49PM +0900, Sergey Senozhatsky wrote:
> On (20/07/29 15:00), peterz@infradead.org wrote:
> > On Wed, Jul 29, 2020 at 10:51:44PM +1000, Herbert Xu wrote:
> > > On Wed, Jul 29, 2020 at 02:47:44PM +0200, peterz@infradead.org 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.

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

[tip-bot2 for Herbert Xu]

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 <herbert@gondor.apana.org.au>
AuthorDate:    Wed, 29 Jul 2020 22:33:16 +10:00
Committer:     Peter Zijlstra <peterz@infradead.org>
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 <herbert@gondor.apana.org.au>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Waiman Long <longman@redhat.com>
Link: https://lkml.kernel.org/r/20200729123316.GC7047@gondor.apana.org.au
---
 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 {

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

[tip-bot2 for Herbert Xu]

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 <herbert@gondor.apana.org.au>
AuthorDate:    Wed, 29 Jul 2020 22:31:05 +10:00
Committer:     Peter Zijlstra <peterz@infradead.org>
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 <herbert@gondor.apana.org.au>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Waiman Long <longman@redhat.com>
Link: https://lkml.kernel.org/r/20200729123105.GB7047@gondor.apana.org.au
---
 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;

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

[Andy Shevchenko]

On Wed, Jul 29, 2020 at 4:35 PM Waiman Long <longman@redhat.com> 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

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

[Sergey Senozhatsky]

On (20/07/29 16:28), peterz@infradead.org wrote:
> On Wed, Jul 29, 2020 at 10:28:49PM +0900, Sergey Senozhatsky wrote:
> > On (20/07/29 15:00), peterz@infradead.org wrote:
> > > On Wed, Jul 29, 2020 at 10:51:44PM +1000, Herbert Xu wrote:
> > > > On Wed, Jul 29, 2020 at 02:47:44PM +0200, peterz@infradead.org 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

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

[Sergey Senozhatsky]

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

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

[Herbert Xu]

On Wed, Jul 29, 2020 at 06:04:57PM +0300, Andy Shevchenko wrote:
> On Wed, Jul 29, 2020 at 4:35 PM Waiman Long <longman@redhat.com> 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 <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

[Andy Shevchenko]

On Thu, Jul 30, 2020 at 4:00 AM Herbert Xu <herbert@gondor.apana.org.au> 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 <longman@redhat.com> 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

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

[Herbert Xu]

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 <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

[Andy Shevchenko]

On Thu, Jul 30, 2020 at 10:51 AM Herbert Xu <herbert@gondor.apana.org.au> 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

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

[Vineet Gupta]

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

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

[Vineet Gupta]

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
_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: linux-next: build failure after merge of the printk tree

[Petr Mladek]

On Mon 2023-01-16 10:23:01, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the printk tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
> 
> ERROR: modpost: "console_srcu_read_lock_is_held" [drivers/tty/serial/kgdboc.ko] undefined!
> 
> Caused by commit
> 
>   75ffddf01347 ("tty: serial: kgdboc: fix mutex locking order for configure_kgdboc()")

Great catch! Thanks a lot for the report.

It gets fixed by:

--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -123,6 +123,7 @@ bool console_srcu_read_lock_is_held(void)
 {
 	return srcu_read_lock_held(&console_srcu);
 }
+EXPORT_SYMBOL(console_srcu_read_lock_is_held);
 #endif
 
 enum devkmsg_log_bits {


I have added this fix by ammending the problematic commit so that we
do not break bisection, see
https://git.kernel.org/pub/scm/linux/kernel/git/printk/linux.git/commit/?h=rework/console-list-lock&id=3ef5abd9b5c738bc6fa9a65e40331ef2cb03ad9c

Best Regards,
Petr

linux-next: build failure after merge of the printk tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 408 bytes --]

Hi all,

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

ERROR: modpost: "console_srcu_read_lock_is_held" [drivers/tty/serial/kgdboc.ko] undefined!

Caused by commit

  75ffddf01347 ("tty: serial: kgdboc: fix mutex locking order for configure_kgdboc()")

I have used the printk tree from next-20230113 for today.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build failure after merge of the printk tree

[Petr Mladek]

On Mon 2022-11-21 11:00:41, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the printk tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
> 
> ERROR: modpost: "lockdep_assert_console_list_lock_held" [drivers/video/fbdev/xen-fbfront.ko] undefined!
> ERROR: modpost: "lockdep_assert_console_list_lock_held" [drivers/tty/serial/8250/8250.ko] undefined!
> ERROR: modpost: "lockdep_assert_console_list_lock_held" [drivers/tty/serial/samsung_tty.ko] undefined!
> ERROR: modpost: "lockdep_assert_console_list_lock_held" [drivers/tty/serial/kgdboc.ko] undefined!
> ERROR: modpost: "lockdep_assert_console_list_lock_held" [drivers/net/netconsole.ko] undefined!
> 
> Presumably caused by commits
> 
>   c26c1533e3bd ("printk: introduce console_list_lock")
>   a71ba89dc3ed ("console: introduce wrappers to read/write console flags")
>   d3ca62bf29bb ("console: introduce console_is_registered()")

John has fixed this by adding EXPORT_SYMBOL() for this function.

> I have used the printk tree from next-20221118 for today.

It is now fixed in "for-next" branch in printk/linux.git.

Thanks a lot for the report!

Best Regards,
Petr

linux-next: build failure after merge of the printk tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 941 bytes --]

Hi all,

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

ERROR: modpost: "lockdep_assert_console_list_lock_held" [drivers/video/fbdev/xen-fbfront.ko] undefined!
ERROR: modpost: "lockdep_assert_console_list_lock_held" [drivers/tty/serial/8250/8250.ko] undefined!
ERROR: modpost: "lockdep_assert_console_list_lock_held" [drivers/tty/serial/samsung_tty.ko] undefined!
ERROR: modpost: "lockdep_assert_console_list_lock_held" [drivers/tty/serial/kgdboc.ko] undefined!
ERROR: modpost: "lockdep_assert_console_list_lock_held" [drivers/net/netconsole.ko] undefined!

Presumably caused by commits

  c26c1533e3bd ("printk: introduce console_list_lock")
  a71ba89dc3ed ("console: introduce wrappers to read/write console flags")
  d3ca62bf29bb ("console: introduce console_is_registered()")

I have used the printk tree from next-20221118 for today.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build failure after merge of the printk tree

[Chris Down]

Hey folks,

Petr Mladek writes:
>I am sorry for the delay. I waited for a patch with reasonable commit
>message from Chris. I did it myself after all.

My sincere apologies for the delay. I had it in my calendar to do yesterday 
afternoon, but ended up with no time left :-(

Thanks Petr for pushing them, and sorry for the trouble caused.

Re: linux-next: build failure after merge of the printk tree

[Petr Mladek]

On Fri 2021-07-23 09:01:36, Stephen Rothwell wrote:
> Hi all,
> 
> On Tue, 20 Jul 2021 17:43:00 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > Hi all,
> > 
> > After merging the printk tree, today's linux-next build (mips allnoconfig)
> > failed like this:
> > 
> > arch/mips/kernel/genex.o: In function `handle_mcheck_int':
> > (.text+0x190c): undefined reference to `printk'
> > arch/mips/kernel/genex.o: In function `handle_reserved_int':
> > (.text+0x1c8c): undefined reference to `printk'
> > 
> > Caused by commit
> > 
> >   337015573718 ("printk: Userspace format indexing support")
> 
> I am still getting these failures.

I have just pushed the proposed fix into printk/linux.git,
branch for-5.15-printk-index.

I am sorry for the delay. I waited for a patch with reasonable commit
message from Chris. I did it myself after all.

Thanks for the report.

Best Regards,
Petr

Re: linux-next: build failure after merge of the printk tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 605 bytes --]

Hi all,

On Tue, 20 Jul 2021 17:43:00 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi all,
> 
> After merging the printk tree, today's linux-next build (mips allnoconfig)
> failed like this:
> 
> arch/mips/kernel/genex.o: In function `handle_mcheck_int':
> (.text+0x190c): undefined reference to `printk'
> arch/mips/kernel/genex.o: In function `handle_reserved_int':
> (.text+0x1c8c): undefined reference to `printk'
> 
> Caused by commit
> 
>   337015573718 ("printk: Userspace format indexing support")

I am still getting these failures.
-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build failure after merge of the printk tree

[Petr Mladek]

On Wed 2021-07-21 12:22:18, Chris Down wrote:
> Petr Mladek writes:
> > Chris, could you please send it as a proper patch?
> 
> Sure thing -- just to confirm, resend the whole series to you with these
> fixes squashed in, right? :-)

Good question. I would prefer to avoid the rebase. Please, send
it as a followup patch.

Best Regards,
Petr

Re: linux-next: build failure after merge of the printk tree

[Chris Down]

Petr Mladek writes:
>Chris, could you please send it as a proper patch?

Sure thing -- just to confirm, resend the whole series to you with these fixes 
squashed in, right? :-)

Re: linux-next: build failure after merge of the printk tree

[Petr Mladek]

On Tue 2021-07-20 13:28:45, Chris Down wrote:
> Hey folks,
> 
> Naresh Kamboju writes:
> > On Tue, 20 Jul 2021 at 13:13, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > > 
> > > Hi all,
> > > 
> > > After merging the printk tree, today's linux-next build (mips allnoconfig)
> > > failed like this:
> > > 
> > > arch/mips/kernel/genex.o: In function `handle_mcheck_int':
> > > (.text+0x190c): undefined reference to `printk'
> > > arch/mips/kernel/genex.o: In function `handle_reserved_int':
> > > (.text+0x1c8c): undefined reference to `printk'
> > > 
> > > Caused by commit
> > > 
> > >   337015573718 ("printk: Userspace format indexing support")
> > 
> > Following MIPS builds failed at our end due the reported problem.
> 
> Thanks: missed this as I made sure to change all .S files to use _printk,
> but this is in a .h file included in a .S file.
> 
> Here's what's needed. :-)
> 
> diff --git arch/mips/include/asm/asm.h arch/mips/include/asm/asm.h
> index ea4b62ece336..2f8ce94ebaaf 100644
> --- arch/mips/include/asm/asm.h
> +++ arch/mips/include/asm/asm.h
> @@ -114,7 +114,7 @@ symbol		=	value
>  		.set	push;				\
>  		.set	reorder;			\
>  		PTR_LA	a0, 8f;				\
> -		jal	printk;				\
> +		jal	_printk;			\
>  		.set	pop;				\
>  		TEXT(string)
>  #else

Chris, could you please send it as a proper patch?

Best Regards,
Petr

Re: linux-next: build failure after merge of the printk tree

[Chris Down]

Hey folks,

Naresh Kamboju writes:
>On Tue, 20 Jul 2021 at 13:13, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>>
>> Hi all,
>>
>> After merging the printk tree, today's linux-next build (mips allnoconfig)
>> failed like this:
>>
>> arch/mips/kernel/genex.o: In function `handle_mcheck_int':
>> (.text+0x190c): undefined reference to `printk'
>> arch/mips/kernel/genex.o: In function `handle_reserved_int':
>> (.text+0x1c8c): undefined reference to `printk'
>>
>> Caused by commit
>>
>>   337015573718 ("printk: Userspace format indexing support")
>
>Following MIPS builds failed at our end due the reported problem.

Thanks: missed this as I made sure to change all .S files to use _printk, but 
this is in a .h file included in a .S file.

Here's what's needed. :-)

diff --git arch/mips/include/asm/asm.h arch/mips/include/asm/asm.h
index ea4b62ece336..2f8ce94ebaaf 100644
--- arch/mips/include/asm/asm.h
+++ arch/mips/include/asm/asm.h
@@ -114,7 +114,7 @@ symbol		=	value
  		.set	push;				\
  		.set	reorder;			\
  		PTR_LA	a0, 8f;				\
-		jal	printk;				\
+		jal	_printk;			\
  		.set	pop;				\
  		TEXT(string)
  #else

Re: linux-next: build failure after merge of the printk tree

[Naresh Kamboju]

On Tue, 20 Jul 2021 at 13:13, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi all,
>
> After merging the printk tree, today's linux-next build (mips allnoconfig)
> failed like this:
>
> arch/mips/kernel/genex.o: In function `handle_mcheck_int':
> (.text+0x190c): undefined reference to `printk'
> arch/mips/kernel/genex.o: In function `handle_reserved_int':
> (.text+0x1c8c): undefined reference to `printk'
>
> Caused by commit
>
>   337015573718 ("printk: Userspace format indexing support")

Following MIPS builds failed at our end due the reported problem.

 - build/gcc-10-allnoconfig
 - build/gcc-10-ar7_defconfig
 - build/gcc-10-ath79_defconfig
 - build/gcc-10-bcm47xx_defconfig
 - build/gcc-10-bcm63xx_defconfig
 - build/gcc-10-cavium_octeon_defconfig
 - build/gcc-10-defconfig
 - build/gcc-10-e55_defconfig
 - build/gcc-10-malta_defconfig
 - build/gcc-10-nlm_xlp_defconfig
 - build/gcc-10-rt305x_defconfig


>
> --
> Cheers,
> Stephen Rothwell

- Naresh

linux-next: build failure after merge of the printk tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 446 bytes --]

Hi all,

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

arch/mips/kernel/genex.o: In function `handle_mcheck_int':
(.text+0x190c): undefined reference to `printk'
arch/mips/kernel/genex.o: In function `handle_reserved_int':
(.text+0x1c8c): undefined reference to `printk'

Caused by commit

  337015573718 ("printk: Userspace format indexing support")

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build failure after merge of the printk tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 483 bytes --]

Hi Sergey,

On Tue, 28 Jul 2020 10:54:08 +0900 Sergey Senozhatsky <sergey.senozhatsky@gmail.com> wrote:
>
>> Sorry about that. This will be fixed for tomorrow's build. The problems
> is that we have commit dependency between printk and locking tree. So I
> pulled the
> https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=locking/core&id=c935cd62d3fe985d7f0ebea185d2759e8992e96f
> 
> into printk/for-next.

OK, thanks.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build failure after merge of the printk tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 537 bytes --]

Hi Herbert,

On Tue, 28 Jul 2020 11:51:19 +1000 Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> This patch depends on two patches in the tips tree.  I presume
> this build test was done without the tips tree, right?

Of course it was ...

Each tree merged into linux-next should really be standalone (in case
e.g. Linus does not take the other tree).  So to do what you want, you
need those other 2 commits to be in a separate branch that both the tip
tree and the printk tree can merge.
-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build failure after merge of the printk tree

[Sergey Senozhatsky]

On (20/07/28 11:49), Stephen Rothwell wrote:
[..]
> Caused by commit
> 
>   b4a461e72bcb ("printk: Make linux/printk.h self-contained")
> 
> This *may* be interacting with other include file changes in linux-next.
> 
> I have reverted that commit for today.

Hi Stephen,

Sorry about that. This will be fixed for tomorrow's build. The problems
is that we have commit dependency between printk and locking tree. So I
pulled the
https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=locking/core&id=c935cd62d3fe985d7f0ebea185d2759e8992e96f

into printk/for-next.

	-ss

Re: linux-next: build failure after merge of the printk tree

[Herbert Xu]

On Tue, Jul 28, 2020 at 11:49:27AM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the printk tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
> 
> In file included from include/linux/printk.h:10,
>                  from include/linux/kernel.h:15,
>                  from include/linux/list.h:9,
>                  from include/linux/lockdep.h:43,
>                  from include/linux/spinlock_types.h:18,
>                  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:102:24: note: in definition of macro 'raw_spin_lock_init'
>   102 |  __raw_spin_lock_init((lock), #lock, &__key, LD_WAIT_SPIN); \
>       |                        ^~~~
> include/linux/spinlock.h:95:52: note: expected 'raw_spinlock_t *' {aka 'struct raw_spinlock *'} but argument is of type 'int *'
>    95 |   extern void __raw_spin_lock_init(raw_spinlock_t *lock, const char *name,
>       |                                    ~~~~~~~~~~~~~~~~^~~~
> 
> Caused by commit
> 
>   b4a461e72bcb ("printk: Make linux/printk.h self-contained")
> 
> This *may* be interacting with other include file changes in linux-next.
> 
> I have reverted that commit for today.

This patch depends on two patches in the tips tree.  I presume
this build test was done without the tips tree, right?

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

linux-next: build failure after merge of the printk tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2108 bytes --]

Hi all,

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

In file included from include/linux/printk.h:10,
                 from include/linux/kernel.h:15,
                 from include/linux/list.h:9,
                 from include/linux/lockdep.h:43,
                 from include/linux/spinlock_types.h:18,
                 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:102:24: note: in definition of macro 'raw_spin_lock_init'
  102 |  __raw_spin_lock_init((lock), #lock, &__key, LD_WAIT_SPIN); \
      |                        ^~~~
include/linux/spinlock.h:95:52: note: expected 'raw_spinlock_t *' {aka 'struct raw_spinlock *'} but argument is of type 'int *'
   95 |   extern void __raw_spin_lock_init(raw_spinlock_t *lock, const char *name,
      |                                    ~~~~~~~~~~~~~~~~^~~~

Caused by commit

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

This *may* be interacting with other include file changes in linux-next.

I have reverted that commit for today.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build failure after merge of the printk tree

[Petr Mladek]

On Tue 2020-06-23 17:17:38, Peter Zijlstra wrote:
> On Tue, Jun 23, 2020 at 04:28:58PM +0200, Petr Mladek wrote:
> 
> > PS: And yes, it makes sense to push both patches via a single tree to
> > make sure that the lockdep.h split is done first.
> 
> That's what I got you tip/locking/header for, pull that topic branch
> into your tree.

I see, I misunderstood it.
I am going to merge the branch once I get the updated 2nd patch.

Best Regards,
Petr

Re: linux-next: build failure after merge of the printk tree

[Herbert Xu]

On Wed, Jun 24, 2020 at 10:19:29AM +0200, Petr Mladek wrote:
> On Wed 2020-06-24 10:20:58, Herbert Xu wrote:
> > On Tue, Jun 23, 2020 at 04:28:58PM +0200, Petr Mladek wrote:
> > >
> > > It is similar cycle:
> > > 
> > > spinlock_types.h -> lockdep.h -> printk.h -> ratelimit.h -> spinlock_types.h
> > > 
> > > But this time it happens via list.h -> kernel.h ->printk.h.
> > > Where list.h needs READ_ONCE() stuff from compiler.h.
> > 
> > But this is exactly the loop that's fixed by the lockdep_types
> > patch.
> 
> I think that this problem is similar to the one on s390 that was
> solved by compiler.h in arch/s390/include/asm/bug.h
> 
> It is related to
> https://lore.kernel.org/r/20200613142901.65xieioomt6bbqa6@ltop.local
> 
> It seems that we need the same change also in list.h.

Oh I see.  I misunderstood what you said in the first email.
 
> Do you mean to repost the 2nd patch with the change in
> include/linux/list.h?

OK let me test this and repost.

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

Re: linux-next: build failure after merge of the printk tree

[Petr Mladek]

On Wed 2020-06-24 10:20:58, Herbert Xu wrote:
> On Tue, Jun 23, 2020 at 04:28:58PM +0200, Petr Mladek wrote:
> >
> > It is similar cycle:
> > 
> > spinlock_types.h -> lockdep.h -> printk.h -> ratelimit.h -> spinlock_types.h
> > 
> > But this time it happens via list.h -> kernel.h ->printk.h.
> > Where list.h needs READ_ONCE() stuff from compiler.h.
> 
> But this is exactly the loop that's fixed by the lockdep_types
> patch.

I think that this problem is similar to the one on s390 that was
solved by compiler.h in arch/s390/include/asm/bug.h

It is related to
https://lore.kernel.org/r/20200613142901.65xieioomt6bbqa6@ltop.local

It seems that we need the same change also in list.h.


> Did you get a compile failure with *just* the lockdep
> patch?

No, I see the problem only with both patches.


> > PS: And yes, it makes sense to push both patches via a single tree to
> > make sure that the lockdep.h split is done first.
> 
> OK, can I repost this patch with your ack then?

Do you mean to repost the 2nd patch with the change in
include/linux/list.h?

This would be great.

Best Regards,
Petr

Re: linux-next: build failure after merge of the printk tree

[Herbert Xu]

On Tue, Jun 23, 2020 at 04:28:58PM +0200, Petr Mladek wrote:
>
> It is similar cycle:
> 
> spinlock_types.h -> lockdep.h -> printk.h -> ratelimit.h -> spinlock_types.h
> 
> But this time it happens via list.h -> kernel.h ->printk.h.
> Where list.h needs READ_ONCE() stuff from compiler.h.

But this is exactly the loop that's fixed by the lockdep_types
patch.  Did you get a compile failure with *just* the lockdep
patch?

> PS: And yes, it makes sense to push both patches via a single tree to
> make sure that the lockdep.h split is done first.

OK, can I repost this patch with your ack then?

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

Re: linux-next: build failure after merge of the printk tree

[Peter Zijlstra]

On Tue, Jun 23, 2020 at 04:28:58PM +0200, Petr Mladek wrote:

> PS: And yes, it makes sense to push both patches via a single tree to
> make sure that the lockdep.h split is done first.

That's what I got you tip/locking/header for, pull that topic branch
into your tree.

Re: linux-next: build failure after merge of the printk tree

[Petr Mladek]

On Tue 2020-06-23 22:19:37, Herbert Xu wrote:
> On Tue, Jun 23, 2020 at 02:16:38PM +0200, Petr Mladek wrote:
> >
> > I have removed the problematic commit for now. It tried to remove
> > some cyclic dependencies from heavily used include files. It clearly
> > needs more love.
> 
> Hmm, the cyclic dependencies are there because you didn't pull in
> the lockdep_types patch.  The printk patch must go on top of the
> lockdep_types patch.  How about just putting this into the x86 tree
> alongside the lockdep_types patch?

I see the problem with both patches.

> In file included from include/linux/printk.h:10,
>                  from include/linux/kernel.h:15,
>                  from include/linux/list.h:9,
>                  from include/linux/lockdep.h:43,
>                  from include/linux/spinlock_types.h:18,
>                  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 */

It is similar cycle:

spinlock_types.h -> lockdep.h -> printk.h -> ratelimit.h -> spinlock_types.h

But this time it happens via list.h -> kernel.h ->printk.h.
Where list.h needs READ_ONCE() stuff from compiler.h.


My "allmodconfig" build has successfully finished with the following extra
 fix on top of the two patches:

diff --git a/include/linux/list.h b/include/linux/list.h
index aff44d34f4e4..6d606c4036ce 100644
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -6,7 +6,7 @@
 #include <linux/stddef.h>
 #include <linux/poison.h>
 #include <linux/const.h>
-#include <linux/kernel.h>
+#include <linux/compiler.h>
 
 /*
  * Simple doubly linked list implementation.

I suggest to bundle this into the 2nd patch that makes linux/printk.h
self-contained.

Best Regards,
Petr

PS: And yes, it makes sense to push both patches via a single tree to
make sure that the lockdep.h split is done first.

Re: linux-next: build failure after merge of the printk tree

[Herbert Xu]

On Tue, Jun 23, 2020 at 02:16:38PM +0200, Petr Mladek wrote:
>
> I have removed the problematic commit for now. It tried to remove
> some cyclic dependencies from heavily used include files. It clearly
> needs more love.

Hmm, the cyclic dependencies are there because you didn't pull in
the lockdep_types patch.  The printk patch must go on top of the
lockdep_types patch.  How about just putting this into the x86 tree
alongside the lockdep_types patch?

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

Re: linux-next: build failure after merge of the printk tree

[Petr Mladek]

On Tue 2020-06-23 10:26:55, Stephen Rothwell wrote:
> Hi Stephen,
> 
> On Sun, 21 Jun 2020 13:15:54 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > Hi all,
> > 
> > After merging the printk tree, today's linux-next build (x86_64
> > allmodconfig) failed like this:
> > 
> > In file included from include/linux/printk.h:10,
> >                  from include/linux/kernel.h:15,
> >                  from include/linux/list.h:9,
> >                  from include/linux/lockdep.h:43,
> >                  from include/linux/spinlock_types.h:18,
> >                  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:102:24: note: in definition of macro 'raw_spin_lock_init'
> >   102 |  __raw_spin_lock_init((lock), #lock, &__key, LD_WAIT_SPIN); \
> >       |                        ^~~~
> > include/linux/spinlock.h:95:52: note: expected 'raw_spinlock_t *' {aka 'struct raw_spinlock *'} but argument is of type 'int *'
> >    95 |   extern void __raw_spin_lock_init(raw_spinlock_t *lock, const char *name,
> >       |                                    ~~~~~~~~~~~~~~~~^~~~
> > 
> > Caused by commit
> > 
> >   494c8512c90e ("printk: Make linux/printk.h self-contained")
> > 
> > changing include files is hadrer than it loooks :-(
> > 
> > I have used the printk tree from next-20200618 for today.
> 
> I am still getting this failure.

I have removed the problematic commit for now. It tried to remove
some cyclic dependencies from heavily used include files. It clearly
needs more love.

I am sorry for the late reaction. I have semi-lost mails from last 4 days.
I am still trying to recover them.

Best Regards,
Petr

Re: linux-next: build failure after merge of the printk tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2327 bytes --]

Hi Stephen,

On Sun, 21 Jun 2020 13:15:54 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi all,
> 
> After merging the printk tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
> 
> In file included from include/linux/printk.h:10,
>                  from include/linux/kernel.h:15,
>                  from include/linux/list.h:9,
>                  from include/linux/lockdep.h:43,
>                  from include/linux/spinlock_types.h:18,
>                  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:102:24: note: in definition of macro 'raw_spin_lock_init'
>   102 |  __raw_spin_lock_init((lock), #lock, &__key, LD_WAIT_SPIN); \
>       |                        ^~~~
> include/linux/spinlock.h:95:52: note: expected 'raw_spinlock_t *' {aka 'struct raw_spinlock *'} but argument is of type 'int *'
>    95 |   extern void __raw_spin_lock_init(raw_spinlock_t *lock, const char *name,
>       |                                    ~~~~~~~~~~~~~~~~^~~~
> 
> Caused by commit
> 
>   494c8512c90e ("printk: Make linux/printk.h self-contained")
> 
> changing include files is hadrer than it loooks :-(
> 
> I have used the printk tree from next-20200618 for today.

I am still getting this failure.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build failure after merge of the printk tree

[Herbert Xu]

On Sun, Jun 21, 2020 at 01:15:54PM +1000, Stephen Rothwell wrote:
>
> Caused by commit
> 
>   494c8512c90e ("printk: Make linux/printk.h self-contained")
> 
> changing include files is hadrer than it loooks :-(

That's because this patch depends on the lockdep_types patch which
is in the x86 tree.  Petr, this patch needs to go on top of the
lockdep_types patch.

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

linux-next: build failure after merge of the printk tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2106 bytes --]

Hi all,

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

In file included from include/linux/printk.h:10,
                 from include/linux/kernel.h:15,
                 from include/linux/list.h:9,
                 from include/linux/lockdep.h:43,
                 from include/linux/spinlock_types.h:18,
                 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:102:24: note: in definition of macro 'raw_spin_lock_init'
  102 |  __raw_spin_lock_init((lock), #lock, &__key, LD_WAIT_SPIN); \
      |                        ^~~~
include/linux/spinlock.h:95:52: note: expected 'raw_spinlock_t *' {aka 'struct raw_spinlock *'} but argument is of type 'int *'
   95 |   extern void __raw_spin_lock_init(raw_spinlock_t *lock, const char *name,
      |                                    ~~~~~~~~~~~~~~~~^~~~

Caused by commit

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

changing include files is hadrer than it loooks :-(

I have used the printk tree from next-20200618 for today.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build failure after merge of the printk tree

[Petr Mladek]

On Fri 2019-12-06 09:25:03, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the printk tree, today's linux-next build (arm
> multi_v7_defconfig) failed like this:
> 
> kernel/trace/trace.c: In function 'register_tracer':
> kernel/trace/trace.c:1892:3: error: implicit declaration of function 'pr_warning'; did you mean 'pr_warn'? [-Werror=implicit-function-declaration]
>  1892 |   pr_warning("Can not register tracer %s due to lockdown\n",
>       |   ^~~~~~~~~~
>       |   pr_warn
> 
> Caused by commit
> 
>   55130ba7f010 ("printk: Drop pr_warning definition")
> 
> interacting with commit
> 
>   a356646a5685 ("tracing: Do not create directories if lockdown is in affect")
> 
> from Linus' tree.
> 
> I have applied the following merge fix patch for today:
> 
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Fri, 6 Dec 2019 09:21:57 +1100
> Subject: [PATCH] fix up for "printk: Drop pr_warning definition"
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>

I have pushed the patch into printk.git, for-5.5-pr_warning-removal.
And updated the for-next branch.

Thanks a lot for the fix and sorry for the incovenience.

Best Regards,
Petr

linux-next: build failure after merge of the printk tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2558 bytes --]

Hi all,

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

kernel/trace/trace.c: In function 'register_tracer':
kernel/trace/trace.c:1892:3: error: implicit declaration of function 'pr_warning'; did you mean 'pr_warn'? [-Werror=implicit-function-declaration]
 1892 |   pr_warning("Can not register tracer %s due to lockdown\n",
      |   ^~~~~~~~~~
      |   pr_warn

Caused by commit

  55130ba7f010 ("printk: Drop pr_warning definition")

interacting with commit

  a356646a5685 ("tracing: Do not create directories if lockdown is in affect")

from Linus' tree.

I have applied the following merge fix patch for today:

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Fri, 6 Dec 2019 09:21:57 +1100
Subject: [PATCH] fix up for "printk: Drop pr_warning definition"

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 kernel/trace/ring_buffer.c | 2 +-
 kernel/trace/trace.c       | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 4bf050fcfe3b..3f655371eaf6 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -5070,7 +5070,7 @@ static __init int test_ringbuffer(void)
 	int ret = 0;
 
 	if (security_locked_down(LOCKDOWN_TRACEFS)) {
-		pr_warning("Lockdown is enabled, skipping ring buffer tests\n");
+		pr_warn("Lockdown is enabled, skipping ring buffer tests\n");
 		return 0;
 	}
 
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 23459d53d576..6c75410f9698 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1889,7 +1889,7 @@ int __init register_tracer(struct tracer *type)
 	}
 
 	if (security_locked_down(LOCKDOWN_TRACEFS)) {
-		pr_warning("Can not register tracer %s due to lockdown\n",
+		pr_warn("Can not register tracer %s due to lockdown\n",
 			   type->name);
 		return -EPERM;
 	}
@@ -8796,7 +8796,7 @@ struct dentry *tracing_init_dentry(void)
 	struct trace_array *tr = &global_trace;
 
 	if (security_locked_down(LOCKDOWN_TRACEFS)) {
-		pr_warning("Tracing disabled due to lockdown\n");
+		pr_warn("Tracing disabled due to lockdown\n");
 		return ERR_PTR(-EPERM);
 	}
 
@@ -9244,7 +9244,7 @@ __init static int tracer_alloc_buffers(void)
 
 
 	if (security_locked_down(LOCKDOWN_TRACEFS)) {
-		pr_warning("Tracing disabled due to lockdown\n");
+		pr_warn("Tracing disabled due to lockdown\n");
 		return -EPERM;
 	}
 
-- 
2.24.0

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build failure after merge of the printk tree

[Petr Mladek]

On Sat 2018-03-03 23:47:39, Sergey Senozhatsky wrote:
> Cc-ing Tejun
> 
> On (03/02/18 16:54), Petr Mladek wrote:
> [..]
> > > (Though it is not immediately obvious why.)
> > 
> > It is a mistery to me. The error appears when I move any of
> > dump_stack_print_info() or show_regs_print_info() function
> > definitions from kernel/printk/printk.c to lib/dump_stack.c.
> > All the other changes seems unrelated.
> > 
> > The thing is that we basically do not touch dump_stack() definition
> > by that patch.
> 
> Apparently dump_stack_print_info() was in lib/dump_stack.c a long
> time ago, but it was deliberately moved to printk.c, when kernel gained
> a "generic" (dummy) dump_stack() fallback. Some archs, like blackfin,
> define their own dump_stack() symbol and make it global via EXPORT_SYMBOL.
> 
> In case of blackfin that arch-specific dump_stack() symbol invokes a
> global dump_stack_print_info(). If we move dump_stack_print_info() back
> to lib/dump_stack.c then we link both with arch/blackfin/dumpstack.o
> and lib/dump_stack.o, which results in multiple definitions error.
> If we move dump_stack_print_info() out on libdump_stack.o, then we
> never link with lib/dump_stack.o

Ah, I have finally understood the meaning of the libs-y kbuild
variable. It is a nice source of these strange build failures.


> ... so what are we going to do with that.
> 
> a) we can drop the patch and cherry pick only the kexec part
> 
> b) we can try to mark dummy lib/dump_stack() as __weak
>    EXPORT_SYMBOL and remove EXPORT_SYMBOL from arch-specific
>    definitions.

Using the weak symbol makes perfect sense. I am going to
look at the patch.

Thanks a lot everybody for help.

Best Regards,
Petr

Re: linux-next: build failure after merge of the printk tree

[Sergey Senozhatsky]

On (03/05/18 13:27), Greentime Hu wrote:
[..]
> >> Opinions? Will this work?
> >
> > I would think b) is better, thanks for the fix!
> >
> Hi,
> 
> b works in nds32.
> Thanks for the fix :)

Greentime, Dave, thanks!

I'll send out a patch then.

Petr, once the patch has enough Ack/etc do you want to pick it up?
We need to merge it first, before Dave's patch.

	-ss

Re: linux-next: build failure after merge of the printk tree

[Greentime Hu]

2018-03-05 11:20 GMT+08:00 Dave Young <dyoung@redhat.com>:
> On 03/03/18 at 11:47pm, Sergey Senozhatsky wrote:
>> Cc-ing Tejun
>>
>> On (03/02/18 16:54), Petr Mladek wrote:
>> [..]
>> > > (Though it is not immediately obvious why.)
>> >
>> > It is a mistery to me. The error appears when I move any of
>> > dump_stack_print_info() or show_regs_print_info() function
>> > definitions from kernel/printk/printk.c to lib/dump_stack.c.
>> > All the other changes seems unrelated.
>> >
>> > The thing is that we basically do not touch dump_stack() definition
>> > by that patch.
>>
>> Apparently dump_stack_print_info() was in lib/dump_stack.c a long
>> time ago, but it was deliberately moved to printk.c, when kernel gained
>> a "generic" (dummy) dump_stack() fallback. Some archs, like blackfin,
>> define their own dump_stack() symbol and make it global via EXPORT_SYMBOL.
>>
>> In case of blackfin that arch-specific dump_stack() symbol invokes a
>> global dump_stack_print_info(). If we move dump_stack_print_info() back
>> to lib/dump_stack.c then we link both with arch/blackfin/dumpstack.o
>> and lib/dump_stack.o, which results in multiple definitions error.
>> If we move dump_stack_print_info() out on libdump_stack.o, then we
>> never link with lib/dump_stack.o
>>
>> ... so what are we going to do with that.
>>
>> a) we can drop the patch and cherry pick only the kexec part
>>
>> b) we can try to mark dummy lib/dump_stack() as __weak
>>    EXPORT_SYMBOL and remove EXPORT_SYMBOL from arch-specific
>>    definitions.
>>
>>    So we will end up with EXPORT_SYMBOL dump_stack() and archs
>>    may re-define it. If some arch will accidentally mark its
>>    own dump_stack() as EXPORT_SYMBOL then there should be a
>>    linkage warning - a symbol is exported twice.
>>
>>
>> Something like below.
>>
>> Opinions? Will this work?
>
> I would think b) is better, thanks for the fix!
>
Hi,

b works in nds32.
Thanks for the fix :)

>>
>>
>> ========= 8< =========
>>
>> From: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
>> Subject: [PATCH] dump_stack: mark dummy dump_stack() as weak
>>
>> ---
>>  arch/blackfin/kernel/dumpstack.c | 1 -
>>  arch/nds32/kernel/traps.c        | 2 --
>>  lib/dump_stack.c                 | 4 ++--
>>  3 files changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/blackfin/kernel/dumpstack.c b/arch/blackfin/kernel/dumpstack.c
>> index 3c992c1f8ef2..61af017130cd 100644
>> --- a/arch/blackfin/kernel/dumpstack.c
>> +++ b/arch/blackfin/kernel/dumpstack.c
>> @@ -174,4 +174,3 @@ void dump_stack(void)
>>       show_stack(current, &stack);
>>       trace_buffer_restore(tflags);
>>  }
>> -EXPORT_SYMBOL(dump_stack);
>> diff --git a/arch/nds32/kernel/traps.c b/arch/nds32/kernel/traps.c
>> index 8828b4aeb72b..455bb0787367 100644
>> --- a/arch/nds32/kernel/traps.c
>> +++ b/arch/nds32/kernel/traps.c
>> @@ -166,8 +166,6 @@ void dump_stack(void)
>>       __dump(NULL, base_reg);
>>  }
>>
>> -EXPORT_SYMBOL(dump_stack);
>> -
>>  void show_stack(struct task_struct *tsk, unsigned long *sp)
>>  {
>>       unsigned long *base_reg;
>> diff --git a/lib/dump_stack.c b/lib/dump_stack.c
>> index 5cff72f18c4a..9cf4465dbffa 100644
>> --- a/lib/dump_stack.c
>> +++ b/lib/dump_stack.c
>> @@ -85,7 +85,7 @@ static void __dump_stack(void)
>>  #ifdef CONFIG_SMP
>>  static atomic_t dump_lock = ATOMIC_INIT(-1);
>>
>> -asmlinkage __visible void dump_stack(void)
>> +asmlinkage __weak __visible void dump_stack(void)
>>  {
>>       unsigned long flags;
>>       int was_locked;
>> @@ -118,7 +118,7 @@ asmlinkage __visible void dump_stack(void)
>>       local_irq_restore(flags);
>>  }
>>  #else
>> -asmlinkage __visible void dump_stack(void)
>> +asmlinkage __weak __visible void dump_stack(void)
>>  {
>>       __dump_stack();
>>  }
>> --
>> 2.16.2
>>
>
> Thanks
> Dave

Re: linux-next: build failure after merge of the printk tree

[Dave Young]

On 03/03/18 at 11:47pm, Sergey Senozhatsky wrote:
> Cc-ing Tejun
> 
> On (03/02/18 16:54), Petr Mladek wrote:
> [..]
> > > (Though it is not immediately obvious why.)
> > 
> > It is a mistery to me. The error appears when I move any of
> > dump_stack_print_info() or show_regs_print_info() function
> > definitions from kernel/printk/printk.c to lib/dump_stack.c.
> > All the other changes seems unrelated.
> > 
> > The thing is that we basically do not touch dump_stack() definition
> > by that patch.
> 
> Apparently dump_stack_print_info() was in lib/dump_stack.c a long
> time ago, but it was deliberately moved to printk.c, when kernel gained
> a "generic" (dummy) dump_stack() fallback. Some archs, like blackfin,
> define their own dump_stack() symbol and make it global via EXPORT_SYMBOL.
> 
> In case of blackfin that arch-specific dump_stack() symbol invokes a
> global dump_stack_print_info(). If we move dump_stack_print_info() back
> to lib/dump_stack.c then we link both with arch/blackfin/dumpstack.o
> and lib/dump_stack.o, which results in multiple definitions error.
> If we move dump_stack_print_info() out on libdump_stack.o, then we
> never link with lib/dump_stack.o
> 
> ... so what are we going to do with that.
> 
> a) we can drop the patch and cherry pick only the kexec part
> 
> b) we can try to mark dummy lib/dump_stack() as __weak
>    EXPORT_SYMBOL and remove EXPORT_SYMBOL from arch-specific
>    definitions.
> 
>    So we will end up with EXPORT_SYMBOL dump_stack() and archs
>    may re-define it. If some arch will accidentally mark its
>    own dump_stack() as EXPORT_SYMBOL then there should be a
>    linkage warning - a symbol is exported twice.
> 
> 
> Something like below.
> 
> Opinions? Will this work?

I would think b) is better, thanks for the fix!

> 
> 
> ========= 8< =========
> 
> From: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> Subject: [PATCH] dump_stack: mark dummy dump_stack() as weak
> 
> ---
>  arch/blackfin/kernel/dumpstack.c | 1 -
>  arch/nds32/kernel/traps.c        | 2 --
>  lib/dump_stack.c                 | 4 ++--
>  3 files changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/blackfin/kernel/dumpstack.c b/arch/blackfin/kernel/dumpstack.c
> index 3c992c1f8ef2..61af017130cd 100644
> --- a/arch/blackfin/kernel/dumpstack.c
> +++ b/arch/blackfin/kernel/dumpstack.c
> @@ -174,4 +174,3 @@ void dump_stack(void)
>  	show_stack(current, &stack);
>  	trace_buffer_restore(tflags);
>  }
> -EXPORT_SYMBOL(dump_stack);
> diff --git a/arch/nds32/kernel/traps.c b/arch/nds32/kernel/traps.c
> index 8828b4aeb72b..455bb0787367 100644
> --- a/arch/nds32/kernel/traps.c
> +++ b/arch/nds32/kernel/traps.c
> @@ -166,8 +166,6 @@ void dump_stack(void)
>  	__dump(NULL, base_reg);
>  }
>  
> -EXPORT_SYMBOL(dump_stack);
> -
>  void show_stack(struct task_struct *tsk, unsigned long *sp)
>  {
>  	unsigned long *base_reg;
> diff --git a/lib/dump_stack.c b/lib/dump_stack.c
> index 5cff72f18c4a..9cf4465dbffa 100644
> --- a/lib/dump_stack.c
> +++ b/lib/dump_stack.c
> @@ -85,7 +85,7 @@ static void __dump_stack(void)
>  #ifdef CONFIG_SMP
>  static atomic_t dump_lock = ATOMIC_INIT(-1);
>  
> -asmlinkage __visible void dump_stack(void)
> +asmlinkage __weak __visible void dump_stack(void)
>  {
>  	unsigned long flags;
>  	int was_locked;
> @@ -118,7 +118,7 @@ asmlinkage __visible void dump_stack(void)
>  	local_irq_restore(flags);
>  }
>  #else
> -asmlinkage __visible void dump_stack(void)
> +asmlinkage __weak __visible void dump_stack(void)
>  {
>  	__dump_stack();
>  }
> -- 
> 2.16.2
> 

Thanks
Dave

Re: linux-next: build failure after merge of the printk tree

[Sergey Senozhatsky]

Cc-ing Tejun

On (03/02/18 16:54), Petr Mladek wrote:
[..]
> > (Though it is not immediately obvious why.)
> 
> It is a mistery to me. The error appears when I move any of
> dump_stack_print_info() or show_regs_print_info() function
> definitions from kernel/printk/printk.c to lib/dump_stack.c.
> All the other changes seems unrelated.
> 
> The thing is that we basically do not touch dump_stack() definition
> by that patch.

Apparently dump_stack_print_info() was in lib/dump_stack.c a long
time ago, but it was deliberately moved to printk.c, when kernel gained
a "generic" (dummy) dump_stack() fallback. Some archs, like blackfin,
define their own dump_stack() symbol and make it global via EXPORT_SYMBOL.

In case of blackfin that arch-specific dump_stack() symbol invokes a
global dump_stack_print_info(). If we move dump_stack_print_info() back
to lib/dump_stack.c then we link both with arch/blackfin/dumpstack.o
and lib/dump_stack.o, which results in multiple definitions error.
If we move dump_stack_print_info() out on libdump_stack.o, then we
never link with lib/dump_stack.o

... so what are we going to do with that.

a) we can drop the patch and cherry pick only the kexec part

b) we can try to mark dummy lib/dump_stack() as __weak
   EXPORT_SYMBOL and remove EXPORT_SYMBOL from arch-specific
   definitions.

   So we will end up with EXPORT_SYMBOL dump_stack() and archs
   may re-define it. If some arch will accidentally mark its
   own dump_stack() as EXPORT_SYMBOL then there should be a
   linkage warning - a symbol is exported twice.


Something like below.

Opinions? Will this work?


========= 8< =========

From: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Subject: [PATCH] dump_stack: mark dummy dump_stack() as weak

---
 arch/blackfin/kernel/dumpstack.c | 1 -
 arch/nds32/kernel/traps.c        | 2 --
 lib/dump_stack.c                 | 4 ++--
 3 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/blackfin/kernel/dumpstack.c b/arch/blackfin/kernel/dumpstack.c
index 3c992c1f8ef2..61af017130cd 100644
--- a/arch/blackfin/kernel/dumpstack.c
+++ b/arch/blackfin/kernel/dumpstack.c
@@ -174,4 +174,3 @@ void dump_stack(void)
 	show_stack(current, &stack);
 	trace_buffer_restore(tflags);
 }
-EXPORT_SYMBOL(dump_stack);
diff --git a/arch/nds32/kernel/traps.c b/arch/nds32/kernel/traps.c
index 8828b4aeb72b..455bb0787367 100644
--- a/arch/nds32/kernel/traps.c
+++ b/arch/nds32/kernel/traps.c
@@ -166,8 +166,6 @@ void dump_stack(void)
 	__dump(NULL, base_reg);
 }
 
-EXPORT_SYMBOL(dump_stack);
-
 void show_stack(struct task_struct *tsk, unsigned long *sp)
 {
 	unsigned long *base_reg;
diff --git a/lib/dump_stack.c b/lib/dump_stack.c
index 5cff72f18c4a..9cf4465dbffa 100644
--- a/lib/dump_stack.c
+++ b/lib/dump_stack.c
@@ -85,7 +85,7 @@ static void __dump_stack(void)
 #ifdef CONFIG_SMP
 static atomic_t dump_lock = ATOMIC_INIT(-1);
 
-asmlinkage __visible void dump_stack(void)
+asmlinkage __weak __visible void dump_stack(void)
 {
 	unsigned long flags;
 	int was_locked;
@@ -118,7 +118,7 @@ asmlinkage __visible void dump_stack(void)
 	local_irq_restore(flags);
 }
 #else
-asmlinkage __visible void dump_stack(void)
+asmlinkage __weak __visible void dump_stack(void)
 {
 	__dump_stack();
 }
-- 
2.16.2

Re: linux-next: build failure after merge of the printk tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 548 bytes --]

Hi Petr,

On Fri, 2 Mar 2018 16:54:54 +0100 Petr Mladek <pmladek@suse.com> wrote:
>
> It is a mistery to me. The error appears when I move any of
> dump_stack_print_info() or show_regs_print_info() function
> definitions from kernel/printk/printk.c to lib/dump_stack.c.
> All the other changes seems unrelated.

Presumably because the Blackfin dumpstack() calls
dump_stack_print_info() so if you move that into lib/dumpstack.o, then
that file is dragged in and it contains another copy of dumpstack().

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build failure after merge of the printk tree

[Petr Mladek]

On Fri 2018-03-02 16:07:32, Stephen Rothwell wrote:
> Hi Petr,
> 
> After merging the printk tree, today's linux-next build (bfin
> BF518F-EZBRD_defconfig) failed like this:
> 
> lib/dump_stack.o: In function `dump_stack':
> lib/dump_stack.c:122: multiple definition of `dump_stack'
> arch/blackfin/kernel/dumpstack.o:arch/blackfin/kernel/dumpstack.c:166: first defined here
> 
> Presumably caused by commit
> 
>   8040af489957 ("printk: move dump stack related code to lib/dump_stack.c")

I could confirm that it is caused by this commit. I have temporary
removed it from printk.git.

> (Though it is not immediately obvious why.)

It is a mistery to me. The error appears when I move any of
dump_stack_print_info() or show_regs_print_info() function
definitions from kernel/printk/printk.c to lib/dump_stack.c.
All the other changes seems unrelated.

The thing is that we basically do not touch dump_stack() definition
by that patch.

> This fails all the blackfin builds.  nds32 (a new architecture) also
> has a dump_stack function.

Good to know!

Best Regards,
Petr

linux-next: build failure after merge of the printk tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 606 bytes --]

Hi Petr,

After merging the printk tree, today's linux-next build (bfin
BF518F-EZBRD_defconfig) failed like this:

lib/dump_stack.o: In function `dump_stack':
lib/dump_stack.c:122: multiple definition of `dump_stack'
arch/blackfin/kernel/dumpstack.o:arch/blackfin/kernel/dumpstack.c:166: first defined here

Presumably caused by commit

  8040af489957 ("printk: move dump stack related code to lib/dump_stack.c")

(Though it is not immediately obvious why.)

This fails all the blackfin builds.  nds32 (a new architecture) also
has a dump_stack function.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build failure after merge of the printk tree

[Petr Mladek]

On Thu 2017-12-07 10:26:31, Sergey Senozhatsky wrote:
> On (12/07/17 11:37), Stephen Rothwell wrote:
> [..]
> > include/linux/kallsyms.h: In function 'dereference_symbol_descriptor':
> > include/linux/kallsyms.h:63:8: error: implicit declaration of function '__module_address' [-Werror=implicit-function-declaration]
> >   mod = __module_address((unsigned long)ptr);
> >         ^
> > include/linux/kallsyms.h:67:9: error: implicit declaration of function 'dereference_module_function_descriptor' [-Werror=implicit-function-declaration]
> >    ptr = dereference_module_function_descriptor(mod, ptr);
> >          ^
> > 
> > and many more ...
> > 
> > Caused by commit
> > 
> >   78675fe41d57 ("symbol lookup: introduce dereference_symbol_descriptor()")
> > 
> > I have used the printk tree from next-20171206 (with its version of the
> > above patch reverted due to yesterday's error) for today.
> 
> will fix   :(

Sigh, the dependencies are quite complicated here.

Anyway, Sergey's fix for these non-x86 architectures is merged in printk.git
now.

Best Regards,
Petr

Re: linux-next: build failure after merge of the printk tree

[Sergey Senozhatsky]

On (12/07/17 11:37), Stephen Rothwell wrote:
[..]
> include/linux/kallsyms.h: In function 'dereference_symbol_descriptor':
> include/linux/kallsyms.h:63:8: error: implicit declaration of function '__module_address' [-Werror=implicit-function-declaration]
>   mod = __module_address((unsigned long)ptr);
>         ^
> include/linux/kallsyms.h:67:9: error: implicit declaration of function 'dereference_module_function_descriptor' [-Werror=implicit-function-declaration]
>    ptr = dereference_module_function_descriptor(mod, ptr);
>          ^
> 
> and many more ...
> 
> Caused by commit
> 
>   78675fe41d57 ("symbol lookup: introduce dereference_symbol_descriptor()")
> 
> I have used the printk tree from next-20171206 (with its version of the
> above patch reverted due to yesterday's error) for today.

will fix   :(

	-ss

Re: linux-next: build failure after merge of the printk tree

[Sergey Senozhatsky]

On (12/06/17 11:17), Petr Mladek wrote:
> On Wed 2017-12-06 14:23:00, Stephen Rothwell wrote:
> > Hi Petr,
> > 
> > After merging the printk tree, today's linux-next build (x86_64
> > allnoconfig) failed like this:
> > 
> > lib/vsprintf.o: In function `pointer':
> > vsprintf.c:(.text+0x2260): undefined reference to `dereference_symbol_descriptor'
> > 
> > Caused by commit
> > 
> >   b77929215209 ("symbol lookup: introduce dereference_symbol_descriptor()")
> 
> I have just pushed the fixed commit from Sergey and will improve my
> test coverage.
> 
> > I have reverted that commit for today.
> 
> It should work now. I am sorry for the troubles.

Stephen,

please keep that commit

	("symbol lookup: introduce dereference_symbol_descriptor()")

reverted in today's [20171207] linux-next. we have one more problem
with it.

	-ss

linux-next: build failure after merge of the printk tree

[Stephen Rothwell]

Hi Petr,

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

In file included from include/linux/ftrace.h:11:0,
                 from include/linux/perf_event.h:48,
                 from include/linux/trace_events.h:10,
                 from include/trace/syscall.h:7,
                 from include/linux/syscalls.h:82,
                 from init/do_mounts.h:5,
                 from init/do_mounts_initrd.c:22:
include/linux/kallsyms.h: In function 'dereference_symbol_descriptor':
include/linux/kallsyms.h:63:8: error: implicit declaration of function '__module_address' [-Werror=implicit-function-declaration]
  mod = __module_address((unsigned long)ptr);
        ^
include/linux/kallsyms.h:67:9: error: implicit declaration of function 'dereference_module_function_descriptor' [-Werror=implicit-function-declaration]
   ptr = dereference_module_function_descriptor(mod, ptr);
         ^

and many more ...

Caused by commit

  78675fe41d57 ("symbol lookup: introduce dereference_symbol_descriptor()")

I have used the printk tree from next-20171206 (with its version of the
above patch reverted due to yesterday's error) for today.

-- 
Cheers,
Stephen Rothwell

Re: linux-next: build failure after merge of the printk tree

[Sergey Senozhatsky]

On (12/06/17 11:17), Petr Mladek wrote:
> On Wed 2017-12-06 14:23:00, Stephen Rothwell wrote:
> > Hi Petr,
> > 
> > After merging the printk tree, today's linux-next build (x86_64
> > allnoconfig) failed like this:
> > 
> > lib/vsprintf.o: In function `pointer':
> > vsprintf.c:(.text+0x2260): undefined reference to `dereference_symbol_descriptor'
> > 
> > Caused by commit
> > 
> >   b77929215209 ("symbol lookup: introduce dereference_symbol_descriptor()")
> 
> I have just pushed the fixed commit from Sergey and will improve my
> test coverage.
> 
> > I have reverted that commit for today.
> 
> It should work now. I am sorry for the troubles.

it was completely my fault. sorry about that.

	-ss

Re: linux-next: build failure after merge of the printk tree

[Petr Mladek]

On Wed 2017-12-06 14:23:00, Stephen Rothwell wrote:
> Hi Petr,
> 
> After merging the printk tree, today's linux-next build (x86_64
> allnoconfig) failed like this:
> 
> lib/vsprintf.o: In function `pointer':
> vsprintf.c:(.text+0x2260): undefined reference to `dereference_symbol_descriptor'
> 
> Caused by commit
> 
>   b77929215209 ("symbol lookup: introduce dereference_symbol_descriptor()")

I have just pushed the fixed commit from Sergey and will improve my
test coverage.

> I have reverted that commit for today.

It should work now. I am sorry for the troubles.

Best Regards,
Petr

Re: linux-next: build failure after merge of the printk tree

[Sergey Senozhatsky]

Hello,

On (12/06/17 14:23), Stephen Rothwell wrote:
> Hi Petr,
> 
> After merging the printk tree, today's linux-next build (x86_64
> allnoconfig) failed like this:
> 
> lib/vsprintf.o: In function `pointer':
> vsprintf.c:(.text+0x2260): undefined reference to `dereference_symbol_descriptor'
> 
> Caused by commit
> 
>   b77929215209 ("symbol lookup: introduce dereference_symbol_descriptor()")
> 
> I have reverted that commit for today.


thanks. will fix it.

	-ss

linux-next: build failure after merge of the printk tree

[Stephen Rothwell]

Hi Petr,

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

lib/vsprintf.o: In function `pointer':
vsprintf.c:(.text+0x2260): undefined reference to `dereference_symbol_descriptor'

Caused by commit

  b77929215209 ("symbol lookup: introduce dereference_symbol_descriptor()")

I have reverted that commit for today.

-- 
Cheers,
Stephen Rothwell