Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement ticket spinlock #10605

Merged
merged 1 commit into from
Oct 6, 2023
Merged

Conversation

TaiJuWu
Copy link
Contributor

@TaiJuWu TaiJuWu commented Sep 12, 2023

Summary

A proposal to slove #1488
Implement ticket spinlock.

Impact

spinlock

Testing

./tools/configure.sh -l qemu-armv8a:nsh_smp
pass ostest

@TaiJuWu TaiJuWu marked this pull request as draft September 12, 2023 15:24
@TaiJuWu TaiJuWu force-pushed the fair_spinlock branch 3 times, most recently from 44ed929 to 7273fbe Compare September 12, 2023 16:07
@mu578
Copy link

mu578 commented Sep 12, 2023

You should try to stress it more to see if you don't hit any dead-locking on one cpu, process timing is important, so try to process heavy / greedy calls in each context, like allocating, read-write file, socket streams (triggering nasty interrupts in the middle), leaking memory on purpose, calls to getpid and alike, anything that is known to be resource system intensive.

sched/semaphore/spinlock.c Outdated Show resolved Hide resolved
@TaiJuWu TaiJuWu force-pushed the fair_spinlock branch 3 times, most recently from 4ba48af to 45171fc Compare September 13, 2023 02:29
@TaiJuWu
Copy link
Contributor Author

TaiJuWu commented Sep 15, 2023

You should try to stress it more to see if you don't hit any dead-locking on one cpu, process timing is important, so try to process heavy / greedy calls in each context, like allocating, read-write file, socket streams (triggering nasty interrupts in the middle), leaking memory on purpose, calls to getpid and alike, anything that is known to be resource system intensive.

I have two question:

  1. mem allocating and read-write file seems to use mutex, is it means that I have to modify these to use fair spinlock ?
  2. How can I test performance, can I test it on qemu? I have no physical board.

Could you explain more detail?

@TaiJuWu TaiJuWu force-pushed the fair_spinlock branch 5 times, most recently from f333435 to f62eca2 Compare September 15, 2023 12:50
@TaiJuWu TaiJuWu marked this pull request as ready for review September 15, 2023 14:37
@acassis
Copy link
Contributor

acassis commented Sep 15, 2023

You should try to stress it more to see if you don't hit any dead-locking on one cpu, process timing is important, so try to process heavy / greedy calls in each context, like allocating, read-write file, socket streams (triggering nasty interrupts in the middle), leaking memory on purpose, calls to getpid and alike, anything that is known to be resource system intensive.

I have two question:

1. mem allocating and  read-write file seems to use mutex, is it means that I have to modify these to use fair spinlock ?

2. How can I test performance, can I test it on qemu? I have no physical board.

Could you explain more detail?

I think you can use QEMU or even the SIM on Linux, but you will need to add some instrumentation to confirm everything is working as expected.

@TaiJuWu
Copy link
Contributor Author

TaiJuWu commented Sep 16, 2023

You should try to stress it more to see if you don't hit any dead-locking on one cpu, process timing is important, so try to process heavy / greedy calls in each context, like allocating, read-write file, socket streams (triggering nasty interrupts in the middle), leaking memory on purpose, calls to getpid and alike, anything that is known to be resource system intensive.

I have two question:

1. mem allocating and  read-write file seems to use mutex, is it means that I have to modify these to use fair spinlock ?

2. How can I test performance, can I test it on qemu? I have no physical board.

Could you explain more detail?

I think you can use QEMU or even the SIM on Linux, but you will need to add some instrumentation to confirm everything is working as expected.

I try following doc to enable INSTRUMENTATION.
But the result shows
image

This result is different from what I excepted.
Is the reason is encode my shell and ash?

@mu578
Copy link

mu578 commented Sep 16, 2023

You should try to stress it more to see if you don't hit any dead-locking on one cpu, process timing is important, so try to process heavy / greedy calls in each context, like allocating, read-write file, socket streams (triggering nasty interrupts in the middle), leaking memory on purpose, calls to getpid and alike, anything that is known to be resource system intensive.

I have two question:

  1. mem allocating and read-write file seems to use mutex, is it means that I have to modify these to use fair spinlock ?
  2. How can I test performance, can I test it on qemu? I have no physical board.

