From 08c4376606eb2678164f52b97893941f29346721 Mon Sep 17 00:00:00 2001 From: Masayuki Ishikawa Date: Wed, 2 Sep 2020 16:18:25 +0900 Subject: [PATCH] arch, include, sched : Refactor ARCH_GLOBAL_IRQDISABLE related code Summary: - ARCH_GLOBAL_IRQDISABLE was initially introduced for LC823450 SMP - At that time, i.MX6 (quad Cortex-A9) did not use this config - However, this option is now used for all CPUs which support SMP - So it's good timing for refactoring the code Impact: - Should have no impact because the logic is the same for SMP Testing: - Tested with board: spresense:smp, spresense:wifi_smp - Tested with qemu: esp32-core:smp, maix-bit:smp, sabre-6quad:smp - Build only: lc823450-xgevk:rndis, sam4cmp-db:nsh Signed-off-by: Masayuki Ishikawa --- arch/Kconfig | 9 --------- arch/arm/Kconfig | 3 --- arch/arm/src/sam34/Kconfig | 1 - arch/risc-v/Kconfig | 1 - arch/xtensa/Kconfig | 1 - include/nuttx/irq.h | 6 ++---- sched/Kconfig | 1 - sched/irq/Make.defs | 2 -- sched/irq/irq_csection.c | 9 --------- sched/irq/irq_spinlock.c | 5 ++--- sched/sched/Make.defs | 4 ---- sched/sched/sched.h | 28 ++------------------------- sched/sched/sched_lock.c | 37 ------------------------------------ sched/sched/sched_thistask.c | 19 ------------------ 14 files changed, 6 insertions(+), 120 deletions(-) diff --git a/arch/Kconfig b/arch/Kconfig index 840453c4b7..79b7e51b57 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -277,15 +277,6 @@ config ARCH_HAVE_RTC_SUBSECONDS bool default n -config ARCH_GLOBAL_IRQDISABLE - bool - default n - ---help--- - Indicates that disabling interrupts on one CPU will either (1) disable - all interrupts globally on all CPUs, or (2) will disable interprocessor - interrupts as well so that no context switches can occur on the CPU - that disabled "local" interrupts. - config ARCH_HAVE_SYSCALL_HOOKS bool default n diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index cb53dd7abc..e20e8aab7a 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -89,7 +89,6 @@ config ARCH_CHIP_IMX6 select BOOT_RUNFROMSDRAM select ARCH_HAVE_ADDRENV select ARCH_NEED_ADDRENV_MAPPING - select ARCH_GLOBAL_IRQDISABLE ---help--- Freescale iMX.6 architectures (Cortex-A9) @@ -132,7 +131,6 @@ config ARCH_CHIP_LC823450 select ARCH_HAVE_HEAPCHECK select ARCH_HAVE_MULTICPU select ARCH_HAVE_I2CRESET - select ARCH_GLOBAL_IRQDISABLE ---help--- ON Semiconductor LC823450 architectures (ARM dual Cortex-M3) @@ -428,7 +426,6 @@ config ARCH_CHIP_CXD56XX select ARCH_HAVE_FPU select ARCH_HAVE_HEAPCHECK select ARCH_HAVE_MULTICPU - select ARCH_GLOBAL_IRQDISABLE select ARCH_HAVE_SDIO if MMCSD select ARCH_HAVE_MATH_H ---help--- diff --git a/arch/arm/src/sam34/Kconfig b/arch/arm/src/sam34/Kconfig index 5affaecd81..8ea5bb9f1c 100644 --- a/arch/arm/src/sam34/Kconfig +++ b/arch/arm/src/sam34/Kconfig @@ -247,7 +247,6 @@ config ARCH_CHIP_SAM4CM default n select ARCH_HAVE_MULTICPU select ARCH_HAVE_TICKLESS - select ARCH_GLOBAL_IRQDISABLE config ARCH_CHIP_SAM4L bool diff --git a/arch/risc-v/Kconfig b/arch/risc-v/Kconfig index 5b2367130c..ace4067f14 100644 --- a/arch/risc-v/Kconfig +++ b/arch/risc-v/Kconfig @@ -22,7 +22,6 @@ config ARCH_CHIP_K210 select ARCH_HAVE_MPU select ARCH_HAVE_TESTSET select ARCH_HAVE_MULTICPU - select ARCH_GLOBAL_IRQDISABLE ---help--- Kendryte K210 processor (RISC-V 64bit core with GC extensions) diff --git a/arch/xtensa/Kconfig b/arch/xtensa/Kconfig index 59f7c40148..7ee94b51b8 100644 --- a/arch/xtensa/Kconfig +++ b/arch/xtensa/Kconfig @@ -16,7 +16,6 @@ config ARCH_CHIP_ESP32 select ARCH_HAVE_MULTICPU select ARCH_HAVE_MODULE_TEXT select ARCH_TOOLCHAIN_GNU - select ARCH_GLOBAL_IRQDISABLE ---help--- The ESP32 is a dual-core system from Espressif with two Harvard architecture Xtensa LX6 CPUs. All embedded memory, external memory diff --git a/include/nuttx/irq.h b/include/nuttx/irq.h index 6e9e2a8dc3..07474bc154 100644 --- a/include/nuttx/irq.h +++ b/include/nuttx/irq.h @@ -260,8 +260,7 @@ void leave_critical_section(irqstate_t flags); * ****************************************************************************/ -#if defined(CONFIG_SMP) && defined(CONFIG_SPINLOCK_IRQ) && \ - defined(CONFIG_ARCH_GLOBAL_IRQDISABLE) +#if defined(CONFIG_SMP) && defined(CONFIG_SPINLOCK_IRQ) irqstate_t spin_lock_irqsave(void); #else # define spin_lock_irqsave() enter_critical_section() @@ -289,8 +288,7 @@ irqstate_t spin_lock_irqsave(void); * ****************************************************************************/ -#if defined(CONFIG_SMP) && defined(CONFIG_SPINLOCK_IRQ) && \ - defined(CONFIG_ARCH_GLOBAL_IRQDISABLE) +#if defined(CONFIG_SMP) && defined(CONFIG_SPINLOCK_IRQ) void spin_unlock_irqrestore(irqstate_t flags); #else # define spin_unlock_irqrestore(f) leave_critical_section(f) diff --git a/sched/Kconfig b/sched/Kconfig index dcb3911e70..c5fcc3c8bd 100644 --- a/sched/Kconfig +++ b/sched/Kconfig @@ -244,7 +244,6 @@ config SPINLOCK config SPINLOCK_IRQ bool "Support Spinlocks with IRQ control" default n - depends on ARCH_GLOBAL_IRQDISABLE ---help--- Enables support for spinlocks with IRQ control. This feature can be used to protect data in SMP mode. diff --git a/sched/irq/Make.defs b/sched/irq/Make.defs index 0f666a9959..f83e88cef2 100644 --- a/sched/irq/Make.defs +++ b/sched/irq/Make.defs @@ -37,11 +37,9 @@ CSRCS += irq_initialize.c irq_attach.c irq_dispatch.c irq_unexpectedisr.c ifeq ($(CONFIG_SMP),y) ifeq ($(CONFIG_SPINLOCK_IRQ),y) -ifeq ($(CONFIG_ARCH_GLOBAL_IRQDISABLE),y) CSRCS += irq_spinlock.c endif endif -endif ifeq ($(CONFIG_IRQCOUNT),y) CSRCS += irq_csection.c diff --git a/sched/irq/irq_csection.c b/sched/irq/irq_csection.c index 2658d5e09c..6993641fd2 100644 --- a/sched/irq/irq_csection.c +++ b/sched/irq/irq_csection.c @@ -646,15 +646,6 @@ bool irq_cpu_locked(int cpu) return false; } -#if defined(CONFIG_ARCH_HAVE_FETCHADD) && !defined(CONFIG_ARCH_GLOBAL_IRQDISABLE) - /* If the global lockcount has been incremented then simply return true */ - - if (g_global_lockcount > 0) - { - return true; - } -#endif - /* Test if g_cpu_irqlock is locked. We don't really need to use check * g_cpu_irqlock to do this, we can use the g_cpu_set. * diff --git a/sched/irq/irq_spinlock.c b/sched/irq/irq_spinlock.c index 914d8a3c67..0a9ade54da 100644 --- a/sched/irq/irq_spinlock.c +++ b/sched/irq/irq_spinlock.c @@ -30,8 +30,7 @@ #include "sched/sched.h" -#if defined(CONFIG_SMP) && defined (CONFIG_SPINLOCK_IRQ) && \ - defined(CONFIG_ARCH_GLOBAL_IRQDISABLE) +#if defined(CONFIG_SMP) && defined (CONFIG_SPINLOCK_IRQ) /**************************************************************************** * Public Data @@ -127,4 +126,4 @@ void spin_unlock_irqrestore(irqstate_t flags) up_irq_restore(flags); } -#endif /* CONFIG_SMP && CONFIG_SPINLOCK_IRQ && CONFIG_ARCH_GLOBAL_IRQDISABLE */ +#endif /* CONFIG_SMP && CONFIG_SPINLOCK_IRQ */ diff --git a/sched/sched/Make.defs b/sched/sched/Make.defs index e5a4d49734..9825ac39d0 100644 --- a/sched/sched/Make.defs +++ b/sched/sched/Make.defs @@ -86,11 +86,7 @@ endif ifeq ($(CONFIG_SMP),y) CSRCS += sched_tasklistlock.c -ifeq ($(CONFIG_ARCH_GLOBAL_IRQDISABLE),y) CSRCS += sched_thistask.c -else ifeq ($(CONFIG_ARCH_HAVE_FETCHADD),y) -CSRCS += sched_thistask.c -endif endif ifeq ($(CONFIG_SCHED_INSTRUMENTATION_BUFFER),y) diff --git a/sched/sched/sched.h b/sched/sched/sched.h index 8e5a821a43..084ba3c8ee 100644 --- a/sched/sched/sched.h +++ b/sched/sched/sched.h @@ -57,17 +57,12 @@ /* These are macros to access the current CPU and the current task on a CPU. * These macros are intended to support a future SMP implementation. - * NOTE: this_task() for SMP is implemented in sched_thistask.c if the CPU - * supports disabling of inter-processor interrupts or if it supports the - * atomic fetch add operation. + * NOTE: this_task() for SMP is implemented in sched_thistask.c */ #ifdef CONFIG_SMP # define current_task(cpu) ((FAR struct tcb_s *)g_assignedtasks[cpu].head) # define this_cpu() up_cpu_index() -# if !defined(CONFIG_ARCH_GLOBAL_IRQDISABLE) && !defined(CONFIG_ARCH_HAVE_FETCHADD) -# define this_task() (current_task(this_cpu())) -# endif #else # define current_task(cpu) ((FAR struct tcb_s *)g_readytorun.head) # define this_cpu() (0) @@ -353,14 +348,6 @@ extern volatile cpu_set_t g_cpu_lockset SP_SECTION; extern volatile spinlock_t g_cpu_tasklistlock SP_SECTION; -#if defined(CONFIG_ARCH_HAVE_FETCHADD) && !defined(CONFIG_ARCH_GLOBAL_IRQDISABLE) -/* This is part of the sched_lock() logic to handle atomic operations when - * locking the scheduler. - */ - -extern volatile int16_t g_global_lockcount; -#endif - #endif /* CONFIG_SMP */ /**************************************************************************** @@ -425,10 +412,7 @@ void nxsched_continue(FAR struct tcb_s *tcb); #endif #ifdef CONFIG_SMP -#if defined(CONFIG_ARCH_GLOBAL_IRQDISABLE) || \ - defined(CONFIG_ARCH_HAVE_FETCHADD) FAR struct tcb_s *this_task(void); -#endif int nxsched_select_cpu(cpu_set_t affinity); int nxsched_pause_cpu(FAR struct tcb_s *tcb); @@ -436,15 +420,7 @@ int nxsched_pause_cpu(FAR struct tcb_s *tcb); irqstate_t nxsched_lock_tasklist(void); void nxsched_unlock_tasklist(irqstate_t lock); -#if defined(CONFIG_ARCH_HAVE_FETCHADD) && \ - !defined(CONFIG_ARCH_GLOBAL_IRQDISABLE) -# define nxsched_islocked_global() \ - (spin_islocked(&g_cpu_schedlock) || g_global_lockcount > 0) -#else -# define nxsched_islocked_global() \ - spin_islocked(&g_cpu_schedlock) -#endif - +# define nxsched_islocked_global() spin_islocked(&g_cpu_schedlock) # define nxsched_islocked_tcb(tcb) nxsched_islocked_global() #else diff --git a/sched/sched/sched_lock.c b/sched/sched/sched_lock.c index e0dc6f5f65..33b3f71fc2 100644 --- a/sched/sched/sched_lock.c +++ b/sched/sched/sched_lock.c @@ -104,13 +104,6 @@ volatile spinlock_t g_cpu_schedlock SP_SECTION = SP_UNLOCKED; volatile spinlock_t g_cpu_locksetlock SP_SECTION; volatile cpu_set_t g_cpu_lockset SP_SECTION; -#if defined(CONFIG_ARCH_HAVE_FETCHADD) && !defined(CONFIG_ARCH_GLOBAL_IRQDISABLE) -/* This is part of the sched_lock() logic to handle atomic operations when - * locking the scheduler. - */ - -volatile int16_t g_global_lockcount; -#endif #endif /* CONFIG_SMP */ /**************************************************************************** @@ -140,16 +133,9 @@ volatile int16_t g_global_lockcount; int sched_lock(void) { FAR struct tcb_s *rtcb; -#if defined(CONFIG_ARCH_GLOBAL_IRQDISABLE) irqstate_t flags; -#endif int cpu; - /* The following operation is non-atomic unless - * CONFIG_ARCH_GLOBAL_IRQDISABLE or CONFIG_ARCH_HAVE_FETCHADD is defined. - */ - -#if defined(CONFIG_ARCH_GLOBAL_IRQDISABLE) /* If the CPU supports suppression of interprocessor interrupts, then * simple disabling interrupts will provide sufficient protection for * the following operation. @@ -157,19 +143,6 @@ int sched_lock(void) flags = up_irq_save(); -#elif defined(CONFIG_ARCH_HAVE_FETCHADD) - /* If the CPU supports an atomic fetch add operation, then we can use the - * global lockcount to assure that the following operation is atomic. - */ - - DEBUGASSERT((uint16_t)g_global_lockcount < INT16_MAX); /* Not atomic! */ - up_fetchadd16(&g_global_lockcount, 1); -#endif - - /* This operation is safe if CONFIG_ARCH_HAVE_FETCHADD is defined. NOTE - * we cannot use this_task() because it calls sched_lock(). - */ - cpu = this_cpu(); rtcb = current_task(cpu); @@ -180,12 +153,7 @@ int sched_lock(void) if (rtcb == NULL || up_interrupt_context()) { -#if defined(CONFIG_ARCH_GLOBAL_IRQDISABLE) up_irq_restore(flags); -#elif defined(CONFIG_ARCH_HAVE_FETCHADD) - DEBUGASSERT(g_global_lockcount > 0); - up_fetchsub16(&g_global_lockcount, 1); -#endif } else { @@ -228,12 +196,7 @@ int sched_lock(void) rtcb->lockcount++; -#if defined(CONFIG_ARCH_GLOBAL_IRQDISABLE) up_irq_restore(flags); -#elif defined(CONFIG_ARCH_HAVE_FETCHADD) - DEBUGASSERT(g_global_lockcount > 0); - up_fetchsub16(&g_global_lockcount, 1); -#endif #if defined(CONFIG_SCHED_INSTRUMENTATION_PREEMPTION) || \ defined(CONFIG_SCHED_CRITMONITOR) diff --git a/sched/sched/sched_thistask.c b/sched/sched/sched_thistask.c index 1b646de044..57a146757c 100644 --- a/sched/sched/sched_thistask.c +++ b/sched/sched/sched_thistask.c @@ -54,7 +54,6 @@ FAR struct tcb_s *this_task(void) { FAR struct tcb_s *tcb; -#if defined(CONFIG_ARCH_GLOBAL_IRQDISABLE) irqstate_t flags; /* If the CPU supports suppression of interprocessor interrupts, then @@ -63,20 +62,6 @@ FAR struct tcb_s *this_task(void) */ flags = up_irq_save(); -#elif defined(CONFIG_ARCH_HAVE_FETCHADD) - /* Global locking is supported and, hence, sched_lock() will provide the - * necessary protection. - */ - - sched_lock(); -#else - /* REVISIT: Otherwise, there is no protection available. sched_lock() and - * enter_critical section are not viable options here (because both depend - * on this_task()). - */ - -# warning "Missing critical section" -#endif /* Obtain the TCB which is currently running on this CPU */ @@ -84,11 +69,7 @@ FAR struct tcb_s *this_task(void) /* Enable local interrupts */ -#if defined(CONFIG_ARCH_GLOBAL_IRQDISABLE) up_irq_restore(flags); -#elif defined(CONFIG_ARCH_HAVE_FETCHADD) - sched_unlock(); -#endif return tcb; }