mutex: move nxmutex to sched by zyfeier · Pull Request #8645 · apache/nuttx

zyfeier · 2023-02-24T10:07:43Z

Summary

Move nxmutex to sched;
Add mutex_clocklock, mutex_set_protocol and mutex_get_protocol interface;
Mutex is not cancellation point, delete cancellation point check;

Impact

Mutex

Testing

sabre-6quad:nsh ostest

pkarashchenko · 2023-03-02T21:48:47Z

I'm still not sure about this change. I know that some time in the past I drafter something similar, but after re-thinking I'm looking into a next direction: there is already one mutex layer available in user space and it is pthread_mutex. I do not think we need to introduce another layer otherwise it will bring duplication. We need to think how to solve libc issue in a different way. Till now nxmutex was simply a wrapper on top of sem/nxsem, but now it seems to be something more.

zyfeier · 2023-03-03T01:30:17Z

I'm still not sure about this change. I know that some time in the past I drafter something similar, but after re-thinking I'm looking into a next direction: there is already one mutex layer available in user space and it is pthread_mutex. I do not think we need to introduce another layer otherwise it will bring duplication. We need to think how to solve libc issue in a different way. Till now nxmutex was simply a wrapper on top of sem/nxsem, but now it seems to be something more.

@pkarashchenko
Yes, this is only the first step of the modification, the following changes will be dependent on this modification:

pthread mutex is changed to depend on nxmutex instead of nxsem.
nxmutex no longer depends on nxsem and is implemented independently;
Move priority inheritance from nxsem to nxmutex, and optimize the performance of priority inheritance;

zyfeier · 2023-03-09T13:10:43Z

@xiaoxiang781216 @pkarashchenko Any comments on this PR? Please help to review it again.

pkarashchenko · 2023-03-09T14:39:12Z

I will take a look over the weekend.

xiaoxiang781216 · 2023-03-13T06:48:32Z

I will take a look over the weekend.

@pkarashchenko do you finish the review?

pkarashchenko · 2023-03-13T10:44:50Z

I will take a look over the weekend.

@pkarashchenko do you finish the review?

Done. Sorry, I experienced many delays during my trip, so was not able to review on time

pkarashchenko

In general I do not see any issues that stop this PR from merging except one.
I'm missing the overall roadmap for the mutex support. The nxmutex is not a cancellation point but if I recall correctly (@xiaoxiang781216 please correct me if I'm wrong) there is an intention to move pthread_mutex (and maybe pthead_rwlock) to use nxmutex interface. The pthread_mutex interface is not a cancellation point, so all seems to be fine, but pthread_rwlock is a cancellation point.

I really would like to re-inspect the affected area and have nxmutex discussion moving to the mailing list before we are not too far away. By the affected area I mean that printf for example is a cancellation point and many other places as well. I will spend some time this week to figure out if those functions rely on the sem/pthread_mutex interface in user mode and how this change will affect that.

zyfeier · 2023-03-14T02:04:10Z

In general I do not see any issues that stop this PR from merging except one. I'm missing the overall roadmap for the mutex support. The nxmutex is not a cancellation point but if I recall correctly (@xiaoxiang781216 please correct me if I'm wrong) there is an intention to move pthread_mutex (and maybe pthead_rwlock) to use nxmutex interface. The pthread_mutex interface is not a cancellation point, so all seems to be fine, but pthread_rwlock is a cancellation point.

I really would like to re-inspect the affected area and have nxmutex discussion moving to the mailing list before we are not too far away. By the affected area I mean that printf for example is a cancellation point and many other places as well. I will spend some time this week to figure out if those functions rely on the sem/pthread_mutex interface in user mode and how this change will affect that.

@pkarashchenko About pthread rwlock, although pthread_mutex is not a cancellation point, but pthread_cond_wait is, so i think this should be no problem. But it's still necessary to re-inspect the affected area.

1. Move nxmutex to sched and directly depends on the nxsem; 2. Mutex is not cancellation point, delete cancellation point check; Signed-off-by: zhangyuan21 <zhangyuan21@xiaomi.com>

add nxmutex_clocklock, nxmutex_set_protocol and nxmutex_get_protocol interface Signed-off-by: zhangyuan21 <zhangyuan21@xiaomi.com>

pkarashchenko · 2023-03-22T15:30:27Z

I'm planning to start mailing list discussion till the end of this week.

xiaoxiang781216 requested a review from pkarashchenko February 24, 2023 10:09

zyfeier mentioned this pull request Feb 24, 2023

