You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/04/11 06:10:10 UTC
[GitHub] [incubator-nuttx] anjiahao1 opened a new pull request, #5070: libc/semaphore:sem_init change defult protocol
anjiahao1 opened a new pull request, #5070:
URL: https://github.com/apache/incubator-nuttx/pull/5070
Semaphores are generally used in two ways, one is to count resources or notify event, the other is to use as a mutual exclusion lock
In Nuttx, the default behavior of semaphores is mutual exclusion lock. I think it is better to change the default behavior to count resources or notify event, and it will be more general in Linux programs.
Why would I do this:
In porting some programs which confirm POSIX standard, the semaphore is used to notify event, but its default behavior is a mutual exclusion lock(nuttx): One thread post the semaphore to indicate something happen, another thread gets the semaphore and exits. when the first thread post the semaphore again which will cause a crash when CONFIG_PRIORITY_INHERITANCE is enable, because htcb field in semholder_s point to a freed memory.
so, it's better to change the default sem behavior to no priority inheritance.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] hartmannathan commented on a diff in pull request #5070: libc/semaphore:sem_init change defult protocol
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on code in PR #5070:
URL: https://github.com/apache/incubator-nuttx/pull/5070#discussion_r999883081
##########
Documentation/reference/os/mutex.rst:
##########
@@ -0,0 +1,130 @@
+=====================
+Mutual Exclusion lock
+=====================
+
+nxmutex
+=======
+
+Use `nxmutex` prefixed api for protect resources.In fact, nxmutex is implemented
+based on nxsem,The difference between nxmutex and nxsem is that nxmutex supports
+priority inheritance by default, nxsem not support priority inheritance by default
Review Comment:
Perhaps it should be `nxsem does not support priority inheritance by default`.
(Also it seems some spaces are missing after periods and commas.)
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] anjiahao1 closed pull request #5070: libc/semaphore:sem_init change defult protocol
Posted by GitBox <gi...@apache.org>.
anjiahao1 closed pull request #5070: libc/semaphore:sem_init change defult protocol
URL: https://github.com/apache/incubator-nuttx/pull/5070
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] anjiahao1 commented on pull request #5070: libc/semaphore:sem_init change defult protocol
Posted by GitBox <gi...@apache.org>.
anjiahao1 commented on PR #5070:
URL: https://github.com/apache/incubator-nuttx/pull/5070#issuecomment-1284876786
>
[#6965 ](https://github.com/apache/incubator-nuttx/pull/6965) this path repaced all sem use as lock with mutex
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #5070: libc/semaphore:sem_init change defult protocol
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #5070:
URL: https://github.com/apache/incubator-nuttx/pull/5070#issuecomment-1287658210
@xiaoxiang781216
I think we need to merge https://github.com/apache/incubator-nuttx-apps/pull/1358 as well.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5070: libc/semaphore:sem_init change defult protocol
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #5070:
URL: https://github.com/apache/incubator-nuttx/pull/5070#issuecomment-1290019823
The description looks good, @hartmannathan thanks.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] hartmannathan commented on pull request #5070: libc/semaphore:sem_init change defult protocol
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on PR #5070:
URL: https://github.com/apache/incubator-nuttx/pull/5070#issuecomment-1284887367
> >
>
>
>
> [#6965 ](https://github.com/apache/incubator-nuttx/pull/6965) this patch repaced all sem use as lock with mutex
Great! This is a much cleaner design. Thank you!
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5070: libc/semaphore:sem_init change defult protocol
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #5070:
URL: https://github.com/apache/incubator-nuttx/pull/5070#issuecomment-1284871474
> Before, mutual exclusion semaphores had priority inheritance enabled by default; now it is opt-in.
>
> So I wonder if the opposite of commit [48d8383](https://github.com/apache/incubator-nuttx/commit/48d83839b3b63693448ab22c695451ddde5db107) ("sem:remove sem default protocl") should be done also: Locate all mutual exclusion semaphores and call (nx)sem_set_protocol(..., SEM_PRIO_INHERIT)?
it's done by converting all mutual exclusion semaphores to mutex lock in this PR https://github.com/apache/incubator-nuttx/pull/6320
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #5070: libc/semaphore:sem_init change defult protocol
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #5070:
URL: https://github.com/apache/incubator-nuttx/pull/5070#discussion_r998678545
##########
include/semaphore.h:
##########
@@ -48,8 +48,8 @@
/* Bit definitions for the struct sem_s flags field */
-#define PRIOINHERIT_FLAGS_DISABLE (1 << 0) /* Bit 0: Priority inheritance
- * is disabled for this semaphore. */
+#define PRIOINHERIT_FLAGS_ENABLE (1 << 0) /* Bit 0: Priority inheritance
Review Comment:
I do not see where `NXMUTEX_INITIALIZER` and `NXRMUTEX_INITIALIZER` are updated to reflect this change.
##########
arch/arm/src/am335x/am335x_i2c.c:
##########
@@ -316,7 +316,7 @@ static struct am335x_i2c_priv_s am335x_i2c0_priv =
.refs = 0,
.lock = NXMUTEX_INITIALIZER,
#ifndef CONFIG_I2C_POLLED
- .sem_isr = NXSEM_INITIALIZER(0, PRIOINHERIT_FLAGS_DISABLE),
+ .sem_isr = SEM_INITIALIZER(0),
Review Comment:
Let's use `NXSEM_INITIALIZER` in kernel code
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on a diff in pull request #5070: libc/semaphore:sem_init change defult protocol
Posted by GitBox <gi...@apache.org>.
davids5 commented on code in PR #5070:
URL: https://github.com/apache/incubator-nuttx/pull/5070#discussion_r999812064
##########
Documentation/reference/os/mutex.rst:
##########
@@ -0,0 +1,130 @@
+=====================
+Mutual Exclusion lock
+=====================
+
+nxmutex
+=======
+
+Use `nxmutex` prefixed api for protect resources.In fact, nxmutex is implemented
+based on nxsem,The difference between nxmutex and nxsem is that nxmutex supports
+priority inheritance by default, nxsem not support priority inheritance by default
Review Comment:
```suggestion
priority inheritance by default, nxsem do not support priority inheritance by default
```
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #5070: libc/semaphore:sem_init change defult protocol
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #5070:
URL: https://github.com/apache/incubator-nuttx/pull/5070#discussion_r998952692
##########
include/semaphore.h:
##########
@@ -48,8 +48,8 @@
/* Bit definitions for the struct sem_s flags field */
-#define PRIOINHERIT_FLAGS_DISABLE (1 << 0) /* Bit 0: Priority inheritance
- * is disabled for this semaphore. */
+#define PRIOINHERIT_FLAGS_ENABLE (1 << 0) /* Bit 0: Priority inheritance
Review Comment:
let change NXSEM_INITIALIZER(xxx, PRIOINHERIT_FLAGS_DISABLE) to SEM_INITIALIZER(xxx)
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #5070: libc/semaphore:sem_init change defult protocol
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #5070:
URL: https://github.com/apache/incubator-nuttx/pull/5070#issuecomment-1100765147
Related to https://github.com/apache/incubator-nuttx/issues/6084
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] anjiahao1 commented on pull request #5070: libc/semaphore:sem_init change defult protocol
Posted by GitBox <gi...@apache.org>.
anjiahao1 commented on PR #5070:
URL: https://github.com/apache/incubator-nuttx/pull/5070#issuecomment-1283381549
> How is this PR related to PR #7344 ? Should they be combined to a single PR?
ok,i will use single PR and add documentation later
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #5070: libc/semaphore:sem_init change defult protocol
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #5070:
URL: https://github.com/apache/incubator-nuttx/pull/5070#discussion_r998952692
##########
include/semaphore.h:
##########
@@ -48,8 +48,8 @@
/* Bit definitions for the struct sem_s flags field */
-#define PRIOINHERIT_FLAGS_DISABLE (1 << 0) /* Bit 0: Priority inheritance
- * is disabled for this semaphore. */
+#define PRIOINHERIT_FLAGS_ENABLE (1 << 0) /* Bit 0: Priority inheritance
Review Comment:
let change NXMUTEX_INITIALIZER(xxx, PRIOINHERIT_FLAGS_DISABLE) to MUTEX_INITIALIZER(xxx)
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] hartmannathan commented on pull request #5070: libc/semaphore:sem_init change defult protocol
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on PR #5070:
URL: https://github.com/apache/incubator-nuttx/pull/5070#issuecomment-1289974488
I have documented the mitigation entry for the next release's Release Notes here: https://cwiki.apache.org/confluence/display/NUTTX/NuttX+11.1.0#NuttX11.1.0-DefaultSemaphoreProtocolChanged
Please check and let me know if there are any mistakes!
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] hartmannathan commented on pull request #5070: libc/semaphore:sem_init change defult protocol
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on PR #5070:
URL: https://github.com/apache/incubator-nuttx/pull/5070#issuecomment-1283377442
How is this PR related to PR #7344 ? Should they be combined to a single PR?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5070: libc/semaphore:sem_init change defult protocol
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #5070:
URL: https://github.com/apache/incubator-nuttx/pull/5070#issuecomment-1282127307
@anjiahao1 let rebase this patch and fix the conflict.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5070: libc/semaphore:sem_init change defult protocol
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #5070:
URL: https://github.com/apache/incubator-nuttx/pull/5070#issuecomment-1282475173
@pkarashchenko @hartmannathan @masayuki2009 this is the last step to correct the default value of priority inheritance, please review this patch and https://github.com/apache/incubator-nuttx/pull/7344.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #5070: libc/semaphore:sem_init change defult protocol
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #5070:
URL: https://github.com/apache/incubator-nuttx/pull/5070#discussion_r998290536
##########
sched/semaphore/sem_holder.c:
##########
@@ -707,7 +707,7 @@ void nxsem_add_holder_tcb(FAR struct tcb_s *htcb, FAR sem_t *sem)
* inheritance is effectively disabled.
*/
- if (htcb->flink != NULL && (sem->flags & PRIOINHERIT_FLAGS_DISABLE) == 0)
+ if (htcb->flink != NULL && (sem->flags & PRIOINHERIT_FLAGS_ENABLE) == 0)
Review Comment:
```suggestion
if (htcb->flink != NULL && (sem->flags & PRIOINHERIT_FLAGS_ENABLE) != 0)
```
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] hartmannathan commented on a diff in pull request #5070: libc/semaphore:sem_init change defult protocol
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on code in PR #5070:
URL: https://github.com/apache/incubator-nuttx/pull/5070#discussion_r999892096
##########
Documentation/reference/os/mutex.rst:
##########
@@ -0,0 +1,130 @@
+=====================
+Mutual Exclusion lock
+=====================
+
+nxmutex
+=======
+
+Use `nxmutex` prefixed api for protect resources.In fact, nxmutex is implemented
+based on nxsem,The difference between nxmutex and nxsem is that nxmutex supports
+priority inheritance by default, nxsem not support priority inheritance by default
+
+usually usage:
+
+call nxmutex_init() for driver,when two task will use driver, Their timing will be
+============== ==============
+taskA taskB
+=============== ==============
+nxmutex_lock() nxmutex_lock()
+=============== ==============
+get lock runing wait for lock
Review Comment:
```suggestion
get lock running wait for lock
```
##########
Documentation/reference/os/mutex.rst:
##########
@@ -0,0 +1,130 @@
+=====================
+Mutual Exclusion lock
+=====================
+
+nxmutex
+=======
+
+Use `nxmutex` prefixed api for protect resources.In fact, nxmutex is implemented
+based on nxsem,The difference between nxmutex and nxsem is that nxmutex supports
+priority inheritance by default, nxsem not support priority inheritance by default
+
+usually usage:
+
+call nxmutex_init() for driver,when two task will use driver, Their timing will be
+============== ==============
+taskA taskB
+=============== ==============
+nxmutex_lock() nxmutex_lock()
+=============== ==============
+get lock runing wait for lock
+================ ==============
+nxmutex_unlock() wait for lock
+================ ===============
+ get lock runing
+================ ===============
+ nxmutex_unlock
+
+Priority inheritance
+====================
+
+if `CONFIG_PRIORITY_INHERITANCE` is chosen, The priority of the task holding the mutex
+may be changed.
+This is a example:
Review Comment:
```suggestion
This is an example:
```
##########
Documentation/reference/os/mutex.rst:
##########
@@ -0,0 +1,130 @@
+=====================
+Mutual Exclusion lock
+=====================
+
+nxmutex
+=======
+
+Use `nxmutex` prefixed api for protect resources.In fact, nxmutex is implemented
+based on nxsem,The difference between nxmutex and nxsem is that nxmutex supports
+priority inheritance by default, nxsem not support priority inheritance by default
+
+usually usage:
+
+call nxmutex_init() for driver,when two task will use driver, Their timing will be
+============== ==============
+taskA taskB
+=============== ==============
+nxmutex_lock() nxmutex_lock()
+=============== ==============
+get lock runing wait for lock
+================ ==============
+nxmutex_unlock() wait for lock
+================ ===============
+ get lock runing
Review Comment:
```suggestion
get lock running
```
##########
Documentation/reference/os/mutex.rst:
##########
@@ -0,0 +1,130 @@
+=====================
+Mutual Exclusion lock
+=====================
+
+nxmutex
+=======
+
+Use `nxmutex` prefixed api for protect resources.In fact, nxmutex is implemented
+based on nxsem,The difference between nxmutex and nxsem is that nxmutex supports
+priority inheritance by default, nxsem not support priority inheritance by default
+
+usually usage:
+
+call nxmutex_init() for driver,when two task will use driver, Their timing will be
+============== ==============
+taskA taskB
+=============== ==============
+nxmutex_lock() nxmutex_lock()
+=============== ==============
+get lock runing wait for lock
+================ ==============
+nxmutex_unlock() wait for lock
+================ ===============
+ get lock runing
+================ ===============
+ nxmutex_unlock
+
+Priority inheritance
+====================
+
+if `CONFIG_PRIORITY_INHERITANCE` is chosen, The priority of the task holding the mutex
+may be changed.
+This is a example:
+
+ There are three tasks.Their priorities are high, medium, and low.
+ We refer to them as `Htask` `Mtask` `Ltask`
+
+ `Htask` and `Ltask` will hold the same mutex. `Mtask` does not hold mutex
+
+if `CONFIG_PRIORITY_INHERITANCE` is not chosen, task runing order
Review Comment:
```suggestion
if `CONFIG_PRIORITY_INHERITANCE` is not chosen, task running order
```
##########
Documentation/reference/os/mutex.rst:
##########
@@ -0,0 +1,130 @@
+=====================
+Mutual Exclusion lock
+=====================
+
+nxmutex
+=======
+
+Use `nxmutex` prefixed api for protect resources.In fact, nxmutex is implemented
+based on nxsem,The difference between nxmutex and nxsem is that nxmutex supports
+priority inheritance by default, nxsem not support priority inheritance by default
+
+usually usage:
+
+call nxmutex_init() for driver,when two task will use driver, Their timing will be
Review Comment:
```suggestion
call nxmutex_init() for driver. When two tasks will use driver, their timing will be:
```
##########
Documentation/reference/os/mutex.rst:
##########
@@ -0,0 +1,130 @@
+=====================
+Mutual Exclusion lock
+=====================
+
+nxmutex
+=======
+
+Use `nxmutex` prefixed api for protect resources.In fact, nxmutex is implemented
+based on nxsem,The difference between nxmutex and nxsem is that nxmutex supports
+priority inheritance by default, nxsem not support priority inheritance by default
+
+usually usage:
+
+call nxmutex_init() for driver,when two task will use driver, Their timing will be
+============== ==============
+taskA taskB
+=============== ==============
+nxmutex_lock() nxmutex_lock()
+=============== ==============
+get lock runing wait for lock
+================ ==============
+nxmutex_unlock() wait for lock
+================ ===============
+ get lock runing
+================ ===============
+ nxmutex_unlock
+
+Priority inheritance
+====================
+
+if `CONFIG_PRIORITY_INHERITANCE` is chosen, The priority of the task holding the mutex
+may be changed.
+This is a example:
+
+ There are three tasks.Their priorities are high, medium, and low.
+ We refer to them as `Htask` `Mtask` `Ltask`
+
+ `Htask` and `Ltask` will hold the same mutex. `Mtask` does not hold mutex
+
+if `CONFIG_PRIORITY_INHERITANCE` is not chosen, task runing order
+ #. `Ltask` hold a mutex first
+ #. Then `Htask` runing, `Htask` can't hold the mutex,so wait
Review Comment:
```suggestion
#. Then `Htask` running, `Htask` can't hold the mutex,so wait
```
##########
Documentation/reference/os/mutex.rst:
##########
@@ -0,0 +1,130 @@
+=====================
+Mutual Exclusion lock
+=====================
+
+nxmutex
+=======
+
+Use `nxmutex` prefixed api for protect resources.In fact, nxmutex is implemented
+based on nxsem,The difference between nxmutex and nxsem is that nxmutex supports
+priority inheritance by default, nxsem not support priority inheritance by default
+
+usually usage:
+
+call nxmutex_init() for driver,when two task will use driver, Their timing will be
+============== ==============
+taskA taskB
+=============== ==============
+nxmutex_lock() nxmutex_lock()
+=============== ==============
+get lock runing wait for lock
+================ ==============
+nxmutex_unlock() wait for lock
+================ ===============
+ get lock runing
+================ ===============
+ nxmutex_unlock
+
+Priority inheritance
+====================
+
+if `CONFIG_PRIORITY_INHERITANCE` is chosen, The priority of the task holding the mutex
+may be changed.
+This is a example:
+
+ There are three tasks.Their priorities are high, medium, and low.
Review Comment:
```suggestion
There are three tasks. Their priorities are high, medium, and low.
```
##########
Documentation/reference/os/mutex.rst:
##########
@@ -0,0 +1,130 @@
+=====================
+Mutual Exclusion lock
+=====================
+
+nxmutex
+=======
+
+Use `nxmutex` prefixed api for protect resources.In fact, nxmutex is implemented
Review Comment:
```suggestion
Use `nxmutex` prefixed api to protect resources. In fact, nxmutex is implemented
```
##########
Documentation/reference/os/mutex.rst:
##########
@@ -0,0 +1,130 @@
+=====================
+Mutual Exclusion lock
+=====================
+
+nxmutex
+=======
+
+Use `nxmutex` prefixed api for protect resources.In fact, nxmutex is implemented
+based on nxsem,The difference between nxmutex and nxsem is that nxmutex supports
+priority inheritance by default, nxsem not support priority inheritance by default
+
+usually usage:
+
+call nxmutex_init() for driver,when two task will use driver, Their timing will be
+============== ==============
+taskA taskB
+=============== ==============
+nxmutex_lock() nxmutex_lock()
+=============== ==============
+get lock runing wait for lock
+================ ==============
+nxmutex_unlock() wait for lock
+================ ===============
+ get lock runing
+================ ===============
+ nxmutex_unlock
+
+Priority inheritance
+====================
+
+if `CONFIG_PRIORITY_INHERITANCE` is chosen, The priority of the task holding the mutex
+may be changed.
+This is a example:
+
+ There are three tasks.Their priorities are high, medium, and low.
+ We refer to them as `Htask` `Mtask` `Ltask`
+
+ `Htask` and `Ltask` will hold the same mutex. `Mtask` does not hold mutex
+
+if `CONFIG_PRIORITY_INHERITANCE` is not chosen, task runing order
+ #. `Ltask` hold a mutex first
+ #. Then `Htask` runing, `Htask` can't hold the mutex,so wait
+ #. Then `Mtask` runing, because `Mtask` priority higher than `Ltask`.
Review Comment:
```suggestion
#. Then `Mtask` running, because `Mtask` priority higher than `Ltask`.
```
##########
Documentation/reference/os/mutex.rst:
##########
@@ -0,0 +1,130 @@
+=====================
+Mutual Exclusion lock
+=====================
+
+nxmutex
+=======
+
+Use `nxmutex` prefixed api for protect resources.In fact, nxmutex is implemented
+based on nxsem,The difference between nxmutex and nxsem is that nxmutex supports
Review Comment:
```suggestion
based on nxsem. The difference between nxmutex and nxsem is that nxmutex supports
```
##########
Documentation/reference/os/mutex.rst:
##########
@@ -0,0 +1,130 @@
+=====================
+Mutual Exclusion lock
+=====================
+
+nxmutex
+=======
+
+Use `nxmutex` prefixed api for protect resources.In fact, nxmutex is implemented
+based on nxsem,The difference between nxmutex and nxsem is that nxmutex supports
+priority inheritance by default, nxsem not support priority inheritance by default
+
+usually usage:
+
+call nxmutex_init() for driver,when two task will use driver, Their timing will be
+============== ==============
+taskA taskB
+=============== ==============
+nxmutex_lock() nxmutex_lock()
+=============== ==============
+get lock runing wait for lock
+================ ==============
+nxmutex_unlock() wait for lock
+================ ===============
+ get lock runing
+================ ===============
+ nxmutex_unlock
+
+Priority inheritance
+====================
+
+if `CONFIG_PRIORITY_INHERITANCE` is chosen, The priority of the task holding the mutex
Review Comment:
```suggestion
If `CONFIG_PRIORITY_INHERITANCE` is chosen, the priority of the task holding the mutex
```
##########
Documentation/reference/os/mutex.rst:
##########
@@ -0,0 +1,130 @@
+=====================
+Mutual Exclusion lock
+=====================
+
+nxmutex
+=======
+
+Use `nxmutex` prefixed api for protect resources.In fact, nxmutex is implemented
+based on nxsem,The difference between nxmutex and nxsem is that nxmutex supports
+priority inheritance by default, nxsem not support priority inheritance by default
+
+usually usage:
+
+call nxmutex_init() for driver,when two task will use driver, Their timing will be
+============== ==============
+taskA taskB
+=============== ==============
+nxmutex_lock() nxmutex_lock()
+=============== ==============
+get lock runing wait for lock
+================ ==============
+nxmutex_unlock() wait for lock
+================ ===============
+ get lock runing
+================ ===============
+ nxmutex_unlock
+
+Priority inheritance
+====================
+
+if `CONFIG_PRIORITY_INHERITANCE` is chosen, The priority of the task holding the mutex
+may be changed.
+This is a example:
+
+ There are three tasks.Their priorities are high, medium, and low.
+ We refer to them as `Htask` `Mtask` `Ltask`
+
+ `Htask` and `Ltask` will hold the same mutex. `Mtask` does not hold mutex
+
+if `CONFIG_PRIORITY_INHERITANCE` is not chosen, task runing order
+ #. `Ltask` hold a mutex first
+ #. Then `Htask` runing, `Htask` can't hold the mutex,so wait
+ #. Then `Mtask` runing, because `Mtask` priority higher than `Ltask`.
+ #. When `Mtask` finish, `Ltask` will start running.
+ #. When `Ltask` finish, `Htask` will start running.
+
+From the above process, we can see that the medium-priority tasks run ahead of
+the high-priority tasks, which is unacceptable.
+
+if `CONFIG_PRIORITY_INHERITANCE` is chosen, task runing order
+ #. `Ltask` hold a mutex first.
+ #. Then `Htask` runing, `Htask` can't hold the mutex, then boost the priority of `Ltask`
+to be the same as `Htask`.
+ #. Because `Ltask` priority is higher than `Mtask`,so `Mtask` not runing.
+ #. When 'Ltask' finish, `Htask` will start runing.
Review Comment:
```suggestion
#. When 'Ltask' finish, `Htask` will start running.
```
##########
Documentation/reference/os/mutex.rst:
##########
@@ -0,0 +1,130 @@
+=====================
+Mutual Exclusion lock
+=====================
+
+nxmutex
+=======
+
+Use `nxmutex` prefixed api for protect resources.In fact, nxmutex is implemented
+based on nxsem,The difference between nxmutex and nxsem is that nxmutex supports
+priority inheritance by default, nxsem not support priority inheritance by default
+
+usually usage:
+
+call nxmutex_init() for driver,when two task will use driver, Their timing will be
+============== ==============
+taskA taskB
+=============== ==============
+nxmutex_lock() nxmutex_lock()
+=============== ==============
+get lock runing wait for lock
+================ ==============
+nxmutex_unlock() wait for lock
+================ ===============
+ get lock runing
+================ ===============
+ nxmutex_unlock
+
+Priority inheritance
+====================
+
+if `CONFIG_PRIORITY_INHERITANCE` is chosen, The priority of the task holding the mutex
+may be changed.
+This is a example:
+
+ There are three tasks.Their priorities are high, medium, and low.
+ We refer to them as `Htask` `Mtask` `Ltask`
+
+ `Htask` and `Ltask` will hold the same mutex. `Mtask` does not hold mutex
+
+if `CONFIG_PRIORITY_INHERITANCE` is not chosen, task runing order
+ #. `Ltask` hold a mutex first
+ #. Then `Htask` runing, `Htask` can't hold the mutex,so wait
+ #. Then `Mtask` runing, because `Mtask` priority higher than `Ltask`.
+ #. When `Mtask` finish, `Ltask` will start running.
+ #. When `Ltask` finish, `Htask` will start running.
+
+From the above process, we can see that the medium-priority tasks run ahead of
+the high-priority tasks, which is unacceptable.
+
+if `CONFIG_PRIORITY_INHERITANCE` is chosen, task runing order
Review Comment:
```suggestion
if `CONFIG_PRIORITY_INHERITANCE` is chosen, task running order
```
##########
Documentation/reference/os/mutex.rst:
##########
@@ -0,0 +1,130 @@
+=====================
+Mutual Exclusion lock
+=====================
+
+nxmutex
+=======
+
+Use `nxmutex` prefixed api for protect resources.In fact, nxmutex is implemented
+based on nxsem,The difference between nxmutex and nxsem is that nxmutex supports
+priority inheritance by default, nxsem not support priority inheritance by default
+
+usually usage:
+
+call nxmutex_init() for driver,when two task will use driver, Their timing will be
+============== ==============
+taskA taskB
+=============== ==============
+nxmutex_lock() nxmutex_lock()
+=============== ==============
+get lock runing wait for lock
+================ ==============
+nxmutex_unlock() wait for lock
+================ ===============
+ get lock runing
+================ ===============
+ nxmutex_unlock
+
+Priority inheritance
+====================
+
+if `CONFIG_PRIORITY_INHERITANCE` is chosen, The priority of the task holding the mutex
+may be changed.
+This is a example:
+
+ There are three tasks.Their priorities are high, medium, and low.
+ We refer to them as `Htask` `Mtask` `Ltask`
+
+ `Htask` and `Ltask` will hold the same mutex. `Mtask` does not hold mutex
+
+if `CONFIG_PRIORITY_INHERITANCE` is not chosen, task runing order
+ #. `Ltask` hold a mutex first
+ #. Then `Htask` runing, `Htask` can't hold the mutex,so wait
+ #. Then `Mtask` runing, because `Mtask` priority higher than `Ltask`.
+ #. When `Mtask` finish, `Ltask` will start running.
+ #. When `Ltask` finish, `Htask` will start running.
+
+From the above process, we can see that the medium-priority tasks run ahead of
+the high-priority tasks, which is unacceptable.
+
+if `CONFIG_PRIORITY_INHERITANCE` is chosen, task runing order
+ #. `Ltask` hold a mutex first.
+ #. Then `Htask` runing, `Htask` can't hold the mutex, then boost the priority of `Ltask`
+to be the same as `Htask`.
+ #. Because `Ltask` priority is higher than `Mtask`,so `Mtask` not runing.
+ #. When 'Ltask' finish, `Htask` will start runing.
+ #. When `Htask` finish, `Mtask` will start runing.
+
+Priority inheritance prevents medium-priority tasks from running ahead of
+high-priority tasks
+
+Api description
+===============
+.. c:function:: void nxmutex_init(FAR mutex_t *mutex)
+
+ This function initialize the UNNAMED mutex
+ :param mutex: mutex to be initialized.
+
+ :return:
+ Zero(OK) is returned on success.A negated errno value is returned on failure.
+
+.. c:function:: void nxmutex_destroy(FAR mutex_t *mutex)
+
+ This function destroy the UNNAMED mutex
+ :param mutex: mutex to be destroyed.
+
+ :return:
+ Zero(OK) is returned on success.A negated errno value is returned on failure.
+
+.. c:function:: void nxmutex_lock(FAR mutex_t *mutex)
+
+ This function attempts to lock the mutex referenced by 'mutex'. The
+ mutex is implemented with a semaphore, so if the semaphore value is
+ (<=) zero, then the calling task will not return until it successfully
+ acquires the lock.
+
+ :param mutex: mutex descriptor.
+
+ :return:
+ Zero(OK) is returned on success.A negated errno value is returned on failure.
+
+.. c:function:: void nxmutex_trylock(FAR mutex_t *mutex)
+
+ This function locks the mutex only if the mutex is currently not locked.
+ If the mutex has been locked already, the call returns without blocking.
+
+ :param mutex: mutex descriptor.
+
+ :return:
+ Zero(OK) is returned on success.A negated errno value is returned on failure.
+ Possible returned errors:
+
+ EINVAL - Invalid attempt to lock the mutex
+ EAGAIN - The mutex is not available.
+
+.. c:function:: void nxmutex_is_locked(FAR mutex_t *mutex)
+
+ This function get the lock state the mutex referenced by 'mutex'.
+
+ :param mutex: mutex descriptor.
+
+ :return:
+ if mutex is locked will return `ture`. if not will reutrn `false`
+
+.. c:function:: void nxmutex_unlock(FAR mutex_t *mutex)
+
+ This function attempts to unlock the mutex referenced by 'mutex'.
+
+ :param mutex: mutex descriptor.
+
+ :return:
+ Zero(OK) is returned on success.A negated errno value is returned on failure.
+
+.. c:function:: void nxmutex_reset(FAR mutex_t *mutex)
+
+ This function rset mutex states by 'mutex'.
Review Comment:
```suggestion
This function resets mutex states by 'mutex'.
```
##########
Documentation/reference/os/mutex.rst:
##########
@@ -0,0 +1,130 @@
+=====================
+Mutual Exclusion lock
+=====================
+
+nxmutex
+=======
+
+Use `nxmutex` prefixed api for protect resources.In fact, nxmutex is implemented
+based on nxsem,The difference between nxmutex and nxsem is that nxmutex supports
+priority inheritance by default, nxsem not support priority inheritance by default
+
+usually usage:
+
+call nxmutex_init() for driver,when two task will use driver, Their timing will be
+============== ==============
+taskA taskB
+=============== ==============
+nxmutex_lock() nxmutex_lock()
+=============== ==============
+get lock runing wait for lock
+================ ==============
+nxmutex_unlock() wait for lock
+================ ===============
+ get lock runing
+================ ===============
+ nxmutex_unlock
+
+Priority inheritance
+====================
+
+if `CONFIG_PRIORITY_INHERITANCE` is chosen, The priority of the task holding the mutex
+may be changed.
+This is a example:
+
+ There are three tasks.Their priorities are high, medium, and low.
+ We refer to them as `Htask` `Mtask` `Ltask`
+
+ `Htask` and `Ltask` will hold the same mutex. `Mtask` does not hold mutex
+
+if `CONFIG_PRIORITY_INHERITANCE` is not chosen, task runing order
+ #. `Ltask` hold a mutex first
+ #. Then `Htask` runing, `Htask` can't hold the mutex,so wait
+ #. Then `Mtask` runing, because `Mtask` priority higher than `Ltask`.
+ #. When `Mtask` finish, `Ltask` will start running.
+ #. When `Ltask` finish, `Htask` will start running.
+
+From the above process, we can see that the medium-priority tasks run ahead of
+the high-priority tasks, which is unacceptable.
+
+if `CONFIG_PRIORITY_INHERITANCE` is chosen, task runing order
+ #. `Ltask` hold a mutex first.
+ #. Then `Htask` runing, `Htask` can't hold the mutex, then boost the priority of `Ltask`
+to be the same as `Htask`.
+ #. Because `Ltask` priority is higher than `Mtask`,so `Mtask` not runing.
Review Comment:
```suggestion
#. Because `Ltask` priority is higher than `Mtask`,so `Mtask` not running.
```
##########
Documentation/reference/os/mutex.rst:
##########
@@ -0,0 +1,130 @@
+=====================
+Mutual Exclusion lock
+=====================
+
+nxmutex
+=======
+
+Use `nxmutex` prefixed api for protect resources.In fact, nxmutex is implemented
+based on nxsem,The difference between nxmutex and nxsem is that nxmutex supports
+priority inheritance by default, nxsem not support priority inheritance by default
+
+usually usage:
+
+call nxmutex_init() for driver,when two task will use driver, Their timing will be
+============== ==============
+taskA taskB
+=============== ==============
+nxmutex_lock() nxmutex_lock()
+=============== ==============
+get lock runing wait for lock
+================ ==============
+nxmutex_unlock() wait for lock
+================ ===============
+ get lock runing
+================ ===============
+ nxmutex_unlock
+
+Priority inheritance
+====================
+
+if `CONFIG_PRIORITY_INHERITANCE` is chosen, The priority of the task holding the mutex
+may be changed.
+This is a example:
+
+ There are three tasks.Their priorities are high, medium, and low.
+ We refer to them as `Htask` `Mtask` `Ltask`
+
+ `Htask` and `Ltask` will hold the same mutex. `Mtask` does not hold mutex
+
+if `CONFIG_PRIORITY_INHERITANCE` is not chosen, task runing order
+ #. `Ltask` hold a mutex first
+ #. Then `Htask` runing, `Htask` can't hold the mutex,so wait
+ #. Then `Mtask` runing, because `Mtask` priority higher than `Ltask`.
+ #. When `Mtask` finish, `Ltask` will start running.
+ #. When `Ltask` finish, `Htask` will start running.
+
+From the above process, we can see that the medium-priority tasks run ahead of
+the high-priority tasks, which is unacceptable.
+
+if `CONFIG_PRIORITY_INHERITANCE` is chosen, task runing order
+ #. `Ltask` hold a mutex first.
+ #. Then `Htask` runing, `Htask` can't hold the mutex, then boost the priority of `Ltask`
Review Comment:
```suggestion
#. Then `Htask` running, `Htask` can't hold the mutex, then boost the priority of `Ltask`
```
##########
Documentation/reference/os/mutex.rst:
##########
@@ -0,0 +1,130 @@
+=====================
+Mutual Exclusion lock
+=====================
+
+nxmutex
+=======
+
+Use `nxmutex` prefixed api for protect resources.In fact, nxmutex is implemented
+based on nxsem,The difference between nxmutex and nxsem is that nxmutex supports
+priority inheritance by default, nxsem not support priority inheritance by default
+
+usually usage:
+
+call nxmutex_init() for driver,when two task will use driver, Their timing will be
+============== ==============
+taskA taskB
+=============== ==============
+nxmutex_lock() nxmutex_lock()
+=============== ==============
+get lock runing wait for lock
+================ ==============
+nxmutex_unlock() wait for lock
+================ ===============
+ get lock runing
+================ ===============
+ nxmutex_unlock
+
+Priority inheritance
+====================
+
+if `CONFIG_PRIORITY_INHERITANCE` is chosen, The priority of the task holding the mutex
+may be changed.
+This is a example:
+
+ There are three tasks.Their priorities are high, medium, and low.
+ We refer to them as `Htask` `Mtask` `Ltask`
+
+ `Htask` and `Ltask` will hold the same mutex. `Mtask` does not hold mutex
+
+if `CONFIG_PRIORITY_INHERITANCE` is not chosen, task runing order
+ #. `Ltask` hold a mutex first
+ #. Then `Htask` runing, `Htask` can't hold the mutex,so wait
+ #. Then `Mtask` runing, because `Mtask` priority higher than `Ltask`.
+ #. When `Mtask` finish, `Ltask` will start running.
+ #. When `Ltask` finish, `Htask` will start running.
+
+From the above process, we can see that the medium-priority tasks run ahead of
+the high-priority tasks, which is unacceptable.
+
+if `CONFIG_PRIORITY_INHERITANCE` is chosen, task runing order
+ #. `Ltask` hold a mutex first.
+ #. Then `Htask` runing, `Htask` can't hold the mutex, then boost the priority of `Ltask`
+to be the same as `Htask`.
+ #. Because `Ltask` priority is higher than `Mtask`,so `Mtask` not runing.
+ #. When 'Ltask' finish, `Htask` will start runing.
+ #. When `Htask` finish, `Mtask` will start runing.
Review Comment:
```suggestion
#. When `Htask` finish, `Mtask` will start running.
```
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #5070: libc/semaphore:sem_init change defult protocol
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #5070:
URL: https://github.com/apache/incubator-nuttx/pull/5070
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #5070: libc/semaphore:sem_init change defult protocol
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #5070:
URL: https://github.com/apache/incubator-nuttx/pull/5070#discussion_r997983175
##########
drivers/input/tsc2007.c:
##########
@@ -1230,11 +1230,11 @@ int tsc2007_register(FAR struct i2c_master_s *dev,
nxsem_init(&priv->devsem, 0, 1); /* Initialize device structure semaphore */
nxsem_init(&priv->waitsem, 0, 0); /* Initialize pen event wait semaphore */
- /* The event wait semaphore is used for signaling and, hence, should not
- * have priority inheritance enabled.
+ /* The event dev semaphore is used for lock, hence,
+ * should have priority inheritance enabled.
*/
- nxsem_set_protocol(&priv->waitsem, SEM_PRIO_NONE);
+ nxsem_set_protocol(&priv->devsem, SEM_PRIO_INHERIT);
Review Comment:
revert the change, let's do all change in https://github.com/apache/incubator-nuttx/pull/7344
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #5070: libc/semaphore:sem_init change defult protocol
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #5070:
URL: https://github.com/apache/incubator-nuttx/pull/5070#discussion_r999580260
##########
include/nuttx/mutex.h:
##########
@@ -98,6 +98,7 @@ static inline int nxmutex_init(FAR mutex_t *mutex)
return _SEM_ERRVAL(ret);
}
+ _SEM_SETPROTOCOL(mutex, PRIOINHERIT_FLAGS_ENABLE);
Review Comment:
```suggestion
_SEM_SETPROTOCOL(mutex, SEM_PRIO_INHERIT);
```
##########
arch/arm/src/kinetis/kinetis_i2c.c:
##########
@@ -191,7 +191,7 @@ static struct kinetis_i2cdev_s g_i2c0_dev =
.config = &kinetis_i2c0_config,
.refs = 0,
.lock = NXMUTEX_INITIALIZER,
- .wait = NXSEM_INITIALIZER(0, PRIOINHERIT_FLAGS_DISABLE),
+ .wait = SEM_INITIALIZER(0),
Review Comment:
I still think that we need to use `NXSEM_INITIALIZER(0, 0)` for kernel code, just from style point of view, but I will not block PR because of this.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #5070: libc/semaphore:sem_init change defult protocol
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #5070:
URL: https://github.com/apache/incubator-nuttx/pull/5070#discussion_r999647622
##########
arch/arm/src/kinetis/kinetis_i2c.c:
##########
@@ -191,7 +191,7 @@ static struct kinetis_i2cdev_s g_i2c0_dev =
.config = &kinetis_i2c0_config,
.refs = 0,
.lock = NXMUTEX_INITIALIZER,
- .wait = NXSEM_INITIALIZER(0, PRIOINHERIT_FLAGS_DISABLE),
+ .wait = SEM_INITIALIZER(0),
Review Comment:
But, it isn't a issue to use a macro or function which doesn't start with NX or nx prefix, if the the function/macro doesn't touch errno or other per task data(e.g. file handle).
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org