Generic Ticket Spinlocks

PATCH In review [email protected] 4 participants 17 messages Mar 19, 2022 – Mar 22, 2022 1 message not in the archive

Subsystems (5)

RISC-V Generic include/asm header files Locking primitives Openrisc architecture +1 more subsystems

lore

Share & tools

lore

[email protected]

author maintainer

2022-03-19 03:54 UTC #0 lore raw

v2 cover Generic Ticket Spinlocks

From: Guo Ren <[email protected]>

Palmer:
Peter sent an RFC out about a year ago
<https://lore.kernel.org/lkml/[email protected]/>,
but after a spirited discussion it looks like we lost track of things. IIRC there was broad consensus on this being the way to go, but there was a lot of discussion so I wasn't sure. Given that it's been a year, I figured it'd be best to just send this out again formatted a bit more explicitly as a patch.

This has had almost no testing (just a build test on RISC-V defconfig), but I wanted to send it out largely as-is because I didn't have a SOB from Peter on the code. I had sent around something sort of similar in spirit, but this looks completely re-written. Just to play it safe I wanted to send out almost exactly as it was posted. I'd probably rename this tspinlock and tspinlock_types, as the mis-match kind of makes my eyes go funny, but I don't really care that much. I'll also go through the other ports and see if there's any more candidates, I seem to remember there having been more than just OpenRISC but it's been a while.

I'm in no big rush for this and given the complex HW dependencies I think it's best to target it for 5.19, that'd give us a full merge window for folks to test/benchmark it on their systems to make sure it's OK. RISC-V has a forward progress guarantee so we should be safe, but these can always trip things up.

Guo:
Update V2 with Arnd's suggestion [1].

[1] https://lore.kernel.org/linux-arch/CAK8P3a0NMPVGVw7===uEOtNnu1hr1GqimMbZT+Kea1CUxRvPmw at mail.gmail.com/raw

Changes in V2:
- Follow Arnd suggestion to make the patch series more generic.
- Add csky in the series.
- Combine RISC-V's two patches into one.
- Modify openrisc's patch to suit the new generic version.

Guo Ren (1):
csky: Move to generic ticket-spinlock

Palmer Dabbelt (1):
RISC-V: Move to ticket-spinlocks & RW locks

Peter Zijlstra (3):
asm-generic: ticket-lock: New generic ticket-based spinlock
asm-generic: qspinlock: Indicate the use of mixed-size atomics
openrisc: Move to ticket-spinlock

 arch/csky/include/asm/Kbuild               |   3 +-
 arch/csky/include/asm/spinlock.h           |  89 --------------
 arch/csky/include/asm/spinlock_types.h     |  27 -----
 arch/openrisc/Kconfig                      |   1 -
 arch/openrisc/include/asm/Kbuild           |   7 +-
 arch/openrisc/include/asm/spinlock.h       |  27 -----
 arch/openrisc/include/asm/spinlock_types.h |   7 --
 arch/riscv/Kconfig                         |   1 +
 arch/riscv/include/asm/Kbuild              |   2 +
 arch/riscv/include/asm/spinlock.h          | 135 ---------------------
 arch/riscv/include/asm/spinlock_types.h    |  25 ----
 include/asm-generic/qspinlock.h            |  30 +++++
 include/asm-generic/spinlock.h             |  11 +-
 include/asm-generic/spinlock_types.h       |  15 +++
 include/asm-generic/ticket-lock-types.h    |  11 ++
 include/asm-generic/ticket-lock.h          |  86 +++++++++++++
 16 files changed, 157 insertions(+), 320 deletions(-)
 delete mode 100644 arch/csky/include/asm/spinlock.h
 delete mode 100644 arch/csky/include/asm/spinlock_types.h
 delete mode 100644 arch/openrisc/include/asm/spinlock.h
 delete mode 100644 arch/openrisc/include/asm/spinlock_types.h
 delete mode 100644 arch/riscv/include/asm/spinlock.h
 delete mode 100644 arch/riscv/include/asm/spinlock_types.h
 create mode 100644 include/asm-generic/spinlock_types.h
 create mode 100644 include/asm-generic/ticket-lock-types.h
 create mode 100644 include/asm-generic/ticket-lock.h

Signature · 1 line

2.25.1

[email protected]

author maintainer

2022-03-19 03:54 UTC #1 lore raw

PATCH v2 1/5 asm-generic: ticket-lock: New generic ticket-based spinlock

From: Peter Zijlstra <[email protected]>

This is a simple, fair spinlock. Specifically it doesn't have all the subtle memory model dependencies that qspinlock has, which makes it more suitable for simple systems as it is more likely to be correct.

[Palmer: commit text]

Signed-off-by Palmer Dabbelt <[email protected]>

I have specifically not included Peter's SOB on this, as he sent his original patch
<https://lore.kernel.org/lkml/[email protected]/>
without one.

---
 include/asm-generic/spinlock.h          | 11 +++-
 include/asm-generic/spinlock_types.h    | 15 +++++
 include/asm-generic/ticket-lock-types.h | 11 ++++
 include/asm-generic/ticket-lock.h       | 86 +++++++++++++++++++++++++
 4 files changed, 120 insertions(+), 3 deletions(-)
 create mode 100644 include/asm-generic/spinlock_types.h
 create mode 100644 include/asm-generic/ticket-lock-types.h
 create mode 100644 include/asm-generic/ticket-lock.h

include/asm-generic/spinlock.h +8 -3

index adaf6acab172..a8e2aa1bcea4 100644
@@ -1,12 +1,17 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 #ifndef __ASM_GENERIC_SPINLOCK_H
 #define __ASM_GENERIC_SPINLOCK_H
+
 /*
- * You need to implement asm/spinlock.h for SMP support. The generic
- * version does not handle SMP.
+ * Using ticket-spinlock.h as generic for SMP support.
  */
 #ifdef CONFIG_SMP
-#error need an architecture specific asm/spinlock.h
+#include <asm-generic/ticket-lock.h>
+#ifdef CONFIG_QUEUED_RWLOCKS
+#include <asm-generic/qrwlock.h>
+#else
+#error Please select ARCH_USE_QUEUED_RWLOCKS in architecture Kconfig
+#endif
 #endif
 
 #endif /* __ASM_GENERIC_SPINLOCK_H */

include/asm-generic/spinlock_types.h new file +15 -0

new file mode 100644
index 000000000000..ba8ef4b731ba
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_GENERIC_SPINLOCK_TYPES_H
+#define __ASM_GENERIC_SPINLOCK_TYPES_H
+
+/*
+ * Using ticket spinlock as generic for SMP support.
+ */
+#ifdef CONFIG_SMP
+#include <asm-generic/ticket-lock-types.h>
+#include <asm-generic/qrwlock_types.h>
+#else
+#error The asm-generic/spinlock_types.h is not for CONFIG_SMP=n
+#endif
+
+#endif /* __ASM_GENERIC_SPINLOCK_TYPES_H */

include/asm-generic/ticket-lock-types.h new file +11 -0

new file mode 100644
index 000000000000..829759aedda8
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __ASM_GENERIC_TICKET_LOCK_TYPES_H
+#define __ASM_GENERIC_TICKET_LOCK_TYPES_H
+
+#include <linux/types.h>
+typedef atomic_t arch_spinlock_t;
+
+#define __ARCH_SPIN_LOCK_UNLOCKED	ATOMIC_INIT(0)
+
+#endif /* __ASM_GENERIC_TICKET_LOCK_TYPES_H */

include/asm-generic/ticket-lock.h new file +86 -0

86 lines not on this page · raw message

Signature · 1 line

2.25.1

Arnd Bergmann

maintainer

2022-03-19 11:52 UTC #2 lore raw

Re: [PATCH V2 1/5] asm-generic: ticket-lock: New generic ticket-based spinlock

Quoted 6 lines · On Sat, Mar 19, 2022 at 4:54 AM <[email protected]> wrote:

/*
- * You need to implement asm/spinlock.h for SMP support. The generic
- * version does not handle SMP.
+ * Using ticket-spinlock.h as generic for SMP support.
*/
#ifdef CONFIG_SMP

include/asm-generic/spinlock.h line 15

Quoted 8 lines

include/asm-generic/spinlock.h +6 -1

-#error need an architecture specific asm/spinlock.h
+#include <asm-generic/ticket-lock.h>
+#ifdef CONFIG_QUEUED_RWLOCKS
+#include <asm-generic/qrwlock.h>
+#else
+#error Please select ARCH_USE_QUEUED_RWLOCKS in architecture Kconfig
+#endif
 #endif

There is no need for the !CONFIG_SMP case, as asm/spinlock.h only ever gets included for SMP builds in the first place. This was already a mistake in the existing code, but your change would be the time to fix it.

I would also drop the !CONFIG_QUEUED_RWLOCKS case, just include it unconditionally. If any architecture wants the ticket spinlock in combination with a custom rwlock, they can simply include the asm-generic/ticket-lock.h from their asm/spinlock.h, but more likely any architecture that can use the ticket spinlock will also want the qrwlock anyway.

Arnd

Guo Ren

author maintainer

2022-03-19 13:26 UTC #3 lore raw

Re: [PATCH V2 1/5] asm-generic: ticket-lock: New generic ticket-based spinlock

Quoted 21 lines · Arnd Bergmann

