xiaoxiang781216 commented on code in PR #16194:
URL: https://github.com/apache/nuttx/pull/16194#discussion_r2040887759
##########
include/semaphore.h:
##########
@@ -104,8 +104,16 @@ struct semholder_s
struct sem_s
{
- volatile int32_t semcount; /* >0 -> Num counts available */
- /* <0 -> Num tasks waiting for semaphore */
+ union
+ {
+ volatile int32_t semcount; /* >0 -> Num counts available */
+ /* <0 -> Num tasks waiting for semaphore
*/
+ volatile uint32_t mholder; /* == NXMUTEX_NO_HOLDER -> mutex has no
holder */
Review Comment:
why not keep the type as int32_t? so we can check mholder < 0 mean blocking
##########
include/nuttx/semaphore.h:
##########
@@ -45,23 +45,43 @@
/* semcount, flags, waitlist, hhead */
# define NXSEM_INITIALIZER(c, f) \
- {(c), (f), SEM_WAITLIST_INITIALIZER, NULL}
+ {{c}, (f), SEM_WAITLIST_INITIALIZER, NULL}
# else
/* semcount, flags, waitlist, holder[2] */
# define NXSEM_INITIALIZER(c, f) \
- {(c), (f), SEM_WAITLIST_INITIALIZER, SEMHOLDER_INITIALIZER}
+ {{(c)}, (f), SEM_WAITLIST_INITIALIZER, SEMHOLDER_INITIALIZER}
# endif
#else /* CONFIG_PRIORITY_INHERITANCE */
/* semcount, flags, waitlist */
# define NXSEM_INITIALIZER(c, f) \
- {(c), (f), SEM_WAITLIST_INITIALIZER}
+ {{(c)}, (f), SEM_WAITLIST_INITIALIZER}
#endif /* CONFIG_PRIORITY_INHERITANCE */
-/* Macro to retrieve sem count */
+/* Macros to retrieve sem count and to check if nxsem is mutex */
-#define NXSEM_COUNT(s) ((FAR atomic_t *)&(s)->semcount)
+#define NXSEM_COUNT(s) ((FAR atomic_t *)&(s)->val.semcount)
+#define NXSEM_IS_MUTEX(s) (((s)->flags & SEM_TYPE_MUTEX) != 0)
+
+/* Mutex related helper macros */
+
+#define NXMUTEX_BLOCKS_BIT (((uint32_t)1) << 31)
Review Comment:
let's check `mholder >= 0` and remove NXMUTEX_BLOCKS_BIT?
##########
include/semaphore.h:
##########
@@ -104,8 +104,16 @@ struct semholder_s
struct sem_s
{
- volatile int32_t semcount; /* >0 -> Num counts available */
Review Comment:
since we always use macro access semcount/mholder, why not remove the union?
##########
include/nuttx/semaphore.h:
##########
@@ -45,23 +45,43 @@
/* semcount, flags, waitlist, hhead */
# define NXSEM_INITIALIZER(c, f) \
- {(c), (f), SEM_WAITLIST_INITIALIZER, NULL}
+ {{c}, (f), SEM_WAITLIST_INITIALIZER, NULL}
# else
/* semcount, flags, waitlist, holder[2] */
# define NXSEM_INITIALIZER(c, f) \
- {(c), (f), SEM_WAITLIST_INITIALIZER, SEMHOLDER_INITIALIZER}
+ {{(c)}, (f), SEM_WAITLIST_INITIALIZER, SEMHOLDER_INITIALIZER}
# endif
#else /* CONFIG_PRIORITY_INHERITANCE */
/* semcount, flags, waitlist */
# define NXSEM_INITIALIZER(c, f) \
- {(c), (f), SEM_WAITLIST_INITIALIZER}
+ {{(c)}, (f), SEM_WAITLIST_INITIALIZER}
#endif /* CONFIG_PRIORITY_INHERITANCE */
-/* Macro to retrieve sem count */
+/* Macros to retrieve sem count and to check if nxsem is mutex */
-#define NXSEM_COUNT(s) ((FAR atomic_t *)&(s)->semcount)
+#define NXSEM_COUNT(s) ((FAR atomic_t *)&(s)->val.semcount)
+#define NXSEM_IS_MUTEX(s) (((s)->flags & SEM_TYPE_MUTEX) != 0)
+
+/* Mutex related helper macros */
+
+#define NXMUTEX_BLOCKS_BIT (((uint32_t)1) << 31)
+
+#define NXMUTEX_NO_HOLDER ((uint32_t)0x7ffffffe)
+#define NXMUTEX_RESET ((uint32_t)0x7fffffff)
+
+/* Macro to retrieve mutex's atomic holder's ptr */
+
+#define NXMUTEX_HOLDER(s) ((FAR atomic_t *)&(s)->val.mholder)
+
+/* Check if holder value (TID) is not NO_HOLDER or RESET */
+
+#define NXMUTEX_ACQUIRED(h) (!(((h) & NXMUTEX_NO_HOLDER) ==
NXMUTEX_NO_HOLDER))
+
+/* Check if mutex is acquired and blocks some other task */
+
+#define NXMUTEX_BLOCKS(h) (NXMUTEX_ACQUIRED(h) && ((h) &
NXMUTEX_BLOCKS_BIT))
Review Comment:
why need check NXMUTEX_ACQUIRED?
##########
include/nuttx/mutex.h:
##########
@@ -36,10 +36,11 @@
* Pre-processor Definitions
****************************************************************************/
-#define NXMUTEX_NO_HOLDER ((pid_t)-1)
-#define NXMUTEX_INITIALIZER {NXSEM_INITIALIZER(1, SEM_TYPE_MUTEX | \
- SEM_PRIO_INHERIT), NXMUTEX_NO_HOLDER}
-#define NXRMUTEX_INITIALIZER {NXMUTEX_INITIALIZER, 0}
+#define NXMUTEX_INITIALIZER { \
+ NXSEM_INITIALIZER(NXMUTEX_NO_HOLDER, SEM_TYPE_MUTEX | SEM_PRIO_INHERIT), \
+ NXMUTEX_NO_HOLDER}
Review Comment:
why not remove holder at line 52?
##########
include/nuttx/semaphore.h:
##########
@@ -45,23 +45,43 @@
/* semcount, flags, waitlist, hhead */
# define NXSEM_INITIALIZER(c, f) \
- {(c), (f), SEM_WAITLIST_INITIALIZER, NULL}
+ {{c}, (f), SEM_WAITLIST_INITIALIZER, NULL}
# else
/* semcount, flags, waitlist, holder[2] */
# define NXSEM_INITIALIZER(c, f) \
- {(c), (f), SEM_WAITLIST_INITIALIZER, SEMHOLDER_INITIALIZER}
+ {{(c)}, (f), SEM_WAITLIST_INITIALIZER, SEMHOLDER_INITIALIZER}
# endif
#else /* CONFIG_PRIORITY_INHERITANCE */
/* semcount, flags, waitlist */
# define NXSEM_INITIALIZER(c, f) \
- {(c), (f), SEM_WAITLIST_INITIALIZER}
+ {{(c)}, (f), SEM_WAITLIST_INITIALIZER}
#endif /* CONFIG_PRIORITY_INHERITANCE */
-/* Macro to retrieve sem count */
+/* Macros to retrieve sem count and to check if nxsem is mutex */
-#define NXSEM_COUNT(s) ((FAR atomic_t *)&(s)->semcount)
+#define NXSEM_COUNT(s) ((FAR atomic_t *)&(s)->val.semcount)
+#define NXSEM_IS_MUTEX(s) (((s)->flags & SEM_TYPE_MUTEX) != 0)
+
+/* Mutex related helper macros */
+
+#define NXMUTEX_BLOCKS_BIT (((uint32_t)1) << 31)
+
+#define NXMUTEX_NO_HOLDER ((uint32_t)0x7ffffffe)
Review Comment:
should we continue use NXSEM_ prefix to keep the consistent in this file?
--
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-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
