feat(ipi): tie ipi to smp feature, add sync mechanism for ipi and support lazy flush tlb by li041 · Pull Request #29 · Starry-OS/arceos

li041 · 2025-12-11T08:08:21Z

feat(smp): support cross-core TLB flush arceos-org/page_table_multiarch#32
feat(smp): track per-task CPU residency to avoid unnecessary cross-core TLB shootdowns StarryOS#84

Copilot

Pull request overview

This PR introduces IPI (Inter-Processor Interrupt) synchronization mechanisms and ties the IPI feature to SMP support, enabling lazy TLB flushing across multiple CPUs. The changes include adding a wait/sync mechanism for IPI operations, tracking secondary CPU startup state, and implementing TLB flush operations that leverage the IPI system.

Key changes:

Added synchronization support to IPI operations with a wait parameter and completion tracking via AtomicBool flags
Introduced SECONDARY_CPUS_STARTED flag to track when secondary CPUs are ready
Implemented TLB flush interface that uses IPI for cross-CPU coordination

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
modules/axtask/src/task.rs	Adds `on_cpu_mask()` method to TaskExt trait for CPU affinity tracking
modules/axruntime/src/lib.rs	Initializes axipi module and signals secondary CPU startup completion
modules/axmm/Cargo.toml	Enables SMP feature for page_table_multiarch dependency
modules/axipi/src/tlb.rs	New file implementing TLB flush interface using IPI mechanism
modules/axipi/src/queue.rs	Updates IPI event queue to track event names and completion flags
modules/axipi/src/lib.rs	Adds synchronization support, secondary CPU tracking, and new multicast functions
modules/axipi/src/event.rs	Extends IpiEvent structure with name field and completion flag
modules/axipi/Cargo.toml	Adds dependencies for TLB flush implementation
api/axfeat/Cargo.toml	Ties IPI feature to SMP and adds axmm dependency

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

modules/axipi/src/tlb.rs

modules/axipi/src/lib.rs

api/axfeat/Cargo.toml

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

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

modules/axipi/src/lib.rs

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

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

Copilot · 2025-12-23T07:18:17Z

modules/axipi/src/lib.rs

+    let cpu_num = axconfig::plat::CPU_NUM;
+    let callback = callback.into();
+
+    let mut done_flags: Vec<Arc<AtomicBool>> = Vec::new(cpu_num - 1);


Vec::new() does not accept a capacity parameter. Use Vec::with_capacity(cpu_num - 1) instead to preallocate the vector with the correct capacity.

Copilot · 2025-12-23T07:18:17Z

modules/axipi/src/lib.rs

    // Execute callback on current CPU immediately
    callback.clone().call();
+
+    let mut done_flags: Vec<Arc<AtomicBool>> = Vec::new(cpu_num);


Vec::new() does not accept a capacity parameter. Use Vec::with_capacity(cpu_num) instead to preallocate the vector with the correct capacity.

Copilot · 2025-12-23T07:18:17Z

modules/axipi/src/lib.rs