Quoted 6 lines · On Sat, Mar 19, 2022 at 4:54 AM <[email protected]> wrote:
/*
- * You need to implement asm/spinlock.h for SMP support. The generic
- * version does not handle SMP.
+ * Using ticket-spinlock.h as generic for SMP support.
*/
#ifdef CONFIG_SMP
include/asm-generic/spinlock.h line 15
Quoted 8 lines
include/asm-generic/spinlock.h +6 -1
-#error need an architecture specific asm/spinlock.h
+#include <asm-generic/ticket-lock.h>
+#ifdef CONFIG_QUEUED_RWLOCKS
+#include <asm-generic/qrwlock.h>
+#else
+#error Please select ARCH_USE_QUEUED_RWLOCKS in architecture Kconfig
+#endif
 #endif
There is no need for the !CONFIG_SMP case, as asm/spinlock.h only ever gets included for SMP builds in the first place. This was already a mistake in the existing code, but your change would be the time to fix it.

Quoted 6 lines

I would also drop the !CONFIG_QUEUED_RWLOCKS case, just include it unconditionally. If any architecture wants the ticket spinlock in combination with a custom rwlock, they can simply include the asm-generic/ticket-lock.h from their asm/spinlock.h, but more likely any architecture that can use the ticket spinlock will also want the qrwlock anyway.

I agree, !CONFIG_SMP & !CONFIG_QUEUED_RWLOCKS are unnecessary.

@Palmer, you could pick back the series, thx.

Quoted 2 lines

Arnd

Signature · 4 lines

Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

Stafford Horne

maintainer

2022-03-22 03:10 UTC #4 lore raw

Re: [OpenRISC] [PATCH V2 1/5] asm-generic: ticket-lock: New generic ticket-based spinlock

In reply to [email protected]

Hello,

There is a problem with this patch on Big Endian machines, see below.

Quoted 30 lines · guoren at kernel.org

From: Peter Zijlstra <[email protected]>
This is a simple, fair spinlock. Specifically it doesn't have all the subtle memory model dependencies that qspinlock has, which makes it more suitable for simple systems as it is more likely to be correct.
[Palmer: commit text]
Signed-off-by Palmer Dabbelt <[email protected]>
--
I have specifically not included Peter's SOB on this, as he sent his original patch
<https://lore.kernel.org/lkml/[email protected]/>
without one.
---
 include/asm-generic/spinlock.h          | 11 +++-
 include/asm-generic/spinlock_types.h    | 15 +++++
 include/asm-generic/ticket-lock-types.h | 11 ++++
 include/asm-generic/ticket-lock.h       | 86 +++++++++++++++++++++++++
 4 files changed, 120 insertions(+), 3 deletions(-)
 create mode 100644 include/asm-generic/spinlock_types.h
 create mode 100644 include/asm-generic/ticket-lock-types.h
 create mode 100644 include/asm-generic/ticket-lock.h
include/asm-generic/ticket-lock.h new file +0 -0
new file mode 100644
index 000000000000..59373de3e32a

...

include/asm-generic/ticket-lock.h

Quoted 3 lines

include/asm-generic/ticket-lock.h new file +3 -0

+static __always_inline void ticket_unlock(arch_spinlock_t *lock)
+{
+	u16 *ptr = (u16 *)lock + __is_defined(__BIG_ENDIAN);

As mentioned, this patch series breaks SMP on OpenRISC. I traced it to this line. The above __is_defined(__BIG_ENDIAN) does not return 1 as expected even on BIG_ENDIAN machines. This works:

include/asm-generic/ticket-lock.h +2 -1

index 59373de3e32a..52b5dc9ffdba 100644
@@ -26,6 +26,7 @@
 #define __ASM_GENERIC_TICKET_LOCK_H
 
 #include <linux/atomic.h>
+#include <linux/kconfig.h>
 #include <asm-generic/ticket-lock-types.h>
 
 static __always_inline void ticket_lock(arch_spinlock_t *lock)

@@ -51,7 +52,7 @@ static __always_inline bool ticket_trylock(arch_spinlock_t *lock)
 
 static __always_inline void ticket_unlock(arch_spinlock_t *lock)
 {
-       u16 *ptr = (u16 *)lock + __is_defined(__BIG_ENDIAN);
+       u16 *ptr = (u16 *)lock + IS_ENABLED(CONFIG_CPU_BIG_ENDIAN);
        u32 val = atomic_read(lock);
 
        smp_store_release(ptr, (u16)val + 1);

Quoted 5 lines

include/asm-generic/ticket-lock.h new file +5 -0
+	u32 val = atomic_read(lock);
+
+	smp_store_release(ptr, (u16)val + 1);
+}
+

Waiman Long

reviewer

2022-03-22 15:54 UTC #5 lore raw

Re: [OpenRISC] [PATCH V2 1/5] asm-generic: ticket-lock: New generic ticket-based spinlock

Quoted 68 lines · Stafford Horne

Hello,
There is a problem with this patch on Big Endian machines, see below.
Quoted 30 lines · guoren at kernel.org
From: Peter Zijlstra <[email protected]>
This is a simple, fair spinlock. Specifically it doesn't have all the subtle memory model dependencies that qspinlock has, which makes it more suitable for simple systems as it is more likely to be correct.
[Palmer: commit text]
Signed-off-by Palmer Dabbelt <[email protected]>
--
I have specifically not included Peter's SOB on this, as he sent his original patch
<https://lore.kernel.org/lkml/[email protected]/>
without one.
---
  include/asm-generic/spinlock.h          | 11 +++-
  include/asm-generic/spinlock_types.h    | 15 +++++
  include/asm-generic/ticket-lock-types.h | 11 ++++
  include/asm-generic/ticket-lock.h       | 86 +++++++++++++++++++++++++
  4 files changed, 120 insertions(+), 3 deletions(-)
create mode 100644 include/asm-generic/spinlock_types.h create mode 100644 include/asm-generic/ticket-lock-types.h create mode 100644 include/asm-generic/ticket-lock.h
include/asm-generic/ticket-lock.h new file +0 -0
new file mode 100644
index 000000000000..59373de3e32a
...
include/asm-generic/ticket-lock.h
Quoted 3 lines
include/asm-generic/ticket-lock.h new file +3 -0
+static __always_inline void ticket_unlock(arch_spinlock_t *lock)
+{
+	u16 *ptr = (u16 *)lock + __is_defined(__BIG_ENDIAN);
As mentioned, this patch series breaks SMP on OpenRISC. I traced it to this line. The above __is_defined(__BIG_ENDIAN) does not return 1 as expected even on BIG_ENDIAN machines. This works:
include/asm-generic/ticket-lock.h +2 -1
index 59373de3e32a..52b5dc9ffdba 100644
@@ -26,6 +26,7 @@
  #define __ASM_GENERIC_TICKET_LOCK_H
 
  #include <linux/atomic.h>
+#include <linux/kconfig.h>
  #include <asm-generic/ticket-lock-types.h>
 
  static __always_inline void ticket_lock(arch_spinlock_t *lock)
@@ -51,7 +52,7 @@ static __always_inline bool ticket_trylock(arch_spinlock_t *lock)
 
  static __always_inline void ticket_unlock(arch_spinlock_t *lock)
  {
-       u16 *ptr = (u16 *)lock + __is_defined(__BIG_ENDIAN);
+       u16 *ptr = (u16 *)lock + IS_ENABLED(CONFIG_CPU_BIG_ENDIAN);
         u32 val = atomic_read(lock);
 
         smp_store_release(ptr, (u16)val + 1);

Quoted 5 lines

include/asm-generic/ticket-lock.h
Quoted 5 lines
include/asm-generic/ticket-lock.h new file +5 -0
+	u32 val = atomic_read(lock);
+
+	smp_store_release(ptr, (u16)val + 1);
+}
+

__BIG_ENDIAN is defined in <linux/kconfig.h>. I believe that if you include <linux/kconfig.h>, the second hunk is not really needed and vice versa.

Cheers,
Longman

Stafford Horne

maintainer

2022-03-22 21:06 UTC #6 lore raw

Re: [OpenRISC] [PATCH V2 1/5] asm-generic: ticket-lock: New generic ticket-based spinlock

Quoted 65 lines · Waiman Long

Quoted 64 lines · Stafford Horne
Hello,
There is a problem with this patch on Big Endian machines, see below.
Quoted 30 lines · guoren at kernel.org
From: Peter Zijlstra <[email protected]>
This is a simple, fair spinlock. Specifically it doesn't have all the subtle memory model dependencies that qspinlock has, which makes it more suitable for simple systems as it is more likely to be correct.
[Palmer: commit text]
Signed-off-by Palmer Dabbelt <[email protected]>
--
I have specifically not included Peter's SOB on this, as he sent his original patch
<https://lore.kernel.org/lkml/[email protected]/>
without one.
---
  include/asm-generic/spinlock.h          | 11 +++-
  include/asm-generic/spinlock_types.h    | 15 +++++
  include/asm-generic/ticket-lock-types.h | 11 ++++
  include/asm-generic/ticket-lock.h       | 86 +++++++++++++++++++++++++
  4 files changed, 120 insertions(+), 3 deletions(-)
create mode 100644 include/asm-generic/spinlock_types.h create mode 100644 include/asm-generic/ticket-lock-types.h create mode 100644 include/asm-generic/ticket-lock.h
include/asm-generic/ticket-lock.h new file +0 -0
new file mode 100644
index 000000000000..59373de3e32a
...
include/asm-generic/ticket-lock.h
Quoted 3 lines
include/asm-generic/ticket-lock.h new file +3 -0
+static __always_inline void ticket_unlock(arch_spinlock_t *lock)
+{
+	u16 *ptr = (u16 *)lock + __is_defined(__BIG_ENDIAN);
As mentioned, this patch series breaks SMP on OpenRISC. I traced it to this line. The above __is_defined(__BIG_ENDIAN) does not return 1 as expected even on BIG_ENDIAN machines. This works:
include/asm-generic/ticket-lock.h +2 -1
index 59373de3e32a..52b5dc9ffdba 100644
@@ -26,6 +26,7 @@
  #define __ASM_GENERIC_TICKET_LOCK_H
  #include <linux/atomic.h>
+#include <linux/kconfig.h>
  #include <asm-generic/ticket-lock-types.h>
  static __always_inline void ticket_lock(arch_spinlock_t *lock)
@@ -51,7 +52,7 @@ static __always_inline bool ticket_trylock(arch_spinlock_t *lock)
  static __always_inline void ticket_unlock(arch_spinlock_t *lock)
  {
-       u16 *ptr = (u16 *)lock + __is_defined(__BIG_ENDIAN);
+       u16 *ptr = (u16 *)lock + IS_ENABLED(CONFIG_CPU_BIG_ENDIAN);
         u32 val = atomic_read(lock);
         smp_store_release(ptr, (u16)val + 1);
 
 

Quoted 8 lines

Quoted 5 lines
include/asm-generic/ticket-lock.h
Quoted 5 lines
include/asm-generic/ticket-lock.h new file +5 -0
+	u32 val = atomic_read(lock);
+
+	smp_store_release(ptr, (u16)val + 1);
+}
+
__BIG_ENDIAN is defined in <linux/kconfig.h>. I believe that if you include <linux/kconfig.h>, the second hunk is not really needed and vice versa.

I thought so too, but it doesn't seem to work. I think __is_defined is not doing what we think in this context. It looks like __is_defined works when a macro is defined as 1, in this case we have __BIG_ENDIAN 4321.

With just the first hunk, we can see we still get 0 for the lock offset as per below:

(quoted diff) +0 -0

b/include/asm-generic/ticket-lock.h
index 59373de3e32a..769561fb6997 100644

include/asm-generic/ticket-lock.h +1 -0

@@ -26,6 +26,7 @@
 #define __ASM_GENERIC_TICKET_LOCK_H
 
 #include <linux/atomic.h>
+#include <linux/kconfig.h>
 #include <asm-generic/ticket-lock-types.h>
 
 static __always_inline void ticket_lock(arch_spinlock_t *lock)

make ARCH=openrisc simple_smp_defconfig
make ARCH=openrisc CROSS_COMPILE=or1k-linux- kernel/locking/spinlock.i grep -C3 'lock +' kernel/locking/spinlock.i

    static inline __attribute__((__gnu_inline__)) __attribute__((__unused__)) __attribute__((__no_instrument_function__)) __attribute__((__always_inline__)) void
    ticket_unlock(arch_spinlock_t *lock)
    {
     u16 *ptr = (u16 *)lock + 0;

     u32 val = atomic_read(lock);

-Stafford

Waiman Long

reviewer

2022-03-22 21:14 UTC #7 lore raw

Re: [OpenRISC] [PATCH V2 1/5] asm-generic: ticket-lock: New generic ticket-based spinlock

Quoted 73 lines · Stafford Horne

Quoted 65 lines · Waiman Long
Quoted 64 lines · Stafford Horne
Hello,
There is a problem with this patch on Big Endian machines, see below.
Quoted 30 lines · guoren at kernel.org
From: Peter Zijlstra <[email protected]>
This is a simple, fair spinlock. Specifically it doesn't have all the subtle memory model dependencies that qspinlock has, which makes it more suitable for simple systems as it is more likely to be correct.
[Palmer: commit text]
Signed-off-by Palmer Dabbelt <[email protected]>
--
I have specifically not included Peter's SOB on this, as he sent his original patch
<https://lore.kernel.org/lkml/[email protected]/>
without one.
---
   include/asm-generic/spinlock.h          | 11 +++-
   include/asm-generic/spinlock_types.h    | 15 +++++
   include/asm-generic/ticket-lock-types.h | 11 ++++
   include/asm-generic/ticket-lock.h       | 86 +++++++++++++++++++++++++
   4 files changed, 120 insertions(+), 3 deletions(-)
create mode 100644 include/asm-generic/spinlock_types.h create mode 100644 include/asm-generic/ticket-lock-types.h create mode 100644 include/asm-generic/ticket-lock.h
include/asm-generic/ticket-lock.h new file +0 -0
new file mode 100644
index 000000000000..59373de3e32a
...
include/asm-generic/ticket-lock.h
Quoted 3 lines
include/asm-generic/ticket-lock.h new file +3 -0
+static __always_inline void ticket_unlock(arch_spinlock_t *lock)
+{
+	u16 *ptr = (u16 *)lock + __is_defined(__BIG_ENDIAN);
As mentioned, this patch series breaks SMP on OpenRISC. I traced it to this line. The above __is_defined(__BIG_ENDIAN) does not return 1 as expected even on BIG_ENDIAN machines. This works:
include/asm-generic/ticket-lock.h +2 -1
index 59373de3e32a..52b5dc9ffdba 100644
@@ -26,6 +26,7 @@
   #define __ASM_GENERIC_TICKET_LOCK_H
   #include <linux/atomic.h>
+#include <linux/kconfig.h>
   #include <asm-generic/ticket-lock-types.h>
   static __always_inline void ticket_lock(arch_spinlock_t *lock)
@@ -51,7 +52,7 @@ static __always_inline bool ticket_trylock(arch_spinlock_t *lock)
   static __always_inline void ticket_unlock(arch_spinlock_t *lock)
   {
-       u16 *ptr = (u16 *)lock + __is_defined(__BIG_ENDIAN);
+       u16 *ptr = (u16 *)lock + IS_ENABLED(CONFIG_CPU_BIG_ENDIAN);
          u32 val = atomic_read(lock);
          smp_store_release(ptr, (u16)val + 1);
 
 
Quoted 7 lines
Quoted 5 lines
include/asm-generic/ticket-lock.h
Quoted 5 lines
include/asm-generic/ticket-lock.h new file +5 -0
+	u32 val = atomic_read(lock);
+
+	smp_store_release(ptr, (u16)val + 1);
+}
+
__BIG_ENDIAN is defined in <linux/kconfig.h>. I believe that if you include <linux/kconfig.h>, the second hunk is not really needed and vice versa.

Quoted 3 lines

I thought so too, but it doesn't seem to work. I think __is_defined is not doing what we think in this context. It looks like __is_defined works when a macro is defined as 1, in this case we have __BIG_ENDIAN 4321.

You are right. __is_defined() only for 1 or not 1. So it can't be used for __BIG_ENDIAN.

I was not aware of that. Anyway, the <linux/kconfig.h> include is not really needed then.

Cheers,
Longman

Stafford Horne

maintainer

2022-03-22 21:24 UTC #8 lore raw

Re: [OpenRISC] [PATCH V2 1/5] asm-generic: ticket-lock: New generic ticket-based spinlock

Quoted 78 lines · Waiman Long

Quoted 73 lines · Stafford Horne
Quoted 65 lines · Waiman Long
Quoted 64 lines · Stafford Horne
Hello,
There is a problem with this patch on Big Endian machines, see below.
Quoted 30 lines · guoren at kernel.org
From: Peter Zijlstra <[email protected]>
This is a simple, fair spinlock. Specifically it doesn't have all the subtle memory model dependencies that qspinlock has, which makes it more suitable for simple systems as it is more likely to be correct.
[Palmer: commit text]
Signed-off-by Palmer Dabbelt <[email protected]>
--
I have specifically not included Peter's SOB on this, as he sent his original patch
<https://lore.kernel.org/lkml/[email protected]/>
without one.
---
   include/asm-generic/spinlock.h          | 11 +++-
   include/asm-generic/spinlock_types.h    | 15 +++++
   include/asm-generic/ticket-lock-types.h | 11 ++++
   include/asm-generic/ticket-lock.h       | 86 +++++++++++++++++++++++++
   4 files changed, 120 insertions(+), 3 deletions(-)
create mode 100644 include/asm-generic/spinlock_types.h create mode 100644 include/asm-generic/ticket-lock-types.h create mode 100644 include/asm-generic/ticket-lock.h
include/asm-generic/ticket-lock.h new file +0 -0
new file mode 100644
index 000000000000..59373de3e32a
...
include/asm-generic/ticket-lock.h
Quoted 3 lines
include/asm-generic/ticket-lock.h new file +3 -0
+static __always_inline void ticket_unlock(arch_spinlock_t *lock)
+{
+	u16 *ptr = (u16 *)lock + __is_defined(__BIG_ENDIAN);
As mentioned, this patch series breaks SMP on OpenRISC. I traced it to this line. The above __is_defined(__BIG_ENDIAN) does not return 1 as expected even on BIG_ENDIAN machines. This works:
include/asm-generic/ticket-lock.h +2 -1
index 59373de3e32a..52b5dc9ffdba 100644
@@ -26,6 +26,7 @@
   #define __ASM_GENERIC_TICKET_LOCK_H
   #include <linux/atomic.h>
+#include <linux/kconfig.h>
   #include <asm-generic/ticket-lock-types.h>
   static __always_inline void ticket_lock(arch_spinlock_t *lock)
@@ -51,7 +52,7 @@ static __always_inline bool ticket_trylock(arch_spinlock_t *lock)
   static __always_inline void ticket_unlock(arch_spinlock_t *lock)
   {
-       u16 *ptr = (u16 *)lock + __is_defined(__BIG_ENDIAN);
+       u16 *ptr = (u16 *)lock + IS_ENABLED(CONFIG_CPU_BIG_ENDIAN);
          u32 val = atomic_read(lock);
          smp_store_release(ptr, (u16)val + 1);
 
 
Quoted 7 lines
Quoted 5 lines
include/asm-generic/ticket-lock.h
Quoted 5 lines
include/asm-generic/ticket-lock.h new file +5 -0
+	u32 val = atomic_read(lock);
+
+	smp_store_release(ptr, (u16)val + 1);
+}
+
__BIG_ENDIAN is defined in <linux/kconfig.h>. I believe that if you include <linux/kconfig.h>, the second hunk is not really needed and vice versa.
Quoted 3 lines
I thought so too, but it doesn't seem to work. I think __is_defined is not doing what we think in this context. It looks like __is_defined works when a macro is defined as 1, in this case we have __BIG_ENDIAN 4321.

Quoted 5 lines

You are right. __is_defined() only for 1 or not 1. So it can't be used for __BIG_ENDIAN.
I was not aware of that. Anyway, the <linux/kconfig.h> include is not really needed then.

Since we use IS_ENABLED which is defined in <linux/kconfig.h> I thought its best to include it. However, it seems the requirement for explicit includes differs from subsystem to subsystem.

Whoever picks up this patch for the series can drop the include. I confirm it works even if we drop the include.

-Stafford

[email protected]

author maintainer

2022-03-19 03:54 UTC #9 lore raw

PATCH v2 2/5 asm-generic: qspinlock: Indicate the use of mixed-size atomics

In reply to [email protected]

From: Peter Zijlstra <[email protected]>

The qspinlock implementation depends on having well behaved mixed-size atomics. This is true on the more widely-used platforms, but these requirements are somewhat subtle and may not be satisfied by all the platforms that qspinlock is used on.

Document these requirements, so ports that use qspinlock can more easily determine if they meet these requirements.

Signed-off-by Palmer Dabbelt <[email protected]>

Signed-off-by Peter Zijlstra (Intel) <[email protected]>

Acked-by Waiman Long <[email protected]>

---
 include/asm-generic/qspinlock.h | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

include/asm-generic/qspinlock.h +30 -0

index d74b13825501..a7a1296b0b4d 100644
@@ -2,6 +2,36 @@
 /*
  * Queued spinlock
  *
+ * A 'generic' spinlock implementation that is based on MCS locks. An
+ * architecture that's looking for a 'generic' spinlock, please first consider
+ * ticket-lock.h and only come looking here when you've considered all the
+ * constraints below and can show your hardware does actually perform better
+ * with qspinlock.
+ *
+ *
+ * It relies on atomic_*_release()/atomic_*_acquire() to be RCsc (or no weaker
+ * than RCtso if you're power), where regular code only expects atomic_t to be
+ * RCpc.
+ *
+ * It relies on a far greater (compared to ticket-lock.h) set of atomic
+ * operations to behave well together, please audit them carefully to ensure
+ * they all have forward progress. Many atomic operations may default to
+ * cmpxchg() loops which will not have good forward progress properties on
+ * LL/SC architectures.
+ *
+ * One notable example is atomic_fetch_or_acquire(), which x86 cannot (cheaply)
+ * do. Carefully read the patches that introduced queued_fetch_set_pending_acquire().
+ *
+ * It also heavily relies on mixed size atomic operations, in specific it
+ * requires architectures to have xchg16; something which many LL/SC
+ * architectures need to implement as a 32bit and+or in order to satisfy the
+ * forward progress guarantees mentioned above.
+ *
+ * Further reading on mixed size atomics that might be relevant:
+ *
+ *   http://www.cl.cam.ac.uk/~pes20/popl17/mixed-size.pdf
+ *
+ *
  * (C) Copyright 2013-2015 Hewlett-Packard Development Company, L.P.
  * (C) Copyright 2015 Hewlett-Packard Enterprise Development LP
  *

Signature · 1 line

2.25.1

[email protected]

author maintainer

2022-03-19 03:54 UTC #10 lore raw

PATCH v2 3/5 csky: Move to generic ticket-spinlock

In reply to [email protected]

From: Guo Ren <[email protected]>

There is no benefit from custom implementation for ticket-spinlock, so move to generic ticket-spinlock for easy maintenance.

Signed-off-by Guo Ren <[email protected]>

Cc Palmer Dabbelt <[email protected]>

---
 arch/csky/include/asm/Kbuild           |  3 +-
 arch/csky/include/asm/spinlock.h       | 89 --------------------------
 arch/csky/include/asm/spinlock_types.h | 27 --------
 3 files changed, 2 insertions(+), 117 deletions(-)
 delete mode 100644 arch/csky/include/asm/spinlock.h
 delete mode 100644 arch/csky/include/asm/spinlock_types.h

arch/csky/include/asm/Kbuild +2 -1

index 904a18a818be..056625619c19 100644
@@ -3,6 +3,7 @@ generic-y += asm-offsets.h
 generic-y += extable.h
 generic-y += gpio.h
 generic-y += kvm_para.h
-generic-y += qrwlock.h
+generic-y += spinlock_types.h
+generic-y += spinlock.h
 generic-y += user.h
 generic-y += vmlinux.lds.h

arch/csky/include/asm/spinlock.h deleted +0 ~~-89~~

89 lines not on this page · raw message

arch/csky/include/asm/spinlock_types.h deleted +0 ~~-27~~

deleted file mode 100644
index db87a12c3827..000000000000
@@ -1,27 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-
-#ifndef __ASM_CSKY_SPINLOCK_TYPES_H
-#define __ASM_CSKY_SPINLOCK_TYPES_H
-
-#ifndef __LINUX_SPINLOCK_TYPES_RAW_H
-# error "please don't include this file directly"
-#endif
-
-#define TICKET_NEXT	16
-
-typedef struct {
-	union {
-		u32 lock;
-		struct __raw_tickets {
-			/* little endian */
-			u16 owner;
-			u16 next;
-		} tickets;
-	};
-} arch_spinlock_t;
-
-#define __ARCH_SPIN_LOCK_UNLOCKED	{ { 0 } }
-
-#include <asm-generic/qrwlock_types.h>
-
-#endif /* __ASM_CSKY_SPINLOCK_TYPES_H */

Signature · 1 line

2.25.1

[email protected]

author maintainer

2022-03-19 03:54 UTC #11 lore raw

PATCH v2 4/5 RISC-V: Move to ticket-spinlocks & RW locks

In reply to [email protected]

From: Palmer Dabbelt <[email protected]>

Our existing spinlocks aren't fair and replacing them has been on the TODO list for a long time. This moves to the recently-introduced ticket spinlocks, which are simple enough that they are likely to be correct and fast on the vast majority of extant implementations.

With the move to fair spinlocks, we might as well move to fair rwlocks.

Signed-off-by Palmer Dabbelt <[email protected]>

---
 arch/riscv/Kconfig                      |   1 +
 arch/riscv/include/asm/Kbuild           |   2 +
 arch/riscv/include/asm/spinlock.h       | 135 ------------------------
 arch/riscv/include/asm/spinlock_types.h |  25 -----
 4 files changed, 3 insertions(+), 160 deletions(-)
 delete mode 100644 arch/riscv/include/asm/spinlock.h
 delete mode 100644 arch/riscv/include/asm/spinlock_types.h

arch/riscv/Kconfig +1 -0

index 5adcbd9b5e88..feb7030cfb6d 100644
@@ -38,6 +38,7 @@ config RISCV
 	select ARCH_SUPPORTS_DEBUG_PAGEALLOC if MMU
 	select ARCH_SUPPORTS_HUGETLBFS if MMU
 	select ARCH_USE_MEMTEST
+	select ARCH_USE_QUEUED_RWLOCKS
 	select ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT if MMU
 	select ARCH_WANT_FRAME_POINTERS
 	select ARCH_WANT_HUGE_PMD_SHARE if 64BIT

arch/riscv/include/asm/Kbuild +2 -0

index 57b86fd9916c..99dab544e5fa 100644
@@ -2,5 +2,7 @@
 generic-y += early_ioremap.h
 generic-y += flat.h
 generic-y += kvm_para.h
+generic-y += spinlock_types.h
+generic-y += spinlock.h
 generic-y += user.h
 generic-y += vmlinux.lds.h

arch/riscv/include/asm/spinlock.h deleted +0 ~~-135~~

135 lines not on this page · raw message

arch/riscv/include/asm/spinlock_types.h deleted +0 ~~-25~~

deleted file mode 100644
index 5a35a49505da..000000000000
@@ -1,25 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * Copyright (C) 2015 Regents of the University of California
- */
-
-#ifndef _ASM_RISCV_SPINLOCK_TYPES_H
-#define _ASM_RISCV_SPINLOCK_TYPES_H
-
-#ifndef __LINUX_SPINLOCK_TYPES_RAW_H
-# error "please don't include this file directly"
-#endif
-
-typedef struct {
-	volatile unsigned int lock;
-} arch_spinlock_t;
-
-#define __ARCH_SPIN_LOCK_UNLOCKED	{ 0 }
-
-typedef struct {
-	volatile unsigned int lock;
-} arch_rwlock_t;
-
-#define __ARCH_RW_LOCK_UNLOCKED		{ 0 }
-
-#endif /* _ASM_RISCV_SPINLOCK_TYPES_H */

Signature · 1 line

2.25.1

[email protected]

author maintainer

2022-03-19 03:54 UTC #12 lore raw

PATCH v2 5/5 openrisc: Move to ticket-spinlock

In reply to [email protected]

From: Peter Zijlstra <[email protected]>

We have no indications that openrisc meets the qspinlock requirements, so move to ticket-spinlock as that is more likey to be correct.

Remove duplicate arch_spin_relax, arch_read_relax, arch_write_relax definition.

Signed-off-by Palmer Dabbelt <[email protected]>

Signed-off-by Guo Ren <[email protected]>

---
 arch/openrisc/Kconfig                      |  1 -
 arch/openrisc/include/asm/Kbuild           |  7 ++----
 arch/openrisc/include/asm/spinlock.h       | 27 ----------------------
 arch/openrisc/include/asm/spinlock_types.h |  7 ------
 4 files changed, 2 insertions(+), 40 deletions(-)
 delete mode 100644 arch/openrisc/include/asm/spinlock.h
 delete mode 100644 arch/openrisc/include/asm/spinlock_types.h

arch/openrisc/Kconfig +0 -1

index f724b3f1aeed..f5fa226362f6 100644
@@ -30,7 +30,6 @@ config OPENRISC
 	select HAVE_DEBUG_STACKOVERFLOW
 	select OR1K_PIC
 	select CPU_NO_EFFICIENT_FFS if !OPENRISC_HAVE_INST_FF1
-	select ARCH_USE_QUEUED_SPINLOCKS
 	select ARCH_USE_QUEUED_RWLOCKS
 	select OMPIC if SMP
 	select ARCH_WANT_FRAME_POINTERS

arch/openrisc/include/asm/Kbuild +2 -5

index ca5987e11053..1df59e446b46 100644
@@ -1,9 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 generic-y += extable.h
 generic-y += kvm_para.h
-generic-y += mcs_spinlock.h
-generic-y += qspinlock_types.h
-generic-y += qspinlock.h
-generic-y += qrwlock_types.h
-generic-y += qrwlock.h
+generic-y += spinlock_types.h
+generic-y += spinlock.h
 generic-y += user.h

arch/openrisc/include/asm/spinlock.h deleted +0 ~~-27~~

deleted file mode 100644
index 264944a71535..000000000000
@@ -1,27 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-or-later */
-/*
- * OpenRISC Linux
- *
- * Linux architectural port borrowing liberally from similar works of
- * others.  All original copyrights apply as per the original source
- * declaration.
- *
- * OpenRISC implementation:
- * Copyright (C) 2003 Matjaz Breskvar <[email protected]>
- * Copyright (C) 2010-2011 Jonas Bonn <[email protected]>
- * et al.
- */
-
-#ifndef __ASM_OPENRISC_SPINLOCK_H
-#define __ASM_OPENRISC_SPINLOCK_H
-
-#include <asm/qspinlock.h>
-
-#include <asm/qrwlock.h>
-
-#define arch_spin_relax(lock)	cpu_relax()
-#define arch_read_relax(lock)	cpu_relax()
-#define arch_write_relax(lock)	cpu_relax()
-
-
-#endif

arch/openrisc/include/asm/spinlock_types.h deleted +0 -7

deleted file mode 100644
index 7c6fb1208c88..000000000000
@@ -1,7 +0,0 @@
-#ifndef _ASM_OPENRISC_SPINLOCK_TYPES_H
-#define _ASM_OPENRISC_SPINLOCK_TYPES_H
-
-#include <asm/qspinlock_types.h>
-#include <asm/qrwlock_types.h>
-
-#endif /* _ASM_OPENRISC_SPINLOCK_TYPES_H */

Signature · 1 line

2.25.1

A message that is not in the archive (sent off-list or not yet received): [email protected]

Guo Ren

author maintainer

2022-03-20 03:05 UTC #14 lore raw

PATCH v2 5/5 openrisc: Move to ticket-spinlock

Hi openrisc guys,

Quoted 2 lines

kernel/signal.c:2625:49: sparse: expected struct sighand_struct *sighand
kernel/signal.c:2625:49: sparse: got struct sighand_struct [noderef] __rcu *sighand

Some warning here, Is that all right? I don't think it is because of changing arch_spinlock_t from struct qspinlock to atomic_t.

Quoted 198 lines · kernel test robot

Hi,
I love your patch! Perhaps something to improve:
[auto build test WARNING on arnd-asm-generic/master]
[also build test WARNING on tip/locking/core openrisc/for-next linus/master v5.17-rc8 next-20220318] [If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:    https://github.com/0day-ci/linux/commits/guoren-kernel-org/Generic-Ticket-Spinlocks/20220319-115644
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arnd/asm-generic.git master
config: openrisc-randconfig-s032-20220319 (https://download.01.org/0day-ci/archive/20220320/202203200824.EQJTy8pW-lkp at intel.com/config)
compiler: or1k-linux-gcc (GCC) 11.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/0day-ci/linux/commit/4e66dc8c71c62011bcb287f66bf5c5363920cd91
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review guoren-kernel-org/Generic-Ticket-Spinlocks/20220319-115644
git checkout 4e66dc8c71c62011bcb287f66bf5c5363920cd91
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=openrisc SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by kernel test robot <[email protected]>
sparse warnings: (new ones prefixed by >>)
kernel/signal.c: note: in included file (through include/uapi/asm-generic/signal.h, include/asm-generic/signal.h, arch/openrisc/include/generated/uapi/asm/signal.h, ...):
include/uapi/asm-generic/signal-defs.h:83:29: sparse: sparse: multiple address spaces given
kernel/signal.c:195:31: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:195:31: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:195:31: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:198:33: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:198:33: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:198:33: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:480:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:480:9: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:480:9: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:484:34: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:484:34: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:484:34: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:517:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:517:9: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:517:9: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:520:36: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:520:36: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:520:36: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:542:53: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected struct k_sigaction *ka @@ got struct k_sigaction [noderef] __rcu * @@
kernel/signal.c:542:53: sparse: expected struct k_sigaction *ka
kernel/signal.c:542:53: sparse: got struct k_sigaction [noderef] __rcu *
include/uapi/asm-generic/signal-defs.h:83:29: sparse: sparse: multiple address spaces given
kernel/signal.c:698:33: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:698:33: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:698:33: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:700:31: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:700:31: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:700:31: sparse: got struct spinlock [noderef] __rcu *
Quoted 1 line
Quoted 1 line
kernel/signal.c:887:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct atomic_t [usertype] *lock @@ got struct atomic_t [noderef] __rcu * @@
kernel/signal.c:887:9: sparse: expected struct atomic_t [usertype] *lock
kernel/signal.c:887:9: sparse: got struct atomic_t [noderef] __rcu *
kernel/signal.c:1082:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct atomic_t [usertype] *lock @@ got struct atomic_t [noderef] __rcu * @@
kernel/signal.c:1082:9: sparse: expected struct atomic_t [usertype] *lock
kernel/signal.c:1082:9: sparse: got struct atomic_t [noderef] __rcu *
kernel/signal.c:1330:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:1330:9: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:1330:9: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:1331:16: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct k_sigaction *action @@ got struct k_sigaction [noderef] __rcu * @@
kernel/signal.c:1331:16: sparse: expected struct k_sigaction *action
kernel/signal.c:1331:16: sparse: got struct k_sigaction [noderef] __rcu *
kernel/signal.c:1350:34: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:1350:34: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:1350:34: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:1928:36: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:1928:36: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:1928:36: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:2038:44: sparse: sparse: cast removes address space '__rcu' of expression
kernel/signal.c:2057:65: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct task_struct *tsk @@ got struct task_struct [noderef] __rcu *parent @@
kernel/signal.c:2057:65: sparse: expected struct task_struct *tsk
kernel/signal.c:2057:65: sparse: got struct task_struct [noderef] __rcu *parent
kernel/signal.c:2058:40: sparse: sparse: cast removes address space '__rcu' of expression
kernel/signal.c:2076:14: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct sighand_struct *psig @@ got struct sighand_struct [noderef] __rcu *[noderef] __rcu sighand @@
kernel/signal.c:2076:14: sparse: expected struct sighand_struct *psig
kernel/signal.c:2076:14: sparse: got struct sighand_struct [noderef] __rcu *[noderef] __rcu sighand
kernel/signal.c:2105:46: sparse: sparse: incorrect type in argument 3 (different address spaces) @@ expected struct task_struct *t @@ got struct task_struct [noderef] __rcu *parent @@
kernel/signal.c:2105:46: sparse: expected struct task_struct *t
kernel/signal.c:2105:46: sparse: got struct task_struct [noderef] __rcu *parent
kernel/signal.c:2106:34: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected struct task_struct *parent @@ got struct task_struct [noderef] __rcu *parent @@
kernel/signal.c:2106:34: sparse: expected struct task_struct *parent
kernel/signal.c:2106:34: sparse: got struct task_struct [noderef] __rcu *parent
kernel/signal.c:2135:24: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct task_struct *parent @@ got struct task_struct [noderef] __rcu *parent @@
kernel/signal.c:2135:24: sparse: expected struct task_struct *parent
kernel/signal.c:2135:24: sparse: got struct task_struct [noderef] __rcu *parent
kernel/signal.c:2138:24: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct task_struct *parent @@ got struct task_struct [noderef] __rcu *real_parent @@
kernel/signal.c:2138:24: sparse: expected struct task_struct *parent
kernel/signal.c:2138:24: sparse: got struct task_struct [noderef] __rcu *real_parent
kernel/signal.c:2171:17: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct sighand_struct *sighand @@ got struct sighand_struct [noderef] __rcu *sighand @@
kernel/signal.c:2171:17: sparse: expected struct sighand_struct *sighand
kernel/signal.c:2171:17: sparse: got struct sighand_struct [noderef] __rcu *sighand
kernel/signal.c:2209:41: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:2209:41: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:2209:41: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:2211:39: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:2211:39: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:2211:39: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:2261:33: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:2261:33: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:2261:33: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:2316:31: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:2316:31: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:2316:31: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:2350:31: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:2350:31: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:2350:31: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:2352:33: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:2352:33: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:2352:33: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:2450:41: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:2450:41: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:2450:41: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:2535:41: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:2535:41: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:2535:41: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:2547:33: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:2547:33: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:2547:33: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:2585:52: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct task_struct *tsk @@ got struct task_struct [noderef] __rcu *parent @@
kernel/signal.c:2585:52: sparse: expected struct task_struct *tsk
kernel/signal.c:2585:52: sparse: got struct task_struct [noderef] __rcu *parent
kernel/signal.c:2587:49: sparse: sparse: cast removes address space '__rcu' of expression
kernel/signal.c:2625:49: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected struct sighand_struct *sighand @@ got struct sighand_struct [noderef] __rcu *sighand @@
kernel/signal.c:2625:49: sparse: expected struct sighand_struct *sighand
kernel/signal.c:2625:49: sparse: got struct sighand_struct [noderef] __rcu *sighand
kernel/signal.c:2961:27: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:2961:27: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:2961:27: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:2981:29: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:2981:29: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:2981:29: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:3048:27: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:3048:27: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:3048:27: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:3050:29: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:3050:29: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:3050:29: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:3201:31: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:3201:31: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:3201:31: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:3204:33: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:3204:33: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:3204:33: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:3591:27: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:3591:27: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:3591:27: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:3603:37: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:3603:37: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:3603:37: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:3608:35: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:3608:35: sparse: expected struct spinlock [usertype] *lock
vim +887 kernel/signal.c
^1da177e4c3f415 Linus Torvalds 2005-04-16  866
fb1d910c178ba0c Tejun Heo      2011-06-14  867  /**
fb1d910c178ba0c Tejun Heo      2011-06-14  868   * ptrace_trap_notify - schedule trap to notify ptracer
fb1d910c178ba0c Tejun Heo      2011-06-14  869   * @t: tracee wanting to notify tracer
fb1d910c178ba0c Tejun Heo      2011-06-14  870   *
fb1d910c178ba0c Tejun Heo      2011-06-14  871   * This function schedules sticky ptrace trap which is cleared on the next
fb1d910c178ba0c Tejun Heo      2011-06-14  872   * TRAP_STOP to notify ptracer of an event.  @t must have been seized by
fb1d910c178ba0c Tejun Heo      2011-06-14  873   * ptracer.
fb1d910c178ba0c Tejun Heo      2011-06-14  874   *
544b2c91a9f14f9 Tejun Heo      2011-06-14  875   * If @t is running, STOP trap will be taken.  If trapped for STOP and
544b2c91a9f14f9 Tejun Heo      2011-06-14  876   * ptracer is listening for events, tracee is woken up so that it can
544b2c91a9f14f9 Tejun Heo      2011-06-14  877   * re-trap for the new event.  If trapped otherwise, STOP trap will be
544b2c91a9f14f9 Tejun Heo      2011-06-14  878   * eventually taken without returning to userland after the existing traps
544b2c91a9f14f9 Tejun Heo      2011-06-14  879   * are finished by PTRACE_CONT.
fb1d910c178ba0c Tejun Heo      2011-06-14  880   *
fb1d910c178ba0c Tejun Heo      2011-06-14  881   * CONTEXT:
fb1d910c178ba0c Tejun Heo      2011-06-14  882   * Must be called with @task->sighand->siglock held.
fb1d910c178ba0c Tejun Heo      2011-06-14  883   */
fb1d910c178ba0c Tejun Heo      2011-06-14  884  static void ptrace_trap_notify(struct task_struct *t)
fb1d910c178ba0c Tejun Heo      2011-06-14  885  {
fb1d910c178ba0c Tejun Heo      2011-06-14  886          WARN_ON_ONCE(!(t->ptrace & PT_SEIZED));
fb1d910c178ba0c Tejun Heo      2011-06-14 @887          assert_spin_locked(&t->sighand->siglock);
fb1d910c178ba0c Tejun Heo      2011-06-14  888
fb1d910c178ba0c Tejun Heo      2011-06-14  889          task_set_jobctl_pending(t, JOBCTL_TRAP_NOTIFY);
910ffdb18a6408e Oleg Nesterov  2013-01-21  890          ptrace_signal_wake_up(t, t->jobctl & JOBCTL_LISTENING);
fb1d910c178ba0c Tejun Heo      2011-06-14  891  }
fb1d910c178ba0c Tejun Heo      2011-06-14  892
--
0-DAY CI Kernel Test Service
https://01.org/lkp

Signature · 4 lines

Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

Stafford Horne

maintainer

2022-03-20 03:36 UTC #15 lore raw

PATCH v2 5/5 openrisc: Move to ticket-spinlock

Quoted 10 lines · Guo Ren

Hi openrisc guys,
Quoted 1 line
kernel/signal.c:2625:49: sparse: expected struct sighand_struct
*sighand
Quoted 1 line
kernel/signal.c:2625:49: sparse: got struct sighand_struct
[noderef] __rcu *sighand
Some warning here, Is that all right? I don't think it is because of changing arch_spinlock_t from struct qspinlock to atomic_t.

I haven't built or tested this series yet. But this doesn't look like a new problem. It looks like this patch series may have introduced a new instance of the existing issue.

I have some patches to clean up sparse warnings and I've seen this pattern before but haven't spent the time to clean it up. This is a good opportunity to do that now. Let me have a look in the next few days.

-Stafford

Quoted 399 lines · kernel test robot

Quoted 7 lines
Hi,
I love your patch! Perhaps something to improve:
[auto build test WARNING on arnd-asm-generic/master] [also build test WARNING on tip/locking/core openrisc/for-next
linus/master v5.17-rc8 next-20220318]
Quoted 5 lines
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/guoren-kernel-org/Generic-Ticket-Spinlocks/20220319-115644
Quoted 1 line
base:
https://git.kernel.org/pub/scm/linux/kernel/git/arnd/asm-generic.git
master
Quoted 1 line
config: openrisc-randconfig-s032-20220319 (
https://download.01.org/0day-ci/archive/20220320/202203200824.EQJTy8pW-lkp at intel.com/config )
Quoted 3 lines
compiler: or1k-linux-gcc (GCC) 11.2.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
-O ~/bin/make.cross
Quoted 4 lines
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-dirty
#
https://github.com/0day-ci/linux/commit/4e66dc8c71c62011bcb287f66bf5c5363920cd91
Quoted 2 lines
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
guoren-kernel-org/Generic-Ticket-Spinlocks/20220319-115644
Quoted 4 lines
git checkout 4e66dc8c71c62011bcb287f66bf5c5363920cd91 # save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross
C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=openrisc SHELL=/bin/bash
Quoted 7 lines
If you fix the issue, kindly add following tag as appropriate
Reported-by kernel test robot <[email protected]>
sparse warnings: (new ones prefixed by >>)
kernel/signal.c: note: in included file (through
include/uapi/asm-generic/signal.h, include/asm-generic/signal.h, arch/openrisc/include/generated/uapi/asm/signal.h, ...):
Quoted 1 line
include/uapi/asm-generic/signal-defs.h:83:29: sparse: sparse:
multiple address spaces given
Quoted 1 line
kernel/signal.c:195:31: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct spinlock [usertype] *lock
@@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:195:31: sparse: expected struct spinlock
[usertype] *lock
Quoted 1 line
kernel/signal.c:195:31: sparse: got struct spinlock [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:198:33: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct spinlock [usertype] *lock
@@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:198:33: sparse: expected struct spinlock
[usertype] *lock
Quoted 1 line
kernel/signal.c:198:33: sparse: got struct spinlock [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:480:9: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct spinlock [usertype] *lock
@@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:480:9: sparse: expected struct spinlock
[usertype] *lock
Quoted 1 line
kernel/signal.c:480:9: sparse: got struct spinlock [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:484:34: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct spinlock [usertype] *lock
@@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:484:34: sparse: expected struct spinlock
[usertype] *lock
Quoted 1 line
kernel/signal.c:484:34: sparse: got struct spinlock [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:517:9: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct spinlock [usertype] *lock
@@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:517:9: sparse: expected struct spinlock
[usertype] *lock
Quoted 1 line
kernel/signal.c:517:9: sparse: got struct spinlock [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:520:36: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct spinlock [usertype] *lock
@@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:520:36: sparse: expected struct spinlock
[usertype] *lock
Quoted 1 line
kernel/signal.c:520:36: sparse: got struct spinlock [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:542:53: sparse: sparse: incorrect type in initializer
(different address spaces) @@ expected struct k_sigaction *ka @@
got struct k_sigaction [noderef] __rcu * @@
Quoted 2 lines
kernel/signal.c:542:53: sparse: expected struct k_sigaction *ka
kernel/signal.c:542:53: sparse: got struct k_sigaction [noderef]
__rcu *
Quoted 1 line
include/uapi/asm-generic/signal-defs.h:83:29: sparse: sparse:
multiple address spaces given
Quoted 1 line
kernel/signal.c:698:33: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct spinlock [usertype] *lock
@@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:698:33: sparse: expected struct spinlock
[usertype] *lock
Quoted 1 line
kernel/signal.c:698:33: sparse: got struct spinlock [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:700:31: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct spinlock [usertype] *lock
@@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:700:31: sparse: expected struct spinlock
[usertype] *lock
Quoted 1 line
kernel/signal.c:700:31: sparse: got struct spinlock [noderef]
__rcu *
Quoted 1 line
Quoted 1 line
Quoted 1 line
kernel/signal.c:887:9: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct atomic_t [usertype] *lock
@@ got struct atomic_t [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:887:9: sparse: expected struct atomic_t
[usertype] *lock
Quoted 1 line
kernel/signal.c:887:9: sparse: got struct atomic_t [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:1082:9: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct atomic_t [usertype] *lock
@@ got struct atomic_t [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:1082:9: sparse: expected struct atomic_t
[usertype] *lock
Quoted 1 line
kernel/signal.c:1082:9: sparse: got struct atomic_t [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:1330:9: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct spinlock [usertype] *lock
@@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:1330:9: sparse: expected struct spinlock
[usertype] *lock
Quoted 1 line
kernel/signal.c:1330:9: sparse: got struct spinlock [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:1331:16: sparse: sparse: incorrect type in assignment
(different address spaces) @@ expected struct k_sigaction *action @@
got struct k_sigaction [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:1331:16: sparse: expected struct k_sigaction
*action
Quoted 1 line
kernel/signal.c:1331:16: sparse: got struct k_sigaction [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:1350:34: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct spinlock [usertype] *lock
@@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:1350:34: sparse: expected struct spinlock
[usertype] *lock
Quoted 1 line
kernel/signal.c:1350:34: sparse: got struct spinlock [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:1928:36: sparse: sparse: incorrect type in
initializer (different address spaces) @@ expected struct spinlock
[usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:1928:36: sparse: expected struct spinlock
[usertype] *lock
Quoted 1 line
kernel/signal.c:1928:36: sparse: got struct spinlock [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:2038:44: sparse: sparse: cast removes address space
'__rcu' of expression
Quoted 1 line
kernel/signal.c:2057:65: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct task_struct *tsk @@
got struct task_struct [noderef] __rcu *parent @@
Quoted 2 lines
kernel/signal.c:2057:65: sparse: expected struct task_struct *tsk
kernel/signal.c:2057:65: sparse: got struct task_struct [noderef]
__rcu *parent
Quoted 1 line
kernel/signal.c:2058:40: sparse: sparse: cast removes address space
'__rcu' of expression
Quoted 1 line
kernel/signal.c:2076:14: sparse: sparse: incorrect type in assignment
(different address spaces) @@ expected struct sighand_struct *psig @@
got struct sighand_struct [noderef] __rcu *[noderef] __rcu sighand @@
Quoted 1 line
kernel/signal.c:2076:14: sparse: expected struct sighand_struct
*psig
Quoted 1 line
kernel/signal.c:2076:14: sparse: got struct sighand_struct
[noderef] __rcu *[noderef] __rcu sighand
Quoted 1 line
kernel/signal.c:2105:46: sparse: sparse: incorrect type in argument 3
(different address spaces) @@ expected struct task_struct *t @@ got
struct task_struct [noderef] __rcu *parent @@
Quoted 2 lines
kernel/signal.c:2105:46: sparse: expected struct task_struct *t
kernel/signal.c:2105:46: sparse: got struct task_struct [noderef]
__rcu *parent
Quoted 1 line
kernel/signal.c:2106:34: sparse: sparse: incorrect type in argument 2
(different address spaces) @@ expected struct task_struct *parent @@
got struct task_struct [noderef] __rcu *parent @@
Quoted 1 line
kernel/signal.c:2106:34: sparse: expected struct task_struct
*parent
Quoted 1 line
kernel/signal.c:2106:34: sparse: got struct task_struct [noderef]
__rcu *parent
Quoted 1 line
kernel/signal.c:2135:24: sparse: sparse: incorrect type in assignment
(different address spaces) @@ expected struct task_struct *parent @@
got struct task_struct [noderef] __rcu *parent @@
Quoted 1 line
kernel/signal.c:2135:24: sparse: expected struct task_struct
*parent
Quoted 1 line
kernel/signal.c:2135:24: sparse: got struct task_struct [noderef]
__rcu *parent
Quoted 1 line
kernel/signal.c:2138:24: sparse: sparse: incorrect type in assignment
(different address spaces) @@ expected struct task_struct *parent @@
got struct task_struct [noderef] __rcu *real_parent @@
Quoted 1 line
kernel/signal.c:2138:24: sparse: expected struct task_struct
*parent
Quoted 1 line
kernel/signal.c:2138:24: sparse: got struct task_struct [noderef]
__rcu *real_parent
Quoted 1 line
kernel/signal.c:2171:17: sparse: sparse: incorrect type in assignment
(different address spaces) @@ expected struct sighand_struct *sighand
@@ got struct sighand_struct [noderef] __rcu *sighand @@
Quoted 1 line
kernel/signal.c:2171:17: sparse: expected struct sighand_struct
*sighand
Quoted 1 line
kernel/signal.c:2171:17: sparse: got struct sighand_struct
[noderef] __rcu *sighand
Quoted 1 line
kernel/signal.c:2209:41: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct spinlock [usertype] *lock
@@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:2209:41: sparse: expected struct spinlock
[usertype] *lock
Quoted 1 line
kernel/signal.c:2209:41: sparse: got struct spinlock [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:2211:39: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct spinlock [usertype] *lock
@@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:2211:39: sparse: expected struct spinlock
[usertype] *lock
Quoted 1 line
kernel/signal.c:2211:39: sparse: got struct spinlock [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:2261:33: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct spinlock [usertype] *lock
@@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:2261:33: sparse: expected struct spinlock
[usertype] *lock
Quoted 1 line
kernel/signal.c:2261:33: sparse: got struct spinlock [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:2316:31: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct spinlock [usertype] *lock
@@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:2316:31: sparse: expected struct spinlock
[usertype] *lock
Quoted 1 line
kernel/signal.c:2316:31: sparse: got struct spinlock [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:2350:31: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct spinlock [usertype] *lock
@@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:2350:31: sparse: expected struct spinlock
[usertype] *lock
Quoted 1 line
kernel/signal.c:2350:31: sparse: got struct spinlock [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:2352:33: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct spinlock [usertype] *lock
@@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:2352:33: sparse: expected struct spinlock
[usertype] *lock
Quoted 1 line
kernel/signal.c:2352:33: sparse: got struct spinlock [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:2450:41: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct spinlock [usertype] *lock
@@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:2450:41: sparse: expected struct spinlock
[usertype] *lock
Quoted 1 line
kernel/signal.c:2450:41: sparse: got struct spinlock [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:2535:41: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct spinlock [usertype] *lock
@@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:2535:41: sparse: expected struct spinlock
[usertype] *lock
Quoted 1 line
kernel/signal.c:2535:41: sparse: got struct spinlock [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:2547:33: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct spinlock [usertype] *lock
@@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:2547:33: sparse: expected struct spinlock
[usertype] *lock
Quoted 1 line
kernel/signal.c:2547:33: sparse: got struct spinlock [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:2585:52: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct task_struct *tsk @@
got struct task_struct [noderef] __rcu *parent @@
Quoted 2 lines
kernel/signal.c:2585:52: sparse: expected struct task_struct *tsk
kernel/signal.c:2585:52: sparse: got struct task_struct [noderef]
__rcu *parent
Quoted 1 line
kernel/signal.c:2587:49: sparse: sparse: cast removes address space
'__rcu' of expression
Quoted 1 line
kernel/signal.c:2625:49: sparse: sparse: incorrect type in
initializer (different address spaces) @@ expected struct
sighand_struct *sighand @@ got struct sighand_struct [noderef] __rcu
*sighand @@
Quoted 1 line
kernel/signal.c:2625:49: sparse: expected struct sighand_struct
*sighand
Quoted 1 line
kernel/signal.c:2625:49: sparse: got struct sighand_struct
[noderef] __rcu *sighand
Quoted 1 line
kernel/signal.c:2961:27: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct spinlock [usertype] *lock
@@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:2961:27: sparse: expected struct spinlock
[usertype] *lock
Quoted 1 line
kernel/signal.c:2961:27: sparse: got struct spinlock [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:2981:29: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct spinlock [usertype] *lock
@@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:2981:29: sparse: expected struct spinlock
[usertype] *lock
Quoted 1 line
kernel/signal.c:2981:29: sparse: got struct spinlock [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:3048:27: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct spinlock [usertype] *lock
@@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:3048:27: sparse: expected struct spinlock
[usertype] *lock
Quoted 1 line
kernel/signal.c:3048:27: sparse: got struct spinlock [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:3050:29: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct spinlock [usertype] *lock
@@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:3050:29: sparse: expected struct spinlock
[usertype] *lock
Quoted 1 line
kernel/signal.c:3050:29: sparse: got struct spinlock [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:3201:31: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct spinlock [usertype] *lock
@@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:3201:31: sparse: expected struct spinlock
[usertype] *lock
Quoted 1 line
kernel/signal.c:3201:31: sparse: got struct spinlock [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:3204:33: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct spinlock [usertype] *lock
@@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:3204:33: sparse: expected struct spinlock
[usertype] *lock
Quoted 1 line
kernel/signal.c:3204:33: sparse: got struct spinlock [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:3591:27: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct spinlock [usertype] *lock
@@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:3591:27: sparse: expected struct spinlock
[usertype] *lock
Quoted 1 line
kernel/signal.c:3591:27: sparse: got struct spinlock [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:3603:37: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct spinlock [usertype] *lock
@@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:3603:37: sparse: expected struct spinlock
[usertype] *lock
Quoted 1 line
kernel/signal.c:3603:37: sparse: got struct spinlock [noderef]
__rcu *
Quoted 1 line
kernel/signal.c:3608:35: sparse: sparse: incorrect type in argument 1
(different address spaces) @@ expected struct spinlock [usertype] *lock
@@ got struct spinlock [noderef] __rcu * @@
Quoted 1 line
kernel/signal.c:3608:35: sparse: expected struct spinlock
[usertype] *lock
Quoted 6 lines
vim +887 kernel/signal.c
^1da177e4c3f415 Linus Torvalds 2005-04-16  866
fb1d910c178ba0c Tejun Heo      2011-06-14  867  /**
fb1d910c178ba0c Tejun Heo      2011-06-14  868   * ptrace_trap_notify -
schedule trap to notify ptracer
Quoted 1 line
fb1d910c178ba0c Tejun Heo 2011-06-14 869 * @t: tracee wanting to
notify tracer
Quoted 2 lines
fb1d910c178ba0c Tejun Heo      2011-06-14  870   *
fb1d910c178ba0c Tejun Heo      2011-06-14  871   * This function
schedules sticky ptrace trap which is cleared on the next
Quoted 1 line
fb1d910c178ba0c Tejun Heo 2011-06-14 872 * TRAP_STOP to notify
ptracer of an event. @t must have been seized by
Quoted 3 lines
fb1d910c178ba0c Tejun Heo      2011-06-14  873   * ptracer.
fb1d910c178ba0c Tejun Heo      2011-06-14  874   *
544b2c91a9f14f9 Tejun Heo      2011-06-14  875   * If @t is running,
STOP trap will be taken. If trapped for STOP and
Quoted 1 line
544b2c91a9f14f9 Tejun Heo 2011-06-14 876 * ptracer is listening
for events, tracee is woken up so that it can
Quoted 1 line
544b2c91a9f14f9 Tejun Heo 2011-06-14 877 * re-trap for the new
event. If trapped otherwise, STOP trap will be
Quoted 1 line
544b2c91a9f14f9 Tejun Heo 2011-06-14 878 * eventually taken
without returning to userland after the existing traps
Quoted 1 line
544b2c91a9f14f9 Tejun Heo 2011-06-14 879 * are finished by
PTRACE_CONT.
Quoted 3 lines
fb1d910c178ba0c Tejun Heo      2011-06-14  880   *
fb1d910c178ba0c Tejun Heo      2011-06-14  881   * CONTEXT:
fb1d910c178ba0c Tejun Heo      2011-06-14  882   * Must be called with
@task->sighand->siglock held.
Quoted 2 lines
fb1d910c178ba0c Tejun Heo      2011-06-14  883   */
fb1d910c178ba0c Tejun Heo      2011-06-14  884  static void
ptrace_trap_notify(struct task_struct *t)
Quoted 2 lines
fb1d910c178ba0c Tejun Heo      2011-06-14  885  {
fb1d910c178ba0c Tejun Heo      2011-06-14  886
WARN_ON_ONCE(!(t->ptrace & PT_SEIZED));
Quoted 1 line
fb1d910c178ba0c Tejun Heo 2011-06-14 @887
assert_spin_locked(&t->sighand->siglock);
Quoted 2 lines
fb1d910c178ba0c Tejun Heo      2011-06-14  888
fb1d910c178ba0c Tejun Heo      2011-06-14  889
task_set_jobctl_pending(t, JOBCTL_TRAP_NOTIFY);
Quoted 1 line
910ffdb18a6408e Oleg Nesterov 2013-01-21 890
ptrace_signal_wake_up(t, t->jobctl & JOBCTL_LISTENING);
Quoted 6 lines
fb1d910c178ba0c Tejun Heo      2011-06-14  891  }
fb1d910c178ba0c Tejun Heo      2011-06-14  892
--
0-DAY CI Kernel Test Service
https://01.org/lkp

Quoted 6 lines

--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.librecores.org/pipermail/openrisc/attachments/20220320/851237c6/attachment-0001.htm>

Stafford Horne

maintainer

2022-03-20 21:13 UTC #16 lore raw

Re: [PATCH V2 5/5] openrisc: Move to ticket-spinlock

Quoted 13 lines · Stafford Horne

Quoted 10 lines · Guo Ren
Hi openrisc guys,
Quoted 1 line
kernel/signal.c:2625:49: sparse: expected struct sighand_struct
*sighand
Quoted 1 line
kernel/signal.c:2625:49: sparse: got struct sighand_struct
[noderef] __rcu *sighand
Some warning here, Is that all right? I don't think it is because of changing arch_spinlock_t from struct qspinlock to atomic_t.

Quoted 7 lines

I haven't built or tested this series yet. But this doesn't look like a new problem. It looks like this patch series may have introduced a new instance of the existing issue.
I have some patches to clean up sparse warnings and I've seen this pattern before but haven't spent the time to clean it up. This is a good opportunity to do that now. Let me have a look in the next few days.

Hello Guo Ren,

I was able to build the patch series for OpenRISC, but after applying the openrisc SMP kernel build no longer can boot. It doesn't boot on single core system or multi-core systems. I don't get any console output to help with debugging it.

It may take be a bit longer to debug this.

-Stafford

Stafford Horne

maintainer

2022-03-22 05:26 UTC #17 lore raw

Re: [PATCH V2 5/5] openrisc: Move to ticket-spinlock

In reply to Guo Ren

Quoted 76 lines · Guo Ren

Hi openrisc guys,
Quoted 2 lines
kernel/signal.c:2625:49: sparse: expected struct sighand_struct *sighand
kernel/signal.c:2625:49: sparse: got struct sighand_struct [noderef] __rcu *sighand
Some warning here, Is that all right? I don't think it is because of changing arch_spinlock_t from struct qspinlock to atomic_t.
Quoted 66 lines · kernel test robot
Hi,
I love your patch! Perhaps something to improve:
[auto build test WARNING on arnd-asm-generic/master]
[also build test WARNING on tip/locking/core openrisc/for-next linus/master v5.17-rc8 next-20220318] [If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:    https://github.com/0day-ci/linux/commits/guoren-kernel-org/Generic-Ticket-Spinlocks/20220319-115644
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arnd/asm-generic.git master
config: openrisc-randconfig-s032-20220319 (https://download.01.org/0day-ci/archive/20220320/202203200824.EQJTy8pW-lkp at intel.com/config)
compiler: or1k-linux-gcc (GCC) 11.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/0day-ci/linux/commit/4e66dc8c71c62011bcb287f66bf5c5363920cd91
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review guoren-kernel-org/Generic-Ticket-Spinlocks/20220319-115644
git checkout 4e66dc8c71c62011bcb287f66bf5c5363920cd91
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=openrisc SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by kernel test robot <[email protected]>
sparse warnings: (new ones prefixed by >>)
kernel/signal.c: note: in included file (through include/uapi/asm-generic/signal.h, include/asm-generic/signal.h, arch/openrisc/include/generated/uapi/asm/signal.h, ...):
include/uapi/asm-generic/signal-defs.h:83:29: sparse: sparse: multiple address spaces given
kernel/signal.c:195:31: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:195:31: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:195:31: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:198:33: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:198:33: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:198:33: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:480:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:480:9: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:480:9: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:484:34: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:484:34: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:484:34: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:517:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:517:9: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:517:9: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:520:36: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:520:36: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:520:36: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:542:53: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected struct k_sigaction *ka @@ got struct k_sigaction [noderef] __rcu * @@
kernel/signal.c:542:53: sparse: expected struct k_sigaction *ka
kernel/signal.c:542:53: sparse: got struct k_sigaction [noderef] __rcu *
include/uapi/asm-generic/signal-defs.h:83:29: sparse: sparse: multiple address spaces given
kernel/signal.c:698:33: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:698:33: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:698:33: sparse: got struct spinlock [noderef] __rcu *
kernel/signal.c:700:31: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct spinlock [usertype] *lock @@ got struct spinlock [noderef] __rcu * @@
kernel/signal.c:700:31: sparse: expected struct spinlock [usertype] *lock
kernel/signal.c:700:31: sparse: got struct spinlock [noderef] __rcu *
Quoted 1 line
Quoted 1 line
kernel/signal.c:887:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct atomic_t [usertype] *lock @@ got struct atomic_t [noderef] __rcu * @@
kernel/signal.c:887:9: sparse: expected struct atomic_t [usertype] *lock
kernel/signal.c:887:9: sparse: got struct atomic_t [noderef] __rcu *

This one is being reported as a new warning. I can see this warning even when testing with x86_64 which uses qspinlock:

kernel/signal.c:542:53: got struct k_sigaction [noderef] __rcu *
./include/uapi/asm-generic/signal-defs.h:83:29: error: multiple
address spaces given
kernel/signal.c:698:33: warning: incorrect type in argument 1
(different address spaces)
kernel/signal.c:698:33: expected struct spinlock [usertype] *lock
kernel/signal.c:698:33: got struct spinlock [noderef] __rcu *
kernel/signal.c:700:31: warning: incorrect type in argument 1
(different address spaces)
kernel/signal.c:700:31: expected struct spinlock [usertype] *lock
kernel/signal.c:700:31: got struct spinlock [noderef] __rcu *
kernel/signal.c:887:9: warning: incorrect type in argument 1
(different address spaces)
kernel/signal.c:887:9: expected struct qspinlock *lock
kernel/signal.c:887:9: got struct qspinlock [noderef] __rcu *
kernel/signal.c:1082:9: warning: incorrect type in argument 1
(different address spaces)
kernel/signal.c:1082:9: expected struct qspinlock *lock

It looks like these are treated as *new* by the kbuild robot because they changed from qspinlock to atomic_t, which changes the match pattern.

I looked into what it takes to fix it but it seems such a change would be very intrusive. I think we can leave as is for now.

-Stafford