Fix-Point commented on PR #21342:
URL: https://github.com/apache/nuttx/pull/21342#issuecomment-6648328749

   > Measured on a quiet host: qemu-armv8a:nsh_smp, 4 CPUs, HVF on an Apple 
core, ostest spinlock test, median of 3 runs, M op/s. This replaces my earlier 
numbers.
   > 
   > threads, lock      master  this PR no WFE/SEV in spinlock.h        SEV 
without DSB
   > 1, spinlock        155.9   11.3    155.2   115.8
   > 1, rspinlock       153.6   11.8    155.7   114.2
   > 2, spinlock        48.8    8.9     44.3    40.4
   > 2, rspinlock       49.5    9.0     41.1    30.9
   > 3, spinlock        30.6    4.8     29.3    21.9
   > 3, rspinlock       28.7    5.1     25.1    18.1
   > 4, spinlock        26.9    3.5     24.6    18.5
   > 4, rspinlock       21.7    4.0     22.0    13.5
   > The last column is for measurement only; it is not correct code. Most of 
the cost is the DSB before SEV in every unlock. This is one platform and a 
lock/unlock loop.
   > 
   > Fix: on arm64, wait with the exclusive monitor armed, the exception that 
[93e055d](https://github.com/apache/nuttx/commit/93e055d96d8506ceeb4ea878457e600cc84e4e0e)
 describes, so that the unlock needs neither SEV nor DSB; or make WFE/SEV an 
option, off by default on arm64.
   
   Thanks for the numbers — they make it clear the wait policy is a platform 
trade-off and must be opt-in. We replaced the unconditional WFE/SEV with a 
configurable lock wait policy, in two new commits:
   
   1. **`sched/Kconfig: add spinlock wait policy choice`** — a Kconfig choice 
between busy wait (`CONFIG_SPINLOCK_NOPWAIT`) and WFE/SEV wait 
(`CONFIG_SPINLOCK_WFESEV`), with `SPINLOCK_WAIT()`/`SPINLOCK_WAKE()` as the 
single policy point. The choice defaults to NOPWAIT; each architecture declares 
where WFE/SEV waiting pays off by selecting the hidden 
`SPINLOCK_WFESEV_DEFAULT` (armv7-a and Intel 64, and the M-profiles when SMP). 
**arm64 does not select it.**
   
   2. **`spinlock: route lock waiting through SPINLOCK_WAIT/SPINLOCK_WAKE`** — 
the lock paths call the policy macros instead of `UP_WFE()`/`UP_SEV()` directly.
   
   This is essentially your "make WFE/SEV an option, off by default on arm64", 
generalized. On arm64 the default NOPWAIT restores the master unlock path 
(plain release store, no DSB/SEV), so the regression in your table is gone — 
the "no WFE/SEV in spinlock.h" column is what to expect. On Intel 64, WFESEV is 
only PAUSE in the wait loop with an empty wake event, so unlock stays free; on 
Arm v7-A the DSB before SEV remains folded into `UP_SEV()` per ARM DDI 0406C 
("Use of WFE and SEV with Locks").
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to