feat(ipc/sem): add semaphore support for dragonOS by mistcoversmyeyes · Pull Request #2172 · DragonOS-Community/DragonOS

mistcoversmyeyes · 2026-08-08T11:14:05Z

Summary

Implement the x86_64 System V semaphore syscalls: semget, semctl, semop, and semtimedop.
Add semaphore-set management to IPC namespaces.
Extract and reuse common System V IPC permission checks.

Scope

Match the Linux 6.6 x86_64 ABI and observable behavior.
SEM_UNDO is out of scope and currently returns ENOSYS.

Acceptance

Valid semaphore syscall requests no longer return ENOSYS.
semop and semtimedop share consistent operation semantics.
Creation, lookup, control, removal, and permission checks match Linux behavior.
Multi-operation requests execute atomically.
Blocking operations wake correctly after value changes or IPC_RMID.
Nonblocking, timeout, signal, invalid-argument, and removed-set errors match Linux behavior.
Concurrent access avoids races, lost wake-ups, use-after-free, and resource leaks.
Existing DragonOS CI tests pass.

Testing

Added 43 System V semaphore dunitests.
QEMU guest test: 43/43 passed.
Format, Clippy, multi-architecture builds, Dunitest, and Integration Test CI passed.

fslongjin · 2026-08-29T09:54:18Z

@codex review

chatgpt-codex-connector · 2026-08-29T09:54:37Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-06T17:21:27.598847Z	`a473663`	Manual request

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fbdee927ce

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

fslongjin

Request changes: this PR establishes a useful base for System V semaphore support, but it does not yet satisfy the Linux 6.6 compatibility and concurrency-safety contract stated in #2142.

The blocking issues are:

IPC_SET permission updates can partially commit on an error, changing the owner even though the syscall returns EINVAL; the shared helper also affects SHM.
SEM_UNDO is rejected with ENOSYS and the new test codifies that incompatibility, while Linux maintains per-process/shared undo state and replays it at process exit.
A single namespace-wide spinlock protects the registry and every semaphore set, so unrelated sets are serialized; the lock also covers allocation-heavy queue simulation and scheduler wakeups.
User-controlled semaphore-set allocation is infallible and can reach the kernel panic allocation handler instead of returning ENOMEM.
SEM_STAT and SEM_STAT_ANY mask their direct table index, causing out-of-range indices to alias valid objects.

The basic syscall wiring, atomic multi-operation simulation, timeout/removal paths, and test breadth are valuable. However, the issues above are architectural or user-visible Linux semantic mismatches rather than optional refinements. Please address them, add the corresponding regression tests, and rerun the guest suite. The current Integration Test check also reports 5666 passed, 1 failed, and 180 skipped; I am not attributing that failure to this PR without further evidence, but the PR description should not claim that Integration Test passed while the check remains red.

fslongjin · 2026-08-29T16:15:32Z

+            .iter()
+            .any(|op| (op.sem_flg as u32) & SemFlags::SEM_UNDO.bits() != 0)
+        {
+            return Err(SystemError::ENOSYS);


[P1] Rejecting every SEM_UNDO operation with ENOSYS is not Linux-compatible System V semaphore behavior. Linux 6.6 maintains sem_undo/semadj state, shares the undo list for CLONE_SYSVSEM, clears adjustments on SETVAL/SETALL/IPC_RMID, and replays them from exit_sem() when a task exits. This is essential crash-recovery behavior: without it, a lock holder exiting can leave peers blocked indefinitely. Please implement the full lifecycle before treating #2142 as complete; the new test should verify Linux behavior instead of expecting ENOSYS.

Confirmed. Full SEM_UNDO lifecycle support is required for Linux 6.6 compatibility, including shared undo state for CLONE_SYSVSEM, semadj updates and limits, cleanup on SETVAL/SETALL/IPC_RMID, and replay on task exit. I am implementing this now and will replace the current ENOSYS test with lifecycle coverage. Keeping this thread open until the implementation and tests are complete.

fslongjin · 2026-08-29T16:15:32Z

+    /// SysV SHM manager (phase one: per-namespace SHM only)
    pub shm: SpinLock<ShmManager>,
+    /// SysV semaphore manager
+    pub sem: SpinLock<SemManager>,


[P1] A namespace-wide spinlock is too broad for semaphore-set state. Every operation on every set, including update_queue(), is serialized here; queue simulation allocates a HashMap, may rescan waiters quadratically, and calls Waker::wake() while this lock is held. A user can therefore stall unrelated semaphore sets in the same namespace. Please keep the manager lock limited to ID/key/quota lookup, store stable Arc<KernelSemSet> objects with per-set locking, use a non-allocating operation fast path, and collect wakeups for execution after releasing the set lock, following Linux's registry/array locking and wake_q separation.

Please keep the manager lock limited to ID/key/quota lookup, store stable Arc<KernelSemSet> objects with per-set locking, use a non-allocating operation fast path, and collect wakeups for execution after releasing the set lock.

I am currently evaluating the necessity and designing the manager/per-set lock separation. No implementation changes for this lock split have started yet.

I am also coordinating this with the latest semaphore queue work. The design will preserve the unlocked queue-growth preparation and fallible-allocation semantics from b3f637c7, rather than duplicate or replace that work. I will post the finalized scope before touching the locking code so we can avoid overlapping changes.

mistcoversmyeyes · 2026-09-02T16:04:21Z

While reviewing the queue-rescan concern, I found a separate Linux 6.6 compatibility issue in the current unified waiter queue.

Linux classifies queued requests into const operations (the entire operation group has no altering operation) and alter operations when they are enqueued: ipc/sem.c#L2127-L2153. Its update path handles wait-for-zero requests before scanning pending alter requests: ipc/sem.c#L1029-L1039. After a queued alter operation succeeds, Linux immediately checks wait-for-zero requests made satisfiable by that change before continuing with further alter operations: ipc/sem.c#L974-L992.

The current DragonOS implementation stores both classes in one VecDeque, so an older alter waiter can run before an already-satisfied const waiter and make its zero condition false again. This can produce behavior different from Linux.

Proposed minimal design for this PR:

Split the set-level queue into pending_const and pending_alter.
Classify an operation group once: all sem_op == 0 goes to pending_const; any nonzero operation goes to pending_alter.
Process ready const waiters before scanning alter waiters.
After any alter waiter changes semaphore values, return to the const phase before continuing alter processing.
Keep the existing full atomic simulation and weak-FIFO alter scan; defer per-semaphore/simple-complex indexing to a separate performance change.

This fixes the observable ordering difference without attempting to eliminate the irreducible worst-case rescan for complex multi-operation groups.

fslongjin · 2026-09-04T02:33:39Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a2d659b25f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-09-04T02:46:54Z

+        let (waiter, waker) = Waiter::new_pair();
+        let timer =
+            deadline_ticks.map(|deadline| Timer::new(TimeoutWaker::new(waker.clone()), deadline));


将等待对象延迟到确认阻塞后创建

当 semop 可立即完成时，这里仍会无条件为 Waker 分配 Arc；带非零超时的 semtimedop 还会额外分配 TimeoutWaker 和 Timer。这使无竞争的信号量快路径以及无效 ID、越界或权限失败等错误路径都承担不必要的堆分配开销；应仅在 simulate_semop 返回 Blocked 且确实需要睡眠后创建这些对象。

AGENTS.md reference: AGENTS.md:L10-L10

Useful? React with 👍 / 👎.

fslongjin · 2026-09-04T17:09:36Z

There are currently conflicts. Please rebase your code onto the latest origin/master branch and resolve the conflicts correctly according to the intended semantics. Please also review the affected changes and adjust the object abstraction and design as needed to ensure that the changes are merged correctly, rather than resolving the conflicts mechanically.

Signed-off-by: longjin <longjin@dragonos.org>

fslongjin · 2026-09-05T14:18:08Z

@codex review

Signed-off-by: longjin <longjin@dragonos.org>

fslongjin · 2026-09-05T20:15:50Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 277351ed23

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Signed-off-by: longjin <longjin@dragonos.org>

fslongjin · 2026-09-06T03:18:07Z

@codex review

chatgpt-codex-connector · 2026-09-06T03:25:25Z

Codex Review: Didn't find any major issues. Hooray!

Reviewed commit: 26adf2c068

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Signed-off-by: longjin <longjin@dragonos.org>

Split semaphore ABI, namespace management, atomic execution and wait queues into focused modules. Centralize terminal publication without rescanning known queues, and represent undo retirement with an explicit phase. Preserve syscall paths, locking and deferred reclamation. Move existing tests with their owning modules and add atomic-attempt regression coverage. Signed-off-by: longjin <longjin@dragonos.org>

fslongjin · 2026-09-06T04:52:15Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 303064c738

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Signed-off-by: longjin <longjin@dragonos.org>

fslongjin · 2026-09-06T05:23:46Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a455a7301a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Signed-off-by: longjin <longjin@dragonos.org>

fslongjin · 2026-09-06T06:34:59Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 55a48ef282

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Preserve the old IPC namespace and actor across namespace publication, then release the fs reference guard before replay. Validate copied SETALL values before checking for concurrent removal. Signed-off-by: longjin <longjin@dragonos.org>

fslongjin · 2026-09-06T07:39:43Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 506bf42a68

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Track both committed semaphore values and shared undo adjustments in the existing completion result. Preserve the result through the immediate undo path and use it for immediate scans and queued retries. Debt-only changes must still retry waiters because they can turn a blocked shared SEM_UNDO operation into ERANGE. Add user-space coverage for immediate and queued debt-only changes. Signed-off-by: longjin <longjin@dragonos.org>

fslongjin · 2026-09-06T12:25:40Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0424b7f82f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Cover creator and current group membership through both primary and supplementary groups after changing gid. Check read/write access, unrelated-group denial, and the separate owner-only IPC_SET/IPC_RMID boundary. Linux 6.6 ipcperms checks both cgid and gid, so retain the existing kernel behavior. Signed-off-by: longjin <longjin@dragonos.org>

fslongjin · 2026-09-06T12:44:39Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3e3843d074

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Keep dense association storage and its group-identity index together. Prepare both buffers outside the manager lock, recheck capacity before publication, and repair moved slots on constant-time expected removal. Preserve Weak identity lifetimes, deferred RMID cleanup, geometric growth and best-effort unlocked reclamation. Extend registry regression coverage for removal, deduplication and concurrent capacity changes. Signed-off-by: longjin <longjin@dragonos.org>

The rcS marker contains brackets that were incorrectly interpreted as a regex character class, causing the monitor to misclassify a system that had already entered userspace. Add host tests executing the actual predicate against literal, binary, CRLF, missing and misleading log inputs. Keep existing timeout and failure policies unchanged. Signed-off-by: longjin <longjin@dragonos.org>

fslongjin · 2026-09-06T17:13:11Z

CI investigation and fix: a473663 corrects a proven boot-monitor bug in the failed Integration Test run. The serial log had already printed the rcS marker, but grep treated its brackets as a regular-expression character class and failed to recognize it. The monitor now matches the literal markers. Six tests execute the actual shell predicate; three failed before the change and all six pass afterward. Timeout and failure policies are unchanged.

The later upload error (no test cases found) was secondary: that run stopped before the test runner started. The monitor correction is not evidence that the underlying pre-test startup stall is fixed. Its exact internal cause has not been established from the available log. I have rerun the failed job at the same SHA as a control; it is currently running the syscall-test step. The new head also has fresh CI runs. Locally, the repository's network startup policy returned successfully after installing its configuration and mounting /run, but that does not exclude an intermittent cold-boot issue.

The semaphore-index change in fa5cdf9 separately passed the kernel build, library tests, actual-source fault/capacity/index probes and 87 DragonOS SysVSem tests plus 20 full-suite repetitions. Three-role adversarial review passed. No tests were skipped or timeouts extended to turn CI green.

fslongjin · 2026-09-06T17:13:12Z

@codex review

chatgpt-codex-connector · 2026-09-06T17:21:26Z

Codex Review: Didn't find any major issues. Bravo.

Reviewed commit: a47366304c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

github-actions Bot added the enhancement New feature or request label Aug 8, 2026

fslongjin reviewed Aug 9, 2026

View reviewed changes

Comment thread kernel/src/ipc/syscall/sys_semop.rs Outdated

mistcoversmyeyes force-pushed the feat/ipc-sem-2142 branch 2 times, most recently from e0bc761 to a266b20 Compare August 17, 2026 09:07

github-actions Bot added the test Unitest/User space test label Aug 19, 2026

mistcoversmyeyes marked this pull request as ready for review August 19, 2026 07:43

mistcoversmyeyes force-pushed the feat/ipc-sem-2142 branch from d9c382f to fbdee92 Compare August 26, 2026 11:29

chatgpt-codex-connector Bot reviewed Aug 29, 2026

View reviewed changes

Comment thread kernel/src/ipc/sem.rs Outdated

Comment thread kernel/src/ipc/ipc_perm.rs Outdated

Comment thread kernel/src/ipc/ipc_perm.rs Outdated

Comment thread kernel/src/ipc/sem.rs Outdated

Comment thread kernel/src/ipc/sem.rs Outdated

fslongjin requested changes Aug 29, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Sep 4, 2026

View reviewed changes

mistcoversmyeyes and others added 12 commits September 5, 2026 12:21

refactor(ipc): extract generic SysV IPC permission module

44b822c

feat(ipc): implement System V semaphore syscalls

22dcdd8

test(ipc): add SysV semaphore dunitest suite

7dabc9e

style(ipc): satisfy kernel formatting checks

a807b3c

style(ipc): translate SysV semaphore comments to English

49075ea

fix(ipc): make IPC_SET permission updates atomic

0650199

fix(ipc): make semaphore set allocation fallible

4d4346c

feat(ipc): implement SysV SEM_UNDO lifecycle

1257bce

fix(ipc): mark allocated IDs in release builds

7e9b503

fix(ipc): prioritize const semaphore waiters

8678779

fix(process): preserve reaped PID identity

26da368

fix(process): preserve exec locking in prepared namespace publication

73467d1

Signed-off-by: longjin <longjin@dragonos.org>

fslongjin force-pushed the feat/ipc-sem-2142 branch from cfa9335 to 73467d1 Compare September 5, 2026 12:44

fix(ipc): harden semaphore allocation and Linux syscall semantics

03c0e03

Signed-off-by: longjin <longjin@dragonos.org>

fix(ipc): defer semaphore removal reclamation until after unlock

277351e

Signed-off-by: longjin <longjin@dragonos.org>

chatgpt-codex-connector Bot reviewed Sep 5, 2026

View reviewed changes

Comment thread kernel/src/ipc/sem.rs Outdated

Comment thread kernel/src/ipc/sem.rs Outdated

fix(ipc): cache wait counts and retain zero undo records

26adf2c

Signed-off-by: longjin <longjin@dragonos.org>

fslongjin added 2 commits September 6, 2026 03:35

docs: remove sem undo implementation plan and design spec

41ebf5a

Signed-off-by: longjin <longjin@dragonos.org>

chatgpt-codex-connector Bot reviewed Sep 6, 2026

View reviewed changes

Comment thread kernel/src/ipc/sem/set/operation.rs Outdated

Comment thread kernel/src/ipc/sem_undo/mod.rs Outdated

perf(ipc): preindex semaphore scratch and reclaim undo record storage

a455a73

Signed-off-by: longjin <longjin@dragonos.org>

chatgpt-codex-connector Bot reviewed Sep 6, 2026

View reviewed changes

Comment thread kernel/src/ipc/sem/abi.rs Outdated

Comment thread kernel/src/ipc/sem_undo/mod.rs Outdated

fix(ipc): correct semaphore ABI and index undo records

55a48ef

Signed-off-by: longjin <longjin@dragonos.org>

chatgpt-codex-connector Bot reviewed Sep 6, 2026

View reviewed changes

Comment thread kernel/src/process/namespace/nsproxy.rs Outdated

Comment thread kernel/src/ipc/sem/manager/control.rs Outdated

Uh oh!

Uh oh!

Conversation

mistcoversmyeyes commented Aug 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related

Summary

Scope

Acceptance

Testing

Uh oh!

Uh oh!

fslongjin commented Aug 29, 2026

Uh oh!

chatgpt-codex-connector Bot commented Aug 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fslongjin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fslongjin Aug 29, 2026

Choose a reason for hiding this comment

Uh oh!

mistcoversmyeyes Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fslongjin Aug 29, 2026

Choose a reason for hiding this comment

Uh oh!

mistcoversmyeyes Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

mistcoversmyeyes commented Sep 2, 2026

Uh oh!

fslongjin commented Sep 4, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

fslongjin commented Sep 4, 2026

Uh oh!

fslongjin commented Sep 5, 2026

Uh oh!

fslongjin commented Sep 5, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

fslongjin commented Sep 6, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 6, 2026

Uh oh!

fslongjin commented Sep 6, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

mistcoversmyeyes commented Aug 8, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Aug 29, 2026 •

edited

Loading