+    if done_flags.is_empty() {
+        return;


The early return for empty done_flags in run_on_each_cpu_except_self prevents IPI from being sent when wait is false. This logic appears inconsistent with run_on_bitmask_except_self (lines 107-111) which always sends IPIs regardless of wait flag. Consider whether IPIs should be sent even when wait=false and done_flags is empty.

Copilot · 2025-12-23T07:18:18Z

modules/axipi/Cargo.toml

 log.workspace = true
 percpu.workspace = true
+crate_interface = "0.1.4"
+axcpu = { git = "https://github.com/arceos-org/axcpu.git", tag = "dev-v03" }


The axcpu dependency is added but does not appear to be used in any of the modified axipi source files. Consider removing this dependency if it's not needed, or add it when it's actually required.

Suggested change

axcpu = { git = "https://github.com/arceos-org/axcpu.git", tag = "dev-v03" }

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

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

Copilot · 2025-12-23T11:31:52Z

modules/axipi/src/tlb.rs

+    fn flush_all(vaddr: Option<VirtAddr>) {
+        if axconfig::plat::CPU_NUM == 1 || !secondary_cpus_ready() {
+            // local
+            axhal::asm::flush_tlb(None);


The local TLB flush is called with None regardless of the vaddr parameter value. This should pass vaddr to maintain consistency with the remote flush behavior on line 20.

Suggested change

axhal::asm::flush_tlb(None);

axhal::asm::flush_tlb(vaddr);

Copilot · 2025-12-23T11:31:52Z

modules/axipi/src/lib.rs

+    let cpu_num = axconfig::plat::CPU_NUM;
+    let callback = callback.into();
+
+    let mut done_flags: Vec<Arc<AtomicBool>> = Vec::with_capacity(cpu_num - 1);


The vector is pre-allocated with cpu_num - 1 capacity but this is only accurate when all other CPUs will receive the IPI. Consider pre-allocating with the exact capacity needed or using Vec::new() without pre-allocation.

Copilot · 2025-12-23T11:31:53Z

modules/axipi/src/lib.rs

    // Execute callback on current CPU immediately
    callback.clone().call();
+
+    let mut done_flags: Vec<Arc<AtomicBool>> = Vec::with_capacity(cpu_num);


The capacity is set to cpu_num but only cpu_num - 1 flags will be added since the current CPU is excluded. The capacity should be cpu_num - 1 to avoid over-allocation.

Suggested change

let mut done_flags: Vec<Arc<AtomicBool>> = Vec::with_capacity(cpu_num);

let mut done_flags: Vec<Arc<AtomicBool>> = Vec::with_capacity(cpu_num - 1);

Godones · 2025-12-30T08:20:13Z

modules/axipi/src/tlb.rs

+        } else {
+            let callback = MulticastCallback::new(move || {
+                axhal::asm::flush_tlb(vaddr);
+            });


Why isn't the local CPU flushing?

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

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

Copilot · 2025-12-31T04:50:54Z

modules/axipi/src/lib.rs

-pub fn run_on_cpu<T: Into<Callback>>(dest_cpu: usize, callback: T) {
-    info!("Send IPI event to CPU {dest_cpu}");
+pub fn run_on_cpu<T: Into<Callback>>(name: &'static str, dest_cpu: usize, callback: T, wait: bool) {
+    let gurad = kernel_guard::NoPreempt::new();


Corrected spelling of 'gurad' to 'guard'.

Copilot · 2025-12-31T04:50:55Z

modules/axipi/src/lib.rs

 /// Executes a callback on all other CPUs via IPI.
-pub fn run_on_each_cpu<T: Into<MulticastCallback>>(callback: T) {
+pub fn run_on_each_cpu<T: Into<MulticastCallback>>(name: &'static str, callback: T, wait: bool) {
+    let gurad = kernel_guard::NoPreempt::new();


Corrected spelling of 'gurad' to 'guard'.

Copilot · 2025-12-31T04:50:55Z

modules/axipi/src/lib.rs

+            }
+        }
+    }
+    drop(gurad); // rescheduling may occur when preemption is re-enabled.


Corrected spelling of 'gurad' to 'guard'.

Copilot AI review requested due to automatic review settings December 11, 2025 08:08

Copilot AI reviewed Dec 11, 2025

View reviewed changes

AsakuraMizu force-pushed the dev branch from 8195846 to 9a39bb7 Compare December 16, 2025 06:37

AsakuraMizu marked this pull request as draft December 16, 2025 07:17

AsakuraMizu mentioned this pull request Dec 16, 2025

feat(smp): track per-task CPU residency to avoid unnecessary cross-core TLB shootdowns Starry-OS/StarryOS#84

Draft

li041 added 2 commits December 23, 2025 12:02

feat(axfeat): tie ipi to smp feature

02d9431

feat(tlb): add sync mechanism for ipi and support lazy flush tlb

fad5887

li041 force-pushed the dev branch from 7e5a081 to fad5887 Compare December 23, 2025 06:59

rm unused import

3631e25

Copilot AI review requested due to automatic review settings December 23, 2025 07:02

Copilot AI reviewed Dec 23, 2025

View reviewed changes

li041 marked this pull request as ready for review December 23, 2025 07:09

li041 added 2 commits December 23, 2025 15:13

task copilot advice

5d5490e

pre-alloc with cpu num for done_flags

7d3fbc5

Copilot AI review requested due to automatic review settings December 23, 2025 07:17

Copilot AI reviewed Dec 23, 2025

View reviewed changes

li041 added 2 commits December 23, 2025 17:10

rm unnecessary check for done_flags

12e5fb5

fix vec init

69d2073

Copilot AI review requested due to automatic review settings December 23, 2025 11:30

Copilot AI reviewed Dec 23, 2025

View reviewed changes

Godones reviewed Dec 30, 2025

View reviewed changes

Godones approved these changes Dec 30, 2025

View reviewed changes

li041 added 2 commits December 30, 2025 19:32

disable preempt during sending ipi

74e63ac

rename functions

c82e4cc

Copilot AI review requested due to automatic review settings December 31, 2025 04:50

Copilot AI reviewed Dec 31, 2025

View reviewed changes

fix typo

9b530e1

AsakuraMizu marked this pull request as draft January 10, 2026 10:08

	let mut done_flags: Vec<Arc<AtomicBool>> = Vec::with_capacity(cpu_num);
	let mut done_flags: Vec<Arc<AtomicBool>> = Vec::with_capacity(cpu_num - 1);

Conversation

li041 commented Dec 11, 2025 • edited by AsakuraMizu Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Godones Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

li041 commented Dec 11, 2025 •

edited by AsakuraMizu

Loading