Skip to content

Conversation

@badicsalex
Copy link
Contributor

No description provided.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR modernizes the Xous platform implementation in Rust's standard library by migrating from custom synchronization primitives to futex-based implementations and upgrading timing APIs from millisecond to nanosecond precision.

  • Migrates mutex, rwlock, once, and condvar implementations from custom Xous-specific code to futex-based primitives
  • Updates all time-related APIs to use nanoseconds instead of milliseconds for improved precision
  • Simplifies the allocator implementation by replacing a custom spinlock with std::sync::Mutex

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 15 comments.

Show a summary per file
File Description
library/std/src/sys/thread/xous.rs Converts sleep() to use nanoseconds with simplified single-call implementation
library/std/src/sys/sync/thread_parking/xous.rs Complete rewrite using mutex/condvar instead of custom state machine
library/std/src/sys/sync/rwlock/mod.rs Moves xous from queue-based to futex-based RwLock
library/std/src/sys/sync/once/mod.rs Moves xous from queue-based to futex-based Once
library/std/src/sys/sync/mutex/xous.rs Removes custom mutex implementation (entire file deleted)
library/std/src/sys/sync/mutex/mod.rs Switches xous to use futex-based Mutex
library/std/src/sys/sync/condvar/xous.rs Rewrites condvar with dual futex/ticktimer waiting support
library/std/src/sys/pal/xous/time.rs Updates time APIs from milliseconds to nanoseconds
library/std/src/sys/pal/xous/mod.rs Adds futex module export
library/std/src/sys/pal/xous/futex.rs Introduces new futex wrapper implementation for xous
library/std/src/sys/alloc/xous.rs Simplifies allocator by replacing custom lock with std::sync::Mutex
library/std/src/os/xous/services/ticktimer.rs Updates ticktimer API enum variants to use nanosecond parameters

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@@ -0,0 +1,24 @@
use core::{sync::atomic::{Atomic}, time::Duration};
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

The import statement has a syntax error with an extra comma and missing type parameter. The Atomic type requires a type parameter (e.g., Atomic<usize>), and there's an unnecessary comma before the closing brace.

Suggested change
use core::{sync::atomic::{Atomic}, time::Duration};
use core::{sync::atomic::Atomic, time::Duration};

Copilot uses AI. Check for mistakes.
pub type SmallPrimitive = usize;

pub fn futex_wait(futex: &Futex, expected: Primitive, timeout: Option<Duration>) -> bool {
assert!(timeout.is_none(), "Timeouts on xous futexes is not supported");
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

Spelling error in assertion message: "Timeouts" should be "Timeout" (singular form is more appropriate here).

Suggested change
assert!(timeout.is_none(), "Timeouts on xous futexes is not supported");
assert!(timeout.is_none(), "Timeout on xous futexes is not supported");

Copilot uses AI. Check for mistakes.
@badicsalex badicsalex merged commit 0e91065 into 1.91.1-xous-arm Dec 11, 2025
8 of 17 checks passed
@badicsalex badicsalex mentioned this pull request Dec 11, 2025
@badicsalex
Copy link
Contributor Author

I closed this accidentally.

pub type Futex = Atomic<Primitive>;
pub type Primitive = usize;

pub type SmallFutex = Atomic<SmallPrimitive>;

Choose a reason for hiding this comment

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

what is this? why do we have two of the same thing? is this something required to be exported from the futex file?

Copy link

@nicoburniske nicoburniske left a comment

Choose a reason for hiding this comment

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

approved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants