Skip to content

Commit

Permalink
Redesign the Task struct to reduce lock contention (#433)
Browse files Browse the repository at this point in the history
* Move a Task's `ExitValue` out of its `RunState` so that `RunState` can be separately accessed atomically.
* Move immutable fields into `Task` and mutable fields into `TaskInner` (excluding atomic fields), which reduces lock contention. Most fields can now be accessed without acquiring locks.
* Use properly-aligned and -sized atomic types such that native atomic instructions are actually used. For this, we switch from `atomic::Atomic` to `crossbeam_utils::atomic::AtomicCell` since it actually compiles down to native atomics.
* Statically assert that all types wrapped in `AtomicCell` actually use native atomic instructions.
* The `runstate` and `running_on_cpu` fields of `Task` are now atomically accessible in a lock-free manner. This is beneficial for blocking/unblocking tasks in a lock-free context, e.g., within an interrupt handler.
* Remove now-unnecessary unsafe statements from `schedule()`
  • Loading branch information
kevinaboos authored Aug 18, 2021
1 parent 21da16c commit df721e3
Show file tree
Hide file tree
Showing 42 changed files with 664 additions and 684 deletions.
53 changes: 30 additions & 23 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 5 additions & 11 deletions applications/bm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1555,8 +1555,7 @@ fn get_prog_name() -> String {
}
};

let locked_task = taskref.lock();
locked_task.name.clone()
taskref.name.clone()
}

/// Helper function to get the PID of current task
Expand All @@ -1569,8 +1568,7 @@ fn getpid() -> usize {
}
};

let locked_task = taskref.lock();
locked_task.id
taskref.id
}


Expand All @@ -1589,13 +1587,9 @@ fn hpet_2_time(msg_header: &str, hpet: u64) -> u64 {

/// Helper function to get current working directory
fn get_cwd() -> Option<DirRef> {
if let Some(taskref) = task::get_my_current_task() {
let locked_task = &taskref.lock();
let curr_env = locked_task.env.lock();
return Some(Arc::clone(&curr_env.working_dir));
}

None
task::get_my_current_task().map(|t|
Arc::clone(&t.get_env().lock().working_dir)
)
}

/// Helper function to make a temporary file to be used to measure read open latencies
Expand Down
6 changes: 1 addition & 5 deletions applications/cat/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,7 @@ pub fn main(args: Vec<String>) -> isize {
};

// grabs the current working directory pointer; this is scoped so that we drop the lock on the task as soon as we get the working directory pointer
let curr_wr = {
let locked_task = taskref.lock();
let curr_env = locked_task.env.lock();
Arc::clone(&curr_env.working_dir)
};
let curr_wr = Arc::clone(&taskref.get_env().lock().working_dir);
let path = Path::new(matches.free[0].to_string());

// navigate to the filepath specified by first argument
Expand Down
14 changes: 2 additions & 12 deletions applications/cd/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,18 +38,8 @@ pub fn main(args: Vec<String>) -> isize {
return -1;
}
};
// grabs the current environment pointer; this is scoped so that we drop the lock on the "cd" task
let curr_env = {
let locked_task = taskref.lock();
Arc::clone(&locked_task.env)
};

// grabs the current working directory pointer; this is scoped so that we drop the lock on the "cd" task
let curr_wr = {
let locked_task = taskref.lock();
let curr_env = locked_task.env.lock();
Arc::clone(&curr_env.working_dir)
};
let curr_env = taskref.get_env();
let curr_wr = Arc::clone(&curr_env.lock().working_dir);