Could you explain more detail?

no there are good calls as is, that's because they will block and will forcibly trigger all sort of events in the middle, such in a unpredictable order, so context switch will happen and a lot, so if you wish: producing mess on purpose.

Qemu on a good host is ok if host and slave share the same arch, meanwhile having a physical board is a must, real clock.

@xiaoxiang781216
Copy link
Contributor

You should try to stress it more to see if you don't hit any dead-locking on one cpu, process timing is important, so try to process heavy / greedy calls in each context, like allocating, read-write file, socket streams (triggering nasty interrupts in the middle), leaking memory on purpose, calls to getpid and alike, anything that is known to be resource system intensive.

I have two question:

1. mem allocating and  read-write file seems to use mutex, is it means that I have to modify these to use fair spinlock ?

2. How can I test performance, can I test it on qemu? I have no physical board.

Could you explain more detail?

I think you can use QEMU or even the SIM on Linux, but you will need to add some instrumentation to confirm everything is working as expected.

I try following doc to enable INSTRUMENTATION. But the result shows image

This result is different from what I excepted. Is the reason is encode my shell and ash?

You need redirect the output to file and open it with https://ui.perfetto.dev/ or https://eclipse.dev/tracecompass/.

@xiaoxiang781216 xiaoxiang781216 linked an issue Sep 16, 2023 that may be closed by this pull request
sched/semaphore/spinlock.c Outdated Show resolved Hide resolved
sched/semaphore/spinlock.c Outdated Show resolved Hide resolved
sched/semaphore/spinlock.c Outdated Show resolved Hide resolved
sched/semaphore/spinlock.c Outdated Show resolved Hide resolved
@TaiJuWu TaiJuWu marked this pull request as draft September 19, 2023 15:14
@TaiJuWu TaiJuWu force-pushed the fair_spinlock branch 2 times, most recently from d6e7fde to 635ff05 Compare September 20, 2023 03:39
@TaiJuWu TaiJuWu force-pushed the fair_spinlock branch 4 times, most recently from bac2470 to 07d0dac Compare October 5, 2023 15:09
@TaiJuWu TaiJuWu marked this pull request as ready for review October 5, 2023 15:10
sched/semaphore/spinlock.c Outdated Show resolved Hide resolved
include/nuttx/spinlock.h Show resolved Hide resolved
include/nuttx/spinlock.h Outdated Show resolved Hide resolved
include/nuttx/spinlock.h Outdated Show resolved Hide resolved
include/nuttx/spinlock.h Show resolved Hide resolved
include/nuttx/spinlock.h Outdated Show resolved Hide resolved
sched/semaphore/spinlock.c Outdated Show resolved Hide resolved
include/nuttx/spinlock.h Outdated Show resolved Hide resolved
sched/semaphore/spinlock.c Outdated Show resolved Hide resolved
sched/semaphore/spinlock.c Outdated Show resolved Hide resolved
sched/irq/irq_csection.c Outdated Show resolved Hide resolved
@TaiJuWu TaiJuWu force-pushed the fair_spinlock branch 3 times, most recently from 8d148bb to 5f7f00d Compare October 6, 2023 01:42
@xiaoxiang781216
Copy link
Contributor

please run './tools/refresh.sh qemu-armv8a/nsh_smp' to fix the ci error.

@TaiJuWu
Copy link
Contributor Author

TaiJuWu commented Oct 6, 2023

please run './tools/refresh.sh qemu-armv8a/nsh_smp' to fix the ci error.

Done.

@xiaoxiang781216
Copy link
Contributor

The style need fix too:

