From 514e77b75e158c28a40e1e165bc98d950f26a03e Mon Sep 17 00:00:00 2001 From: zhangyuan21 Date: Mon, 17 Apr 2023 19:57:21 +0800 Subject: [PATCH] semaphore: Optimize priority inheritance with only one holder This PR is a modification that optimizes priority inheritance for only one holder. After the above modifications are completed, the mutex lock->unlock process that supports priority inheritance can be optimized by 200 cycles. Before modify: 2000 cycle After modify: 1742 cycle Signed-off-by: zhangyuan21 --- .../sim/configs/ostest_oneholder/defconfig | 39 ++++++ include/nuttx/semaphore.h | 3 +- include/semaphore.h | 5 +- libs/libc/semaphore/sem_init.c | 3 +- sched/semaphore/sem_holder.c | 119 ++++++++++-------- tools/ci/testlist/sim-02.dat | 1 + 6 files changed, 110 insertions(+), 60 deletions(-) create mode 100644 boards/sim/sim/sim/configs/ostest_oneholder/defconfig diff --git a/boards/sim/sim/sim/configs/ostest_oneholder/defconfig b/boards/sim/sim/sim/configs/ostest_oneholder/defconfig new file mode 100644 index 0000000000..59b02e5647 --- /dev/null +++ b/boards/sim/sim/sim/configs/ostest_oneholder/defconfig @@ -0,0 +1,39 @@ +# +# This file is autogenerated: PLEASE DO NOT EDIT IT. +# +# You can use "make menuconfig" to make any modifications to the installed .config file. +# You can then do "make savedefconfig" to generate a new defconfig file that includes your +# modifications. +# +CONFIG_ARCH="sim" +CONFIG_ARCH_BOARD="sim" +CONFIG_ARCH_BOARD_SIM=y +CONFIG_ARCH_CHIP="sim" +CONFIG_ARCH_SIM=y +CONFIG_BOARDCTL=y +CONFIG_BOARDCTL_POWEROFF=y +CONFIG_BOARD_LOOPSPERMSEC=100 +CONFIG_CANCELLATION_POINTS=y +CONFIG_DEBUG_ASSERTIONS=y +CONFIG_DEBUG_FEATURES=y +CONFIG_DEBUG_SYMBOLS=y +CONFIG_FS_NAMED_SEMAPHORES=y +CONFIG_IDLETHREAD_STACKSIZE=4096 +CONFIG_INIT_ENTRYPOINT="ostest_main" +CONFIG_MM_KASAN=y +CONFIG_MM_UBSAN=y +CONFIG_MM_UBSAN_TRAP_ON_ERROR=y +CONFIG_PRIORITY_INHERITANCE=y +CONFIG_PTHREAD_CLEANUP_STACKSIZE=3 +CONFIG_PTHREAD_MUTEX_TYPES=y +CONFIG_RAM_START=0x00000000 +CONFIG_SCHED_HAVE_PARENT=y +CONFIG_SCHED_WAITPID=y +CONFIG_SEM_PREALLOCHOLDERS=0 +CONFIG_SIM_WALLTIME_SIGNAL=y +CONFIG_START_DAY=27 +CONFIG_START_MONTH=2 +CONFIG_START_YEAR=2007 +CONFIG_TESTING_OSTEST=y +CONFIG_TESTING_OSTEST_LOOPS=5 +CONFIG_TESTING_OSTEST_POWEROFF=y diff --git a/include/nuttx/semaphore.h b/include/nuttx/semaphore.h index eebb3cc833..0b1becf4ea 100644 --- a/include/nuttx/semaphore.h +++ b/include/nuttx/semaphore.h @@ -48,8 +48,7 @@ /* semcount, flags, waitlist, holder[2] */ # define NXSEM_INITIALIZER(c, f) \ - {(c), (f), SEM_WAITLIST_INITIALIZER, \ - {SEMHOLDER_INITIALIZER, SEMHOLDER_INITIALIZER}} + {(c), (f), SEM_WAITLIST_INITIALIZER, SEMHOLDER_INITIALIZER} # endif #else /* CONFIG_PRIORITY_INHERITANCE */ /* semcount, flags, waitlist */ diff --git a/include/semaphore.h b/include/semaphore.h index 93a5a46120..eb4b1fcc37 100644 --- a/include/semaphore.h +++ b/include/semaphore.h @@ -113,7 +113,7 @@ struct sem_s # if CONFIG_SEM_PREALLOCHOLDERS > 0 FAR struct semholder_s *hhead; /* List of holders of semaphore counts */ # else - struct semholder_s holder[2]; /* Slot for old and new holder */ + struct semholder_s holder; /* Slot for old and new holder */ # endif #endif }; @@ -132,8 +132,7 @@ typedef struct sem_s sem_t; /* semcount, flags, waitlist, holder[2] */ # define SEM_INITIALIZER(c) \ - {(c), 0, SEM_WAITLIST_INITIALIZER, \ - {SEMHOLDER_INITIALIZER, SEMHOLDER_INITIALIZER}} + {(c), 0, SEM_WAITLIST_INITIALIZER, SEMHOLDER_INITIALIZER} # endif #else /* semcount, flags, waitlist */ diff --git a/libs/libc/semaphore/sem_init.c b/libs/libc/semaphore/sem_init.c index 46400b9377..e74c92b842 100644 --- a/libs/libc/semaphore/sem_init.c +++ b/libs/libc/semaphore/sem_init.c @@ -82,8 +82,7 @@ int nxsem_init(FAR sem_t *sem, int pshared, unsigned int value) # if CONFIG_SEM_PREALLOCHOLDERS > 0 sem->hhead = NULL; # else - INITIALIZE_SEMHOLDER(&sem->holder[0]); - INITIALIZE_SEMHOLDER(&sem->holder[1]); + INITIALIZE_SEMHOLDER(&sem->holder); # endif #endif return OK; diff --git a/sched/semaphore/sem_holder.c b/sched/semaphore/sem_holder.c index 2866bf5d5f..157e9e86d1 100644 --- a/sched/semaphore/sem_holder.c +++ b/sched/semaphore/sem_holder.c @@ -89,13 +89,9 @@ nxsem_allocholder(FAR sem_t *sem, FAR struct tcb_s *htcb) sem->hhead = pholder; } #else - if (sem->holder[0].htcb == NULL) + if (sem->holder.htcb == NULL) { - pholder = &sem->holder[0]; - } - else if (sem->holder[1].htcb == NULL) - { - pholder = &sem->holder[1]; + pholder = &sem->holder; } #endif else @@ -145,19 +141,15 @@ nxsem_findholder(FAR sem_t *sem, FAR struct tcb_s *htcb) } } #else - int i; + /* We have one hard-allocated holder structures in sem_t */ - /* We have two hard-allocated holder structures in sem_t */ + pholder = &sem->holder; - for (i = 0; i < 2; i++) + if (pholder->htcb == htcb) { - pholder = &sem->holder[i]; - if (pholder->htcb == htcb) - { - /* Got it! */ + /* Got it! */ - return pholder; - } + return pholder; } #endif @@ -236,6 +228,7 @@ static inline void nxsem_freeholder(FAR sem_t *sem, * Name: nxsem_freecount0holder ****************************************************************************/ +#if CONFIG_SEM_PREALLOCHOLDERS > 0 static int nxsem_freecount0holder(FAR struct semholder_s *pholder, FAR sem_t *sem, FAR void *arg) { @@ -251,6 +244,7 @@ static int nxsem_freecount0holder(FAR struct semholder_s *pholder, return 0; } +#endif /**************************************************************************** * Name: nxsem_foreachholder @@ -278,22 +272,17 @@ static int nxsem_foreachholder(FAR sem_t *sem, holderhandler_t handler, ret = handler(pholder, sem, arg); } #else - int i; + /* We have one hard-allocated holder structures in sem_t */ - /* We have two hard-allocated holder structures in sem_t */ + pholder = &sem->holder; - for (i = 0; i < 2 && ret == 0; i++) + /* The hard-allocated containers may hold a NULL holder */ + + if (pholder->htcb != NULL) { - pholder = &sem->holder[i]; + /* Call the handler */ - /* The hard-allocated containers may hold a NULL holder */ - - if (pholder->htcb != NULL) - { - /* Call the handler */ - - ret = handler(pholder, sem, arg); - } + ret = handler(pholder, sem, arg); } #endif @@ -388,24 +377,13 @@ static int nxsem_dumpholder(FAR struct semholder_s *pholder, FAR sem_t *sem, #endif /**************************************************************************** - * Name: nxsem_restoreholderprio + * Name: nxsem_restore_priority ****************************************************************************/ -static int nxsem_restoreholderprio(FAR struct semholder_s *pholder, - FAR sem_t *sem, FAR void *arg) +static void nxsem_restore_priority(FAR struct tcb_s *htcb) { - FAR struct tcb_s *htcb = pholder->htcb; int hpriority; - /* Release the holder if all counts have been given up - * before reprioritizing causes a context switch. - */ - - if (pholder->counts <= 0) - { - nxsem_freeholder(sem, pholder); - } - /* We attempt to restore thread priority to its base priority. If * there is any thread with the higher priority waiting for the * semaphore held by htcb then this value will be overwritten. @@ -420,6 +398,8 @@ static int nxsem_restoreholderprio(FAR struct semholder_s *pholder, if (htcb->sched_priority != hpriority) { + FAR struct semholder_s *pholder; + /* Try to find the highest priority across all the threads that are * waiting for any semaphore held by htcb. */ @@ -443,10 +423,33 @@ static int nxsem_restoreholderprio(FAR struct semholder_s *pholder, nxsched_set_priority(htcb, hpriority); } +} + +/**************************************************************************** + * Name: nxsem_restoreholderprio + ****************************************************************************/ + +static int nxsem_restoreholderprio(FAR struct semholder_s *pholder, + FAR sem_t *sem, FAR void *arg) +{ + FAR struct tcb_s *htcb = pholder->htcb; + + /* Release the holder if all counts have been given up + * before reprioritizing causes a context switch. + */ + + if (pholder->counts <= 0) + { + nxsem_freeholder(sem, pholder); + } + + nxsem_restore_priority(htcb); return 0; } +#if CONFIG_SEM_PREALLOCHOLDERS > 0 + /**************************************************************************** * Name: nxsem_restoreholderprio_others * @@ -491,6 +494,8 @@ static int nxsem_restoreholderprio_self(FAR struct semholder_s *pholder, return 0; } +#endif + /**************************************************************************** * Public Functions ****************************************************************************/ @@ -576,7 +581,7 @@ void nxsem_destroyholder(FAR sem_t *sem) #else /* There may be an issue if there are multiple holders of the semaphore. */ - DEBUGASSERT(sem->holder[0].htcb == NULL || sem->holder[1].htcb == NULL); + DEBUGASSERT(sem->holder.htcb == NULL); #endif @@ -674,7 +679,11 @@ void nxsem_boost_priority(FAR sem_t *sem) * count. */ +#if CONFIG_SEM_PREALLOCHOLDERS > 0 nxsem_foreachholder(sem, nxsem_boostholderprio, rtcb); +#else + nxsem_boostholderprio(&sem->holder, sem, rtcb); +#endif } /**************************************************************************** @@ -715,19 +724,7 @@ void nxsem_release_holder(FAR sem_t *sem) #if CONFIG_SEM_PREALLOCHOLDERS > 0 for (pholder = sem->hhead; pholder != NULL; pholder = pholder->flink) -#else - /* We have two hard-allocated holder structures in sem_t */ - - for (pholder = &sem->holder[0]; pholder <= &sem->holder[1]; pholder++) -#endif { -#if CONFIG_SEM_PREALLOCHOLDERS == 0 - if (pholder->htcb == NULL) - { - continue; - } -#endif - DEBUGASSERT(pholder->counts > 0); if (pholder->htcb == rtcb) @@ -744,6 +741,11 @@ void nxsem_release_holder(FAR sem_t *sem) /* The current task is not a holder */ DEBUGPANIC(); +#else + pholder = &sem->holder; + DEBUGASSERT(pholder->htcb == rtcb); + nxsem_freeholder(sem, pholder); +#endif } } @@ -800,6 +802,7 @@ void nxsem_restore_baseprio(FAR struct tcb_s *stcb, FAR sem_t *sem) if (stcb != NULL) { +#if CONFIG_SEM_PREALLOCHOLDERS > 0 /* The currently executed thread should be the lower priority * thread that just posted the count and caused this action. * However, we cannot drop the priority of the currently running @@ -814,9 +817,18 @@ void nxsem_restore_baseprio(FAR struct tcb_s *stcb, FAR sem_t *sem) /* Now, find an reprioritize only the ready to run task */ nxsem_foreachholder(sem, nxsem_restoreholderprio_self, stcb); +#else + /* New owner is already the highest priority since the wait queue + * is priority-based, no need to adjust its priority, only restore + * the older owner when posted the count. + */ + + nxsem_restore_priority(this_task()); +#endif } else { +#if CONFIG_SEM_PREALLOCHOLDERS > 0 /* Remove the holder from the list if it's counts is zero. */ nxsem_foreachholder(sem, nxsem_freecount0holder, NULL); @@ -824,6 +836,7 @@ void nxsem_restore_baseprio(FAR struct tcb_s *stcb, FAR sem_t *sem) /* If there are no tasks waiting for available counts, then all holders * should be at their base priority. */ +#endif #ifdef CONFIG_DEBUG_ASSERTIONS nxsem_foreachholder(sem, nxsem_verifyholder, NULL); diff --git a/tools/ci/testlist/sim-02.dat b/tools/ci/testlist/sim-02.dat index c47ec022f9..1e07ebdf40 100644 --- a/tools/ci/testlist/sim-02.dat +++ b/tools/ci/testlist/sim-02.dat @@ -8,6 +8,7 @@ # clang doesn't -fsanitize=kernel-address -Darwin,sim:ostest +-Darwin,sim:ostest_oneholder # macOS doesn't support --wrap flag # ld: unknown option: --wrap