spinlock+sched_lock #14578

hujun260 · 2024-10-31T09:02:03Z

Summary

1 Accelerated the implementation of sched_lock, remove enter_critical_section in sched_lock and
only enter_critical_section when task scheduling is required.
2 we add sched_lock_wo_note/sched_unlock_wo_note and it does not perform instrumentation logic
3 We aim to replace big locks with smaller ones. So we will use spin_lock_irqsave extensively to
replace enter_critical_section in the subsequent process. We imitate the implementation of Linux
by adding sched_lock to spin_lock_irqsave in order to address scenarios where sem_post occurs
within spin_lock_irqsave, which can lead to spinlock failures and deadlocks.

Impact

spinlock and sched_lock

Testing

Build Host:

OS: Ubuntu 20.04
CPU: x86_64
Compiler: GCC 9.4.0

Configuring NuttX and compile:
$ ./tools/configure.sh -l qemu-armv8a:nsh_smp
$ make
Running with qemu
$ qemu-system-aarch64 -cpu cortex-a53 -smp 4 -nographic
-machine virt,virtualization=on,gic-version=3
-net none -chardev stdio,id=con,mux=on -serial chardev:con
-mon chardev=con,mode=readline -kernel ./nuttx

arch/xtensa/src/esp32/esp32_ble_adapter.c

include/nuttx/spinlock.h

sched/sched/sched.h

include/sched.h

sched/sched/sched.h

sched/sched/sched_lock.c

sched/sched/sched_unlock.c

include/nuttx/spinlock.h

xiaoxiang781216 · 2024-11-18T07:25:38Z

@patacongo please review this patch which fix the long issue about sched lock.

sched/sched/sched_lock.c

xiaoxiang781216 · 2024-11-21T16:11:08Z

please fix conflict

…ed_[un]lock reason: 1 Accelerated the implementation of sched_lock, remove enter_critical_section in sched_lock and only enter_critical_section when task scheduling is required. 2 we add sched_lock_wo_note/sched_unlock_wo_note and it does not perform instrumentation logic Signed-off-by: hujun5 <[email protected]>

reason: allow the spin_lock_irqsave_wo_note to respond interrupts during the test set loop and improve RT performance Signed-off-by: hujun5 <[email protected]>

reason: We aim to replace big locks with smaller ones. So we will use spin_lock_irqsave extensively to replace enter_critical_section in the subsequent process. We imitate the implementation of Linux by adding sched_lock to spin_lock_irqsave in order to address scenarios where sem_post occurs within spin_lock_irqsave, which can lead to spinlock failures and deadlocks. Signed-off-by: hujun5 <[email protected]>

hujun260 · 2024-11-22T00:55:49Z

please fix conflict

done

xiaoxiang781216 · 2024-11-22T00:52:18Z

sched/irq/irq_csection.c

@@ -232,7 +232,7 @@ irqstate_t enter_critical_section(void)

          DEBUGASSERT((g_cpu_irqset & (1 << cpu)) == 0);

-          spin_lock(&g_cpu_irqlock);
+          spin_lock_wo_note(&g_cpu_irqlock);


do we need change leave_critical_section too

leave_critical_section use cpu_irqlock_clear which alse use spin_unlock_wo_note

xiaoxiang781216 · 2024-11-22T00:52:58Z

drivers/note/noterpmsg_driver.c

@@ -85,6 +86,8 @@ struct noterpmsg_driver_s g_noterpmsg_driver =
  },
 };

+volatile spinlock_t g_note_driver_lock = SP_UNLOCKED;


remove volatile and add static

xiaoxiang781216 · 2024-11-22T00:53:54Z

drivers/note/noterpmsg_driver.c