../nuttx/tools/checkpatch.sh -u -m -g 57bf9d44d2d93619616a4c87ce96b85986f96c65..HEAD
Error: /home/runner/work/nuttx/nuttx/nuttx/sched/semaphore/spinlock.c:167:6: error: Bad left brace alignment
Error: /home/runner/work/nuttx/nuttx/nuttx/sched/semaphore/spinlock.c:169:6: error: Bad right brace alignment
Error: /home/runner/work/nuttx/nuttx/nuttx/sched/semaphore/spinlock.c:174:6: error: Bad left brace alignment
Error: /home/runner/work/nuttx/nuttx/nuttx/sched/semaphore/spinlock.c:176:6: error: Bad right brace alignment
Error: /home/runner/work/nuttx/nuttx/nuttx/sched/semaphore/spinlock.c:[23](https://github.com/apache/nuttx/actions/runs/6427966184/job/17454400895?pr=10605#step:3:24)1:6: error: Bad left brace alignment
Error: /home/runner/work/nuttx/nuttx/nuttx/sched/semaphore/spinlock.c:233:6: error: Bad right brace alignment
Error: /home/runner/work/nuttx/nuttx/nuttx/sched/semaphore/spinlock.c:238:6: error: Bad left brace alignment
Error: /home/runner/work/nuttx/nuttx/nuttx/sched/semaphore/spinlock.c:[24](https://github.com/apache/nuttx/actions/runs/6427966184/job/17454400895?pr=10605#step:3:25)0:6: error: Bad right brace alignment

you can verify locally by:

./tools/checkpatch.sh -g HEAD~...HEAD

test config: ./tools/configure.sh -l qemu-armv8a:nsh_smp

Pass ostest

No matter big-endian or little-endian, ticket spinlock only check the
next and the owner is equal or not.

If they are equal, it means there is a task hold the lock or lock is
free.

Signed-off-by: TaiJu Wu <[email protected]>

Co-authored-by: Xiang Xiao <[email protected]>
@TaiJuWu
Copy link
Contributor Author

TaiJuWu commented Oct 6, 2023

The style need fix too:

../nuttx/tools/checkpatch.sh -u -m -g 57bf9d44d2d93619616a4c87ce96b85986f96c65..HEAD
Error: /home/runner/work/nuttx/nuttx/nuttx/sched/semaphore/spinlock.c:167:6: error: Bad left brace alignment
Error: /home/runner/work/nuttx/nuttx/nuttx/sched/semaphore/spinlock.c:169:6: error: Bad right brace alignment
Error: /home/runner/work/nuttx/nuttx/nuttx/sched/semaphore/spinlock.c:174:6: error: Bad left brace alignment
Error: /home/runner/work/nuttx/nuttx/nuttx/sched/semaphore/spinlock.c:176:6: error: Bad right brace alignment
Error: /home/runner/work/nuttx/nuttx/nuttx/sched/semaphore/spinlock.c:[23](https://github.com/apache/nuttx/actions/runs/6427966184/job/17454400895?pr=10605#step:3:24)1:6: error: Bad left brace alignment
Error: /home/runner/work/nuttx/nuttx/nuttx/sched/semaphore/spinlock.c:233:6: error: Bad right brace alignment
Error: /home/runner/work/nuttx/nuttx/nuttx/sched/semaphore/spinlock.c:238:6: error: Bad left brace alignment
Error: /home/runner/work/nuttx/nuttx/nuttx/sched/semaphore/spinlock.c:[24](https://github.com/apache/nuttx/actions/runs/6427966184/job/17454400895?pr=10605#step:3:25)0:6: error: Bad right brace alignment

you can verify locally by:

./tools/checkpatch.sh -g HEAD~...HEAD

This is nxstyle issue. The formatting tool seems not to consider the case of nest curly brackets.
I can't find any reasonable format about this case.
The style like this.

@TaiJuWu TaiJuWu force-pushed the fair_spinlock branch 2 times, most recently from 9c3ac47 to b30307a Compare October 6, 2023 12:45
@xiaoxiang781216 xiaoxiang781216 force-pushed the fair_spinlock branch 2 times, most recently from 7ba101e to c3535e6 Compare October 6, 2023 13:11
@xiaoxiang781216 xiaoxiang781216 added the breaking change This change requires a mitigation entry in the release notes. label Oct 6, 2023
@xiaoxiang781216 xiaoxiang781216 merged commit ffba0d1 into apache:master Oct 6, 2023
@TaiJuWu TaiJuWu deleted the fair_spinlock branch October 8, 2023 14:26
@TaiJuWu TaiJuWu restored the fair_spinlock branch October 9, 2023 23:06
@TaiJuWu TaiJuWu deleted the fair_spinlock branch October 9, 2023 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change This change requires a mitigation entry in the release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use fair spinlocks in SMP
4 participants