From 17e968ad0260ea5dbac492b170d10dfa037f0836 Mon Sep 17 00:00:00 2001 From: Indan Zupancic Date: Tue, 14 Jun 2022 13:07:35 +0200 Subject: [PATCH] Remove try_arch_atomic_exchange_rlx This reverts commit 3d2ae69f9cb184c34b6b56d1365e57693e832db4: "ARM/SMP: Re-implement atomic exchange taking sel4 IPI into account" Also removed riscv's try_arch_atomic_exchange_rlx(). This was added because CAS can take very long to finish on ARM if the exclusive reservation granule (ERG) is large for the platform, as any writes on other cores within up to 2Kb could make the CAS fail. The correct fix is to add padding around the global lock equal to ERG to make CAS fast, which would make the extra IPI check redundant. Signed-off-by: Indan Zupancic --- include/arch/arm/arch/model/smp.h | 20 ---------------- include/arch/riscv/arch/model/smp.h | 6 ----- include/arch/x86/arch/model/smp.h | 6 ----- include/smp/lock.h | 37 ++--------------------------- 4 files changed, 2 insertions(+), 67 deletions(-) diff --git a/include/arch/arm/arch/model/smp.h b/include/arch/arm/arch/model/smp.h index f9abe72a9..292a2300e 100644 --- a/include/arch/arm/arch/model/smp.h +++ b/include/arch/arm/arch/model/smp.h @@ -15,25 +15,5 @@ static inline cpu_id_t cpuIndexToID(word_t index) { return BIT(index); } - -static inline bool_t try_arch_atomic_exchange_rlx(void *ptr, void *new_val, void **prev) -{ - uint32_t atomic_status; - void *temp; - - asm volatile( - LD_EX "%[prev_output], [%[ptr_val]] \n\t" /* ret = *ptr */ - ST_EX "%" OP_WIDTH "[atomic_var], %[new_val] , [%[ptr_val]] \n\t" /* *ptr = new */ - : [atomic_var] "=&r"(atomic_status), [prev_output]"=&r"(temp) /* output */ - : [ptr_val] "r"(ptr), [new_val] "r"(new_val) /* input */ - : - ); - - *prev = temp; - - /* On ARM if an atomic operation succeeds, it returns 0 */ - return (atomic_status == 0); -} - #endif /* ENABLE_SMP_SUPPORT */ diff --git a/include/arch/riscv/arch/model/smp.h b/include/arch/riscv/arch/model/smp.h index 996995055..9241d91fa 100644 --- a/include/arch/riscv/arch/model/smp.h +++ b/include/arch/riscv/arch/model/smp.h @@ -43,12 +43,6 @@ static inline void add_hart_to_core_map(word_t hart_id, word_t core_id) coreMap.map[core_id] = hart_id; } -static inline bool_t try_arch_atomic_exchange_rlx(void *ptr, void *new_val, void **prev) -{ - *prev = __atomic_exchange_n((void **)ptr, new_val, __ATOMIC_RELAXED); - return true; -} - static inline CONST cpu_id_t getCurrentCPUIndex(void) { word_t sp; diff --git a/include/arch/x86/arch/model/smp.h b/include/arch/x86/arch/model/smp.h index f73bcb8a9..aac276418 100644 --- a/include/arch/x86/arch/model/smp.h +++ b/include/arch/x86/arch/model/smp.h @@ -36,10 +36,4 @@ static inline PURE word_t getCurrentCPUID(void) return cpu_mapping.index_to_cpu_id[getCurrentCPUIndex()]; } -static inline bool_t try_arch_atomic_exchange_rlx(void *ptr, void *new_val, void **prev) -{ - *prev = __atomic_exchange_n((void **) ptr, new_val, __ATOMIC_RELAXED); - return true; -} - #endif /* ENABLE_SMP_SUPPORT */ diff --git a/include/smp/lock.h b/include/smp/lock.h index 9f9717e3d..7e49adea6 100644 --- a/include/smp/lock.h +++ b/include/smp/lock.h @@ -57,46 +57,13 @@ static inline bool_t FORCE_INLINE clh_is_ipi_pending(word_t cpu) return big_kernel_lock.node_owners[cpu].ipi == 1; } -static inline void *sel4_atomic_exchange(void *ptr, bool_t - irqPath, word_t cpu, int memorder) -{ - clh_qnode_t *prev; - - if (memorder == __ATOMIC_RELEASE || memorder == __ATOMIC_ACQ_REL) { - __atomic_thread_fence(__ATOMIC_RELEASE); - } else if (memorder == __ATOMIC_SEQ_CST) { - __atomic_thread_fence(__ATOMIC_SEQ_CST); - } - - while (!try_arch_atomic_exchange_rlx(&big_kernel_lock.head, - (void *) big_kernel_lock.node_owners[cpu].node, - (void **) &prev)) { - if (clh_is_ipi_pending(cpu)) { - /* we only handle irq_remote_call_ipi here as other type of IPIs - * are async and could be delayed. 'handleIPI' may not return - * based on value of the 'irqPath'. */ - handleIPI(CORE_IRQ_TO_IRQT(cpu, irq_remote_call_ipi), irqPath); - } - - arch_pause(); - } - - if (memorder == __ATOMIC_ACQUIRE || memorder == __ATOMIC_ACQ_REL) { - __atomic_thread_fence(__ATOMIC_ACQUIRE); - } else if (memorder == __ATOMIC_SEQ_CST) { - __atomic_thread_fence(__ATOMIC_SEQ_CST); - } - - return prev; -} - static inline void FORCE_INLINE clh_lock_acquire(word_t cpu, bool_t irqPath) { clh_qnode_t *prev; big_kernel_lock.node_owners[cpu].node->value = CLHState_Pending; - prev = sel4_atomic_exchange(&big_kernel_lock.head, irqPath, cpu, __ATOMIC_ACQ_REL); - + prev = __atomic_exchange_n(&big_kernel_lock.head, + big_kernel_lock.node_owners[cpu].node, __ATOMIC_ACQ_REL); big_kernel_lock.node_owners[cpu].next = prev; /* We do not have an __atomic_thread_fence here as this is already handled by the