nuttx/mutex: do not use non-nx interface in kernel except libs #6376

Closed

gustavonihei reviewed Feb 24, 2023

View reviewed changes

Comment thread libs/libnx/nxfonts/nxfonts_cache.c Outdated

gustavonihei reviewed Feb 24, 2023

View reviewed changes

Comment thread sched/mutex/mutex.h

gustavonihei reviewed Feb 24, 2023

View reviewed changes

Comment thread sched/mutex/mutex_breaklock.c Outdated

gustavonihei reviewed Feb 24, 2023

View reviewed changes

Comment thread sched/mutex/mutex_breaklock.c Outdated

zyfeier force-pushed the mutex branch 10 times, most recently from 8047165 to 3926ae5 Compare March 1, 2023 01:34

zyfeier marked this pull request as ready for review March 1, 2023 03:28

xiaoxiang781216 requested a review from gustavonihei March 1, 2023 18:51

xiaoxiang781216 reviewed Mar 2, 2023

View reviewed changes

Comment thread include/mutex.h Outdated

Comment thread include/nuttx/mutex.h Outdated

pkarashchenko requested review from davids5, hartmannathan and patacongo March 2, 2023 21:49

zyfeier force-pushed the mutex branch 4 times, most recently from cce5b1e to 1e7e1b8 Compare March 3, 2023 09:00

zyfeier changed the title ~~mutex: move nxmutex to sched and add mutex_ interface for userspace~~ mutex: move nxmutex to sched Mar 3, 2023

xiaoxiang781216 reviewed Mar 4, 2023

View reviewed changes

Comment thread syscall/syscall.csv Outdated

Comment thread sched/mutex/mutex_reset.c Outdated

Comment thread include/sys/syscall_lookup.h Outdated

Comment thread include/sys/syscall_lookup.h Outdated

Comment thread sched/mutex/mutex.h Outdated

zyfeier force-pushed the mutex branch 2 times, most recently from 8f24d1b to 5356830 Compare March 6, 2023 10:42

xiaoxiang781216 reviewed Mar 6, 2023

View reviewed changes

zyfeier force-pushed the mutex branch 2 times, most recently from 1ff51b4 to 56616b5 Compare March 7, 2023 12:55

xiaoxiang781216 reviewed Mar 11, 2023

View reviewed changes

zyfeier force-pushed the mutex branch from 56616b5 to adbd15a Compare March 13, 2023 04:51

xiaoxiang781216 approved these changes Mar 13, 2023

View reviewed changes

pkarashchenko reviewed Mar 13, 2023

View reviewed changes

Comment thread include/nuttx/mutex.h Outdated

Comment thread include/nuttx/mutex.h Outdated

Comment thread sched/mutex/mutex_lock.c Outdated

Comment thread sched/mutex/mutex_timedlock.c Outdated

Comment thread sched/mutex/mutex_trylock.c Outdated

zyfeier force-pushed the mutex branch from adbd15a to ee306c6 Compare March 13, 2023 11:36

pkarashchenko reviewed Mar 13, 2023

View reviewed changes

zhangyuan21 added 2 commits March 22, 2023 13:39

sched/mutex: Move nxmutex to sched

906e865

1. Move nxmutex to sched and directly depends on the nxsem; 2. Mutex is not cancellation point, delete cancellation point check; Signed-off-by: zhangyuan21 <zhangyuan21@xiaomi.com>

sched/mutex: Add more mutex interface

9d89ffb

add nxmutex_clocklock, nxmutex_set_protocol and nxmutex_get_protocol interface Signed-off-by: zhangyuan21 <zhangyuan21@xiaomi.com>

zyfeier force-pushed the mutex branch from ee306c6 to 9d89ffb Compare March 22, 2023 12:00

zyfeier marked this pull request as draft April 11, 2023 06:13

zyfeier closed this Apr 18, 2023

zyfeier deleted the mutex branch April 28, 2023 02:00

Uh oh!

Conversation

zyfeier commented Feb 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Impact

Testing

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pkarashchenko commented Mar 2, 2023

Uh oh!

zyfeier commented Mar 3, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zyfeier commented Mar 9, 2023

Uh oh!

pkarashchenko commented Mar 9, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xiaoxiang781216 commented Mar 13, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pkarashchenko commented Mar 13, 2023

Uh oh!

pkarashchenko left a comment

Choose a reason for hiding this comment

Uh oh!

zyfeier commented Mar 14, 2023

Uh oh!

pkarashchenko commented Mar 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

zyfeier commented Feb 24, 2023 •

edited

Loading