// go to root directory
if matches.free.is_empty() {
Expand Down
3 changes: 1 addition & 2 deletions applications/cpu/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ pub fn main(args: Vec<String>) -> isize {

if let Some(runqueue) = runqueue::get_runqueue(apic_id).map(|rq| rq.read()) {
let mut runqueue_contents = String::new();
for task_ref in runqueue.iter() {
let task = task_ref.lock();
for task in runqueue.iter() {
runqueue_contents.push_str(&format!("{} ({}) {}\n",
task.name,
task.id,
Expand Down
2 changes: 1 addition & 1 deletion applications/deps/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ fn find_section(section_name: &str) -> Result<StrongSectionRef, String> {


fn get_my_current_namespace() -> Arc<CrateNamespace> {
task::get_my_current_task().map(|t| t.get_namespace()).unwrap_or_else(||
task::get_my_current_task().map(|t| Arc::clone(t.get_namespace())).unwrap_or_else(||
mod_mgmt::get_initial_kernel_namespace().expect("BUG: initial kernel namespace wasn't initialized").clone()
)
}
Expand Down
2 changes: 1 addition & 1 deletion applications/kill/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ fn kill_task(task_id: usize, reap: bool) -> Result<(), String> {
.and_then(|_| runqueue::remove_task_from_all(&task_ref))
.is_ok()
{
println!("Killed task {}", &*task_ref.lock());
println!("Killed task {}", &*task_ref);
if reap {
match task_ref.take_exit_value() {
Some(exit_val) => {
Expand Down
6 changes: 1 addition & 5 deletions applications/less/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,7 @@ fn get_content_string(file_path: String) -> Result<String, String> {
};

// grabs the current working directory pointer; this is scoped so that we drop the lock on the task as soon as we get the working directory pointer
let curr_wr = {
let locked_task = taskref.lock();
let curr_env = locked_task.env.lock();
Arc::clone(&curr_env.working_dir)
};
let curr_wr = Arc::clone(&taskref.get_env().lock().working_dir);
let path = Path::new(file_path);

// navigate to the filepath specified by first argument
Expand Down
8 changes: 4 additions & 4 deletions applications/loadc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ pub fn main(args: Vec<String>) -> isize {

fn rmain(matches: Matches) -> Result<c_int, String> {
let curr_task = task::get_my_current_task().unwrap();
let curr_wd = Arc::clone(&curr_task.lock().env.lock().working_dir);
let curr_wd = Arc::clone(&curr_task.get_env().lock().working_dir);
let namespace = curr_task.get_namespace();
let mmi = Arc::clone(&curr_task.lock().mmi);
let mmi = &curr_task.mmi;

let path = matches.free.get(0).ok_or_else(|| format!("Missing path to ELF executable"))?;
let path = Path::new(path.clone());
Expand All @@ -84,7 +84,7 @@ fn rmain(matches: Matches) -> Result<c_int, String> {
// most important of which are static data sections,
// as it is logically incorrect to have duplicates of data that are supposed to be global system-wide singletons.
// We should throw a warning here if there are no relocations in the file, as it was probably built/linked with the wrong arguments.
overwrite_relocations(&namespace, &mut segments, &elf_file, &mmi, false)?;
overwrite_relocations(&namespace, &mut segments, &elf_file, mmi, false)?;

// Remap each segment's mapped pages using the correct flags; they were previously mapped as always writable.
{
Expand Down Expand Up @@ -272,7 +272,7 @@ fn parse_and_load_elf_executable<'f>(
// debug!("Adjusted segment vaddr: {:#X}, size: {:#X}, {:?}", start_vaddr, memory_size_in_bytes, this_ap.start_address());

let initial_flags = EntryFlags::from_elf_program_flags(prog_hdr.flags());
let mmi = task::get_my_current_task().unwrap().lock().mmi.clone();
let mmi = &task::get_my_current_task().unwrap().mmi;
// Must initially map the memory as writable so we can copy the segment data to it later.
let mut mp = mmi.lock().page_table
.map_allocated_pages(this_ap, initial_flags | EntryFlags::WRITABLE)
Expand Down
7 changes: 1 addition & 6 deletions applications/ls/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,7 @@ pub fn main(args: Vec<String>) -> isize {
}
};

// grabs the current working directory pointer; this is scoped so that we drop the lock on the "cd" task
let curr_wd = {
let locked_task = taskref.lock();
let curr_env = locked_task.env.lock();
Arc::clone(&curr_env.working_dir)
};
let curr_wd = Arc::clone(&taskref.get_env().lock().working_dir);

// print children of working directory if no child is specified
if matches.free.is_empty() {
Expand Down
6 changes: 1 addition & 5 deletions applications/mkdir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,7 @@ pub fn main(args: Vec<String>) -> isize {
// add child dir to current directory
if let Some(taskref) = task::get_my_current_task() {
// grabs a pointer to the current working directory; this is scoped so that we drop the lock on the "mkdir" task as soon as we're finished
let curr_dir = {
let locked_task = &taskref.lock();
let curr_env = locked_task.env.lock();
Arc::clone(&curr_env.working_dir)
};
let curr_dir = Arc::clone(&taskref.get_env().lock().working_dir);
let _new_dir = match VFSDirectory::new(dir_name.to_string(), &curr_dir) {
Ok(dir) => dir,
Err(err) => {println!("{}", err);
Expand Down
Loading

0 comments on commit df721e3

Please sign in to comment.