Hi Andrea,
On 17/07/2024 18:29, Andrea Parri wrote:
+config RISCV_QUEUED_SPINLOCKS
I'm seeing the following warnings with CONFIG_RISCV_QUEUED_SPINLOCKS=y:
In file included from ./arch/riscv/include/generated/asm/qspinlock.h:1,
from kernel/locking/qspinlock.c:24:
./include/asm-generic/qspinlock.h:144:9: warning: "arch_spin_is_locked" redefined
144 | #define arch_spin_is_locked(l) queued_spin_is_locked(l)
| ^~~~~~~~~~~~~~~~~~~
In file included from ./arch/riscv/include/generated/asm/ticket_spinlock.h:1,
from ./arch/riscv/include/asm/spinlock.h:33,
from ./include/linux/spinlock.h:95,
from ./include/linux/sched.h:2142,
from ./include/linux/percpu.h:13,
from kernel/locking/qspinlock.c:19:
./include/asm-generic/ticket_spinlock.h:97:9: note: this is the location of the previous definition
97 | #define arch_spin_is_locked(l) ticket_spin_is_locked(l)
| ^~~~~~~~~~~~~~~~~~~
./include/asm-generic/qspinlock.h:145:9: warning: "arch_spin_is_contended" redefined
145 | #define arch_spin_is_contended(l) queued_spin_is_contended(l)
| ^~~~~~~~~~~~~~~~~~~~~~
./include/asm-generic/ticket_spinlock.h:98:9: note: this is the location of the previous definition
98 | #define arch_spin_is_contended(l) ticket_spin_is_contended(l)
| ^~~~~~~~~~~~~~~~~~~~~~
./include/asm-generic/qspinlock.h:146:9: warning: "arch_spin_value_unlocked" redefined
146 | #define arch_spin_value_unlocked(l) queued_spin_value_unlocked(l)
| ^~~~~~~~~~~~~~~~~~~~~~~~
./include/asm-generic/ticket_spinlock.h:99:9: note: this is the location of the previous definition
99 | #define arch_spin_value_unlocked(l) ticket_spin_value_unlocked(l)
| ^~~~~~~~~~~~~~~~~~~~~~~~
./include/asm-generic/qspinlock.h:147:9: warning: "arch_spin_lock" redefined
147 | #define arch_spin_lock(l) queued_spin_lock(l)
| ^~~~~~~~~~~~~~
./include/asm-generic/ticket_spinlock.h:100:9: note: this is the location of the previous definition
100 | #define arch_spin_lock(l) ticket_spin_lock(l)
| ^~~~~~~~~~~~~~
./include/asm-generic/qspinlock.h:148:9: warning: "arch_spin_trylock" redefined
148 | #define arch_spin_trylock(l) queued_spin_trylock(l)
| ^~~~~~~~~~~~~~~~~
./include/asm-generic/ticket_spinlock.h:101:9: note: this is the location of the previous definition
101 | #define arch_spin_trylock(l) ticket_spin_trylock(l)
| ^~~~~~~~~~~~~~~~~
./include/asm-generic/qspinlock.h:149:9: warning: "arch_spin_unlock" redefined
149 | #define arch_spin_unlock(l) queued_spin_unlock(l)
| ^~~~~~~~~~~~~~~~
./include/asm-generic/ticket_spinlock.h:102:9: note: this is the location of the previous definition
102 | #define arch_spin_unlock(l) ticket_spin_unlock(l)
The following diff resolves them for me (please double check):
diff --git a/arch/riscv/include/asm/spinlock.h b/arch/riscv/include/asm/spinlock.h
index 4856d50006f28..2d59f56a9e2d1 100644
--- a/arch/riscv/include/asm/spinlock.h
+++ b/arch/riscv/include/asm/spinlock.h
@@ -30,7 +30,11 @@ SPINLOCK_BASE_DECLARE(value_unlocked, int, arch_spinlock_t)
#else
+#if defined(CONFIG_RISCV_TICKET_SPINLOCKS)
#include <asm/ticket_spinlock.h>
+#elif defined(CONFIG_RISCV_QUEUED_SPINLOCKS)
+#include <asm/qspinlock.h>
+#endif
#endif
Thanks for testing this config (when I did not...)!
I came up with something slightly different, but same fix in the end,
thanks!
+DEFINE_STATIC_KEY_TRUE(qspinlock_key);
+EXPORT_SYMBOL(qspinlock_key);
+
+static void __init riscv_spinlock_init(void)
+{
+ char *using_ext;
+
+ if (IS_ENABLED(CONFIG_RISCV_ISA_ZACAS) &&
+ IS_ENABLED(CONFIG_RISCV_ISA_ZABHA)) {
+ using_ext = "using Zabha";
+
+ asm goto(ALTERNATIVE("j %[no_zacas]", "nop", 0, RISCV_ISA_EXT_ZACAS, 1)
+ : : : : no_zacas);
+ asm goto(ALTERNATIVE("nop", "j %[qspinlock]", 0, RISCV_ISA_EXT_ZABHA, 1)
+ : : : : qspinlock);
+ }
+
+no_zacas:
+ using_ext = "using Ziccrse";
+ asm goto(ALTERNATIVE("nop", "j %[qspinlock]", 0,
+ RISCV_ISA_EXT_ZICCRSE, 1)
+ : : : : qspinlock);
+
+ static_branch_disable(&qspinlock_key);
+ pr_info("Ticket spinlock: enabled\n");
+
+ return;
+
+qspinlock:
+ pr_info("Queued spinlock %s: enabled\n", using_ext);
+}
+
Your commit message suggests that riscv_spinlock_init() doesn't need to
do anything if CONFIG_RISCV_COMBO_SPINLOCKS=n:
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index d7c31c9b8ead2..b2be1b0b700d2 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -244,6 +244,7 @@ static void __init parse_dtb(void)
#endif
}
+#if defined(CONFIG_RISCV_COMBO_SPINLOCKS)
DEFINE_STATIC_KEY_TRUE(qspinlock_key);
EXPORT_SYMBOL(qspinlock_key);
@@ -275,6 +276,11 @@ static void __init riscv_spinlock_init(void)
qspinlock:
pr_info("Queued spinlock %s: enabled\n", using_ext);
}
+#else
+static void __init riscv_spinlock_init(void)
+{
+}
+#endif
extern void __init init_rt_signal_env(void);
Makes sense? What am I missing?
Totally makes sense, I completely overlooked this when I added the
ticket/queued configs, thanks for taking the time to look into it.
That will be fixed in the next version.
Thanks again,
Alex
Andrea
_______________________________________________
linux-riscv mailing list
linux-riscv@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-riscv
