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

sched/sem_waitirq: Move kmm_map() call to sem_wait() #366

Merged
merged 1 commit into from
Mar 13, 2025

Conversation

jlaitine
Copy link

The kernel mapping should be performed in sem_wait (thread level) as virtual memory mappings cannot be added from interrupt, at least for now.

The reason?

kmm_map() depends on mm_map_add(), which in turn uses a mutex for mutual exclusion. Using mutexes from interrupt level is not permitted.

Mapping tcb->waitobj into kernel virtual memory directly in sem_wait() makes sense, since accessing tcb->waitobj via a user virtual address can lead to unexpected results (the wrong mappings can be in place).

@@ -100,6 +100,10 @@ void nxsem_recover(FAR struct tcb_s *tcb)
*/

sem->semcount++;

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to add header to this file

The kernel mapping should be performed in sem_wait (thread level) as
virtual memory mappings cannot be added from interrupt, at least for now.

The reason?

kmm_map() depends on mm_map_add(), which in turn uses a mutex for mutual
exclusion. Using mutexes from interrupt level is not permitted.

Mapping tcb->waitobj into kernel virtual memory directly in sem_wait()
makes sense, since accessing tcb->waitobj via a user virtual address can
lead to unexpected results (the wrong mappings can be in place).
@jlaitine jlaitine merged commit e1821ab into master Mar 13, 2025
10 checks passed
@jlaitine jlaitine deleted the fix_sem_waitirq branch March 13, 2025 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants