Add futex support #422

kamilaboba123 · 2025-07-17T08:57:22Z

Add library support for futex syscalls and synchronization mechanisms based on them.

Description

Add declarations for phFutexWait() and phFutexWakeup(),
implement a simple mutex - mymutex_t

Motivation and Context

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Chore (refactoring, style fixes, git/CI config, submodule management, no code logic changes)

How Has This Been Tested?

Already covered by automatic testing.
New test added: (add PR link here).
Tested by hand on: (list targets here).
ia32-generic-qemu

Checklist:

My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing linter checks and tests passed.
My changes generate no new compilation warnings for any of the targets.

Special treatment

This PR needs additional PRs to work (list the PRs, preferably in merge-order).
I will merge this PR by myself when appropriate.

gemini-code-assist

Summary of Changes

Hello @kamilaboba123, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces fundamental support for futex-based synchronization mechanisms into the library. It provides direct API calls for futex wait and wakeup operations and demonstrates their usage by implementing a simple userspace mutex. This change lays the groundwork for more efficient and robust concurrency control within the system.

Highlights

New Futex API: I've introduced a new header file, include/sys/futex.h, which declares the phFutexWait and phFutexWakeup functions. These functions provide the core API for interacting with futex (Fast Userspace Mutex) syscalls, enabling low-level synchronization primitives.
Simple Mutex Implementation: I've added a basic mutex implementation, mymutex_t, defined in include/sys/mymutex.h and implemented in sys/mymutex.c. This mymutex_t leverages the newly exposed phFutexWait and phFutexWakeup functions, along with atomic operations, to provide mm_lock and mm_unlock functionality.
Build System Integration: The sys/Makefile has been updated to include the new mymutex.o object file, ensuring that the mymutex implementation is compiled and linked into the system library.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

The pull request introduces library support for futex syscalls and a simple mutex implementation based on them. There are critical type-safety issues in the mymutex implementation due to mismatches between int, unsigned int, and the uint32_t expected by the futex functions. These issues could lead to undefined behavior and portability problems. Additionally, a header file is missing a necessary include. Recommendations focus on correcting these type issues by consistently using atomic_uint32_t and uint32_t, and on fixing the missing include.

include/sys/mymutex.h

sys/mymutex.c

include/sys/mymutex.h

github-actions · 2025-07-17T09:11:23Z

Unit Test Results

8 039 tests ±0 7 515 ✅ ±0 40m 37s ⏱️ -20s
479 suites ±0 524 💤 ±0
1 files ±0 0 ❌ ±0

Results for commit 46f9f67. ± Comparison against base commit 911306b.

♻️ This comment has been updated with latest results.

include/sys/futex.h

include/sys/mycond.h

include/sys/mymutex.h

include/sys/mycond.h

include/sys/mymutex.h

sys/mycond.c

sys/mymutex.c

include/sys/interrupt.h

ziemleszcz · 2025-08-11T07:36:59Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces support for futex-based synchronization primitives, replacing the previous implementations of mutexes and condition variables. The changes are extensive, touching header files and the core threading logic. My review has identified several critical issues, including a buggy spinlock implementation, an incorrect handle calculation in mutexTry, and a race condition in resourceDestroy. I've also pointed out some medium-severity issues related to code maintainability, error handling, and potential performance problems. These issues should be addressed to ensure the correctness and robustness of the new synchronization mechanisms.

sys/threads.c

gemini-code-assist · 2025-08-11T07:39:21Z

sys/threads.c

+}
+
+
+/* TODO: Recursive mutexes are not handled currently */


This TODO correctly notes that recursive mutexes are not handled. However, the mutex_t struct (defined at line 79) contains a recursive_counter field which is never used. This can be misleading for future developers. Please consider either implementing recursive mutexes or removing the recursive_counter field to avoid confusion.

sys/threads.c

gemini-code-assist · 2025-08-11T07:39:21Z

sys/threads.c

+}
+
+
+/* TODO: this code probably should be in sys/interrupt.c, but this is ok for now */


As this TODO comment suggests, the interrupt function seems misplaced in sys/threads.c. For better code organization and maintainability, it should be moved to sys/interrupt.c.

include/sys/threads.h

sys/threads.c

ziemleszcz · 2025-08-18T12:22:15Z

include/sys/interrupt.h



 #include <sys/types.h>
+#include <stdint.h>


Is it needed?

sys/threads.c

ziemleszcz · 2025-08-18T12:44:11Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces support for futexes and implements mutexes and condition variables based on them. The implementation is a good step forward but contains a critical bug in mutexTry that needs to be fixed. Additionally, there are some design considerations regarding the use of fixed-size resource pools that may impact scalability, and a potential unintended change in the default behavior of condition variables. Overall, the changes are valuable but require attention to these points before merging.

sys/threads.c

gemini-code-assist · 2025-08-18T12:45:47Z

sys/threads.c

+#define MUTEXES_SIZE 100
+#define CONDS_SIZE   100
+
+
+struct {
+	spinlock_t spinlock;
+	mutex_t items[MUTEXES_SIZE];
+} mutexes = { 0 };
+
+
+struct {
+	spinlock_t spinlock;
+	cond_t items[CONDS_SIZE];
+} conds = { 0 };


The use of fixed-size global pools for mutexes and condition variables (100 of each) introduces a hard limit on the number of synchronization objects the system can create. This could be a problem for applications that require a large number of these objects.

Consider the following improvements:

Dynamic Allocation: Use malloc to allocate mutex_t and cond_t objects on demand. This would remove the hard limit and make the system more scalable.

Configurability: If static allocation is a requirement, consider making the pool sizes (MUTEXES_SIZE, CONDS_SIZE) configurable at build time.

Efficient Allocation: The current reserveMutex and reserveCond functions use a linear scan to find a free slot. For larger pools, a more efficient mechanism like a free list would improve performance.

gemini-code-assist · 2025-08-18T12:45:47Z

sys/threads.c

+int condCreate(handle_t *h)
+{
+	return condCreateWithAttr(h, NULL);
+}


The new implementation of condCreate changes the default clock type for condition variables. The previous implementation used PH_CLOCK_RELATIVE as the default. The new version calls condCreateWithAttr with NULL attributes, which results in the clock type being initialized to 0. If 0 is not equivalent to PH_CLOCK_RELATIVE, this constitutes a silent change in behavior for any code that uses condWait with a timeout and relies on the default clock type. To maintain backward compatibility, the previous default behavior should be preserved.

int condCreate(handle_t *h) { static const struct condAttr defaultAttr = { .clock = PH_CLOCK_RELATIVE }; return condCreateWithAttr(h, &defaultAttr); }

gemini-code-assist bot reviewed Jul 17, 2025

View reviewed changes

kamilaboba123 self-assigned this Jul 17, 2025

gemini-code-assist bot reviewed Jul 17, 2025

View reviewed changes

include/sys/mymutex.h Outdated Show resolved Hide resolved

sys/mymutex.c Outdated Show resolved Hide resolved

include/sys/mymutex.h Outdated Show resolved Hide resolved

kamilaboba123 force-pushed the kamilaboba123/RTOS-1074 branch 2 times, most recently from 00e1235 to 46f9f67 Compare July 21, 2025 12:47

kamilaboba123 force-pushed the kamilaboba123/RTOS-1074 branch 2 times, most recently from 2907b16 to 5620860 Compare August 1, 2025 12:18

ziemleszcz self-requested a review August 4, 2025 10:30

ziemleszcz reviewed Aug 4, 2025

View reviewed changes

kamilaboba123 force-pushed the kamilaboba123/RTOS-1074 branch 7 times, most recently from d94449b to e81822e Compare August 8, 2025 12:51

gemini-code-assist bot reviewed Aug 11, 2025

View reviewed changes

ziemleszcz reviewed Aug 11, 2025

View reviewed changes

include/sys/threads.h Outdated Show resolved Hide resolved

sys/threads.c Show resolved Hide resolved

sys/threads.c Outdated Show resolved Hide resolved

sys/threads.c Outdated Show resolved Hide resolved

kamilaboba123 force-pushed the kamilaboba123/RTOS-1074 branch from e81822e to fe1d188 Compare August 12, 2025 12:51

github-actions bot reviewed Aug 12, 2025

View reviewed changes

sys/threads.c Outdated Show resolved Hide resolved

sys/threads.c Outdated Show resolved Hide resolved

sys/threads.c Outdated Show resolved Hide resolved

kamilaboba123 force-pushed the kamilaboba123/RTOS-1074 branch 3 times, most recently from 017c575 to 99fa56e Compare August 14, 2025 16:44

ziemleszcz reviewed Aug 18, 2025

View reviewed changes

gemini-code-assist bot reviewed Aug 18, 2025

View reviewed changes

kamilaboba123 force-pushed the kamilaboba123/RTOS-1074 branch 2 times, most recently from 74eddbb to 071b61c Compare August 21, 2025 09:25

futex: Futex based synchronization

a816ad9

kamilaboba123 force-pushed the kamilaboba123/RTOS-1074 branch from 071b61c to a816ad9 Compare August 25, 2025 16:40

		}


		/* TODO: this code probably should be in sys/interrupt.c, but this is ok for now */

Add futex support #422

Are you sure you want to change the base?

Add futex support #422

Uh oh!

Conversation

kamilaboba123 commented Jul 17, 2025

Description

Motivation and Context

Types of changes

How Has This Been Tested?

Checklist:

Special treatment

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jul 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Test Results

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ziemleszcz commented Aug 11, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist bot Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist bot Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ziemleszcz Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ziemleszcz commented Aug 18, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

gemini-code-assist bot Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Aug 18, 2025

github-actions bot commented Jul 17, 2025 •

edited

Loading