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

posix: options: mlock: do not enable for x86 with userspace #83650

Conversation

cfriedt
Copy link
Member

@cfriedt cfriedt commented Jan 7, 2025

Oddly, demand paging does not seem to work wth 32-bit x86 and userspace, so do not imply demand paging in that case.

Forked from #83303

Oddly, demand paging does not seem to work wth 32-bit x86 and
userspace, so do not imply demand paging in that case.

Signed-off-by: Chris Friedt <[email protected]>
@zephyrbot zephyrbot added size: XS A PR changing only a single line of code area: POSIX POSIX API Library labels Jan 7, 2025
@zephyrbot zephyrbot requested a review from ycsin January 7, 2025 13:44
@kartben kartben requested review from npitre and dcpleung January 7, 2025 17:17
@dcpleung
Copy link
Member

dcpleung commented Jan 7, 2025

Is there anyway to reproduce the issue? I don't see any tests calling mlock() directly.

@cfriedt
Copy link
Member Author

cfriedt commented Jan 7, 2025

Is there anyway to reproduce the issue? I don't see any tests calling mlock() directly.

@dcpleung - in the linked PR for XSI_REALTIME.

The commit that should enable tests is the second-last one, IIRC, that uncomments some options.

I should have added error info in the commit message.

@dcpleung
Copy link
Member

dcpleung commented Jan 7, 2025

Ahh... the issue is that some init functions and data must be present in memory before demand paging is initialized. Or else these functions and data would not be in memory to be used, hence the page faults. Here are some quick bits to be added for xsi_thread_ext to run with #83303:

diff --git a/lib/posix/options/barrier.c b/lib/posix/options/barrier.c
index 7e2598b1bde..d68bcaef37c 100644
--- a/lib/posix/options/barrier.c
+++ b/lib/posix/options/barrier.c
@@ -19,7 +19,9 @@ struct posix_barrier {
 	uint32_t count;
 };
 
+__pinned_bss
 static struct posix_barrier posix_barrier_pool[CONFIG_MAX_PTHREAD_BARRIER_COUNT];
+
 SYS_BITARRAY_DEFINE_STATIC(posix_barrier_bitarray, CONFIG_MAX_PTHREAD_BARRIER_COUNT);
 
 /*
@@ -195,6 +197,7 @@ int pthread_barrierattr_destroy(pthread_barrierattr_t *attr)
 	return 0;
 }
 
+__pinned_func
 static int pthread_barrier_pool_init(void)
 {
 	int err;
diff --git a/lib/posix/options/cond.c b/lib/posix/options/cond.c
index f94c2ea7a3b..648988b71ae 100644
--- a/lib/posix/options/cond.c
+++ b/lib/posix/options/cond.c
@@ -17,7 +17,9 @@ LOG_MODULE_REGISTER(pthread_cond, CONFIG_PTHREAD_COND_LOG_LEVEL);
 
 int64_t timespec_to_timeoutms(const struct timespec *abstime);
 
+__pinned_bss
 static struct k_condvar posix_cond_pool[CONFIG_MAX_PTHREAD_COND_COUNT];
+
 SYS_BITARRAY_DEFINE_STATIC(posix_cond_bitarray, CONFIG_MAX_PTHREAD_COND_COUNT);
 
 /*
@@ -209,6 +211,7 @@ int pthread_cond_destroy(pthread_cond_t *cvar)
 	return 0;
 }
 
+__pinned_func
 static int pthread_cond_pool_init(void)
 {
 	int err;
diff --git a/lib/posix/options/mutex.c b/lib/posix/options/mutex.c
index cd320a969dc..6990d139efd 100644
--- a/lib/posix/options/mutex.c
+++ b/lib/posix/options/mutex.c
@@ -29,7 +29,9 @@ static const struct pthread_mutexattr def_attr = {
 	.type = PTHREAD_MUTEX_DEFAULT,
 };
 
+__pinned_bss
 static struct k_mutex posix_mutex_pool[CONFIG_MAX_PTHREAD_MUTEX_COUNT];
+
 static uint8_t posix_mutex_type[CONFIG_MAX_PTHREAD_MUTEX_COUNT];
 SYS_BITARRAY_DEFINE_STATIC(posix_mutex_bitarray, CONFIG_MAX_PTHREAD_MUTEX_COUNT);
 
@@ -451,6 +453,7 @@ int pthread_mutexattr_setprioceiling(pthread_mutexattr_t *attr, int prioceiling)
 
 #endif /* CONFIG_POSIX_THREAD_PRIO_PROTECT */
 
+__pinned_func
 static int pthread_mutex_pool_init(void)
 {
 	int err;
diff --git a/lib/posix/options/pthread.c b/lib/posix/options/pthread.c
index c54a061f9e7..64014fdeb3c 100644
--- a/lib/posix/options/pthread.c
+++ b/lib/posix/options/pthread.c
@@ -83,11 +83,13 @@ BUILD_ASSERT(CONFIG_POSIX_PTHREAD_ATTR_STACKSIZE_BITS + CONFIG_POSIX_PTHREAD_ATT
 
 int64_t timespec_to_timeoutms(const struct timespec *abstime);
 static void posix_thread_recycle(void);
+__pinned_data
 static sys_dlist_t posix_thread_q[] = {
 	SYS_DLIST_STATIC_INIT(&posix_thread_q[POSIX_THREAD_READY_Q]),
 	SYS_DLIST_STATIC_INIT(&posix_thread_q[POSIX_THREAD_RUN_Q]),
 	SYS_DLIST_STATIC_INIT(&posix_thread_q[POSIX_THREAD_DONE_Q]),
 };
+__pinned_bss
 static struct posix_thread posix_thread_pool[CONFIG_MAX_PTHREAD_COUNT];
 static SYS_SEM_DEFINE(pthread_pool_lock, 1, 1);
 static int pthread_concurrency;
@@ -1536,6 +1538,7 @@ int pthread_sigmask(int how, const sigset_t *ZRESTRICT set, sigset_t *ZRESTRICT
 	return ret;
 }
 
+__pinned_func
 static int posix_thread_pool_init(void)
 {
 	ARRAY_FOR_EACH_PTR(posix_thread_pool, th) {

@cfriedt
Copy link
Member Author

cfriedt commented Jan 8, 2025

@dcpleung - if you make a PR, I'll approve :-)

@cfriedt cfriedt marked this pull request as draft January 8, 2025 02:49
@cfriedt
Copy link
Member Author

cfriedt commented Jan 8, 2025

Will keep this in draft until other pr is made

@dcpleung
Copy link
Member

dcpleung commented Jan 8, 2025

#83708 to pin those symbols in place.

@cfriedt cfriedt closed this Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: POSIX POSIX API Library size: XS A PR changing only a single line of code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants