Repository navigation
nuttx: fix some definitions (pollfd, maximum struct sizes) and add extra semaphore methods - #5555
Conversation
4329f7b to
637ea0e
Compare
|
@no1wudi could you take a look? |
Corresponding libc PR: rust-lang/libc#5555
| const __PTHREAD_ATTR_SIZE__: usize = 5; | ||
| const __PTHREAD_MUTEX_SIZE__: usize = 9; | ||
| const __PTHREAD_COND_SIZE__: usize = 7; | ||
| const __PTHREAD_MUTEX_SIZE__: usize = 14; // Assumes CONFIG_LIBC_MUTEX_BACKTRACE=0 |
There was a problem hiding this comment.
Which CONFIG_* option does this value correspond to?
There was a problem hiding this comment.
Based on the maximum, like this:
struct pthread_mutex_s
{
#ifndef CONFIG_PTHREAD_MUTEX_UNSAFE (=n)
FAR struct pthread_mutex_s *flink;
uint8_t flags;
#endif
#ifdef CONFIG_PTHREAD_MUTEX_TYPES (=y)
uint8_t type; /* Type of the mutex. See PTHREAD_MUTEX_* definitions */
rmutex_t mutex; /* Mutex underlying the implementation of the mutex */
#endif
};
struct rmutex_s
{
mutex_t mutex;
unsigned int count;
};
struct mutex_s
{
sem_t sem;
};
struct sem_s
{
union
{
volatile int32_t semcount; /* >0 -> Num counts available */
/* <0 -> Num tasks waiting for semaphore */
volatile uint32_t mholder; /* == NXSEM_NO_MHOLDER -> mutex has no holder */
/* == NXSEM_RESET -> mutex has been reset */
/* Otherwise: */
/* bits[30:0]: TID of the current holder */
/* bit [31]: Mutex is blocking some task */
} val;
uint8_t flags; /* See SEM_PRIO_* definitions */
dq_queue_t waitlist;
#ifdef CONFIG_CUSTOM_SEMAPHORE_MAXVALUE (=y)
int32_t maxvalue;
#endif
#ifdef CONFIG_PRIORITY_INHERITANCE (=y)
# if CONFIG_SEM_PREALLOCHOLDERS > 0 (= 0)
# else
struct semholder_s holder; /* Slot for old and new holder */
# endif
#endif
#ifdef CONFIG_PRIORITY_PROTECT (=y)
uint8_t ceiling; /* The priority ceiling owned by mutex */
uint8_t saved; /* The saved priority of thread before boost */
#endif
};
struct semholder_s
{
#if CONFIG_SEM_PREALLOCHOLDERS > 0 (=0)
#endif
FAR struct semholder_s *tlink; /* List of task held semaphores */
FAR struct sem_s *sem; /* The corresponding semaphore */
FAR struct tcb_s *htcb; /* The corresponding TCB */
int32_t counts; /* Number of counts owned by this holder */
};
struct dq_queue_s
{
FAR dq_entry_t *head;
FAR dq_entry_t *tail;
};Double-checking made me realize that I added one too many for the sem_s (for CONFIG_SEM_PREALLOCHOLDERS=0. I updated accordingly.
There was a problem hiding this comment.
If based on max, maybe we should have a discuss on nuttx side? We still assume something like CONFIG_LIBC_MUTEX_BACKTRACE=0, it's not real "maximum"
There was a problem hiding this comment.
Since CONFIG_LIBC_MUTEX_BACKTRACE is open ended the real max is basically infinite. And since it's mostly a debug feature I added the assumption of it being set to 0 (default).
But it really doesn't matter to me, we can use whatever you prefer.
There was a problem hiding this comment.
OK, larger size is fine for me
Could you add a note to nuttx's Documentation/guides/rust.rst to reflect this change and collect feedback from NuttX side? Thanks
There was a problem hiding this comment.
I opened a PR: apache/nuttx#20408
And I also came across another issue: apache/nuttx#20407
CONFIG_SYSTEM_TIME64 is required for libc::timeval And CONFIG_LIBC_MUTEX_BACKTRACE=0 is required for rust-lang/libc#5555
CONFIG_SYSTEM_TIME64 is required for libc::time_t And CONFIG_LIBC_MUTEX_BACKTRACE=0 is required for rust-lang/libc#5555
And CONFIG_LIBC_MUTEX_BACKTRACE=0 is required for rust-lang/libc#5555
This comment has been minimized.
This comment has been minimized.
|
@rustbot author for resolving the conflict |
|
Reminder, once the PR becomes ready for a review, use |
| } | ||
|
|
||
| #[cfg(not(target_os = "horizon"))] | ||
| #[cfg(all(not(target_os = "horizon"), not(target_os = "nuttx")))] |
There was a problem hiding this comment.
Could you actually also flip flop this from all(not() ...) to not(any(...))?
And CONFIG_LIBC_MUTEX_BACKTRACE=0 is required for rust-lang/libc#5555
…pthread_rwlock_t and sem_t References: - https://github.com/apache/nuttx/blob/fa1bbce9c9bba74217501a80f4ca585e024f663d/include/pthread.h - https://github.com/apache/nuttx/blob/fa1bbce9c9bba74217501a80f4ca585e024f663d/include/nuttx/mutex.h - https://github.com/apache/nuttx/blob/fa1bbce9c9bba74217501a80f4ca585e024f663d/include/semaphore.h
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
I rebased the branch. @rustbot ready |
|
Should this be backported to libc 0.2? The fixes to struct sizes (including the added pollfd fields) seem necessary to use these types soundly, at least when allocating storage in Rust and passing pointers to C (which I’d expect to be common for all of the affected types). But I don’t know anything about nuttx, I was just rolled as reviewer for rust-lang/rust#163202 |
|
@rustbot label +stable-nominated Probably should have been, guess I just missed it in my last sweep |
Corresponding libc PR: rust-lang/libc#5555 pollfd has some extra internal fields on NuttX, so we use mem::zeroed() to initialze.
Corresponding libc PR: rust-lang/libc#5555 pollfd has some extra internal fields on NuttX, so we use mem::zeroed() to initialze.
Corresponding libc PR: rust-lang/libc#5555 pollfd has some extra internal fields on NuttX, so we use mem::zeroed() to initialze.
See individual commits for the reference to the corresponding NuttX source code.