@@ -178,15 +181,15 @@ static bool noterpmsg_transfer(FAR struct noterpmsg_driver_s *drv,
 static void noterpmsg_work(FAR void *priv)
 {
  FAR struct noterpmsg_driver_s *drv = priv;
-  irqstate_t flags = enter_critical_section();
+  irqstate_t flags = spin_lock_irqsave(&g_note_driver_lock);


need call wo_note version

xiaoxiang781216 · 2024-11-22T00:54:03Z

drivers/note/noterpmsg_driver.c


  if (!noterpmsg_transfer(drv, false))
    {
      work_queue(HPWORK, &drv->work, noterpmsg_work, drv,
                 NOTE_RPMSG_WORK_DELAY);
    }

-  leave_critical_section(flags);
+  spin_unlock_irqrestore(&g_note_driver_lock, flags);


xiaoxiang781216 · 2024-11-22T00:57:49Z

include/nuttx/spinlock.h

+
+#ifdef CONFIG_SPINLOCK
+#  define spin_trylock_irqsave_wo_note(l, f) \
+({ \


could.we.avoid.use gcc express statement extension

xiaoxiang781216 · 2024-11-22T00:59:10Z

include/nuttx/spinlock.h

@@ -528,22 +567,29 @@ static inline_function
 irqstate_t spin_lock_irqsave_wo_note(FAR volatile spinlock_t *lock)
 {
  irqstate_t ret;
-  ret = up_irq_save();
+  int me;

  if (NULL == lock)


let's remove NULL support in the future

…id recursive calls. Signed-off-by: hujun5 <[email protected]>

reason: spin_lock_wo_note/spin_unlock_wo_note should be called in matching pairs. Signed-off-by: hujun5 <[email protected]>

reason: 1: spin_lock_init and spin_initialize have similar functionalities. 2: spin_lock and spin_unlock should be called in matching pairs. Signed-off-by: hujun5 <[email protected]>

reason: we.avoid.use gcc express statement extension in spinlock, to enhance compatibility Signed-off-by: hujun5 <[email protected]>

github-actions bot added Area: OS Components OS Components issues Size: L The size of the change in this PR is large labels Oct 31, 2024

xiaoxiang781216 requested a review from anchao October 31, 2024 09:14

hujun260 force-pushed the apache_3 branch from 61fdaf6 to e5d9392 Compare October 31, 2024 10:58

github-actions bot added the Arch: xtensa Issues related to the Xtensa architecture label Oct 31, 2024

hujun260 force-pushed the apache_3 branch from e5d9392 to 2d48282 Compare October 31, 2024 11:22

acassis reviewed Oct 31, 2024

View reviewed changes

arch/xtensa/src/esp32/esp32_ble_adapter.c Show resolved Hide resolved

anchao requested changes Nov 1, 2024

View reviewed changes

include/nuttx/spinlock.h Outdated Show resolved Hide resolved

hujun260 force-pushed the apache_3 branch 7 times, most recently from 157736b to 175819e Compare November 4, 2024 11:26

hujun260 requested a review from xiaoxiang781216 November 6, 2024 11:32

hujun260 mentioned this pull request Nov 6, 2024

wqueue remove enter_critical_section #14623

Open

xiaoxiang781216 requested a review from anchao November 15, 2024 06:26

xiaoxiang781216 reviewed Nov 18, 2024

View reviewed changes

xiaoxiang781216 linked an issue Nov 18, 2024 that may be closed by this pull request

Thread holding spinlock can be blocked. #9531

Open

xiaoxiang781216 requested a review from patacongo November 18, 2024 07:25

xiaoxiang781216 linked an issue Nov 18, 2024 that may be closed by this pull request

Thread can be unexpectedly suspended within a critical section. #1138

Open

anchao reviewed Nov 19, 2024

View reviewed changes

sched/sched/sched_lock.c Outdated Show resolved Hide resolved

sched/sched/sched_lock.c Outdated Show resolved Hide resolved

hujun260 force-pushed the apache_3 branch from 175819e to 815cf96 Compare November 19, 2024 03:39

github-actions bot added the Area: Drivers Drivers issues label Nov 19, 2024

anchao reviewed Nov 19, 2024

View reviewed changes

sched/sched/sched_lock.c Outdated Show resolved Hide resolved

hujun260 force-pushed the apache_3 branch 2 times, most recently from 87f9b88 to cde599d Compare November 20, 2024 01:38

github-actions bot added the Arch: risc-v Issues related to the RISC-V (32-bit or 64-bit) architecture label Nov 20, 2024

github-actions bot added the Area: Memory Management Memory Management issues label Nov 20, 2024

hujun260 force-pushed the apache_3 branch from cde599d to 5d9e540 Compare November 20, 2024 12:27

hujun260 added 3 commits November 22, 2024 08:36

spinlock: improve spin_lock_irqsave_wo_note realtime performance

972a3c3

reason: allow the spin_lock_irqsave_wo_note to respond interrupts during the test set loop and improve RT performance Signed-off-by: hujun5 <[email protected]>

hujun260 force-pushed the apache_3 branch from 5d9e540 to ca8af44 Compare November 22, 2024 00:37

xiaoxiang781216 reviewed Nov 22, 2024

View reviewed changes

hujun260 added 4 commits November 22, 2024 10:11

note: Remove enter_critical_section from the sched_note module to avo…

9a946d0

…id recursive calls. Signed-off-by: hujun5 <[email protected]>

use spin_lock_wo_note replace spin_lock in csection

304d8a0

reason: spin_lock_wo_note/spin_unlock_wo_note should be called in matching pairs. Signed-off-by: hujun5 <[email protected]>

spinlock: use spin_lock_init replace spin_initialize

157a3ab

reason: 1: spin_lock_init and spin_initialize have similar functionalities. 2: spin_lock and spin_unlock should be called in matching pairs. Signed-off-by: hujun5 <[email protected]>

spinlock: use inline replase macro

21e349d

reason: we.avoid.use gcc express statement extension in spinlock, to enhance compatibility Signed-off-by: hujun5 <[email protected]>

hujun260 force-pushed the apache_3 branch from ca8af44 to 21e349d Compare November 22, 2024 02:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

spinlock+sched_lock #14578

spinlock+sched_lock #14578

hujun260 commented Oct 31, 2024

xiaoxiang781216 commented Nov 18, 2024

xiaoxiang781216 commented Nov 21, 2024

hujun260 commented Nov 22, 2024

xiaoxiang781216 Nov 22, 2024

hujun260 Nov 22, 2024

xiaoxiang781216 Nov 22, 2024

hujun260 Nov 22, 2024

xiaoxiang781216 Nov 22, 2024

hujun260 Nov 22, 2024

xiaoxiang781216 Nov 22, 2024

hujun260 Nov 22, 2024

xiaoxiang781216 Nov 22, 2024

hujun260 Nov 22, 2024

xiaoxiang781216 Nov 22, 2024

hujun260 Nov 22, 2024

spinlock+sched_lock #14578

Are you sure you want to change the base?

spinlock+sched_lock #14578

Conversation

hujun260 commented Oct 31, 2024

Summary

Impact

Testing

xiaoxiang781216 commented Nov 18, 2024

xiaoxiang781216 commented Nov 21, 2024

hujun260 commented Nov 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment