1
0
mirror of https://github.com/rcore-os/rCore.git synced 2024-11-25 01:16:18 +04:00

fix data racing on thread crate

The key point is that all methods on 'Processor' must be called with interrupt disabled. Otherwise if an interrupt happened inside a method, and then the thread is switched to other CPUs, it will touch other 'Processor'.
This commit is contained in:
Runji Wang 2019-11-30 18:11:03 +08:00
parent 20f8b45888
commit af83913188
10 changed files with 36 additions and 47 deletions

6
kernel/Cargo.lock generated
View File

@ -356,7 +356,7 @@ dependencies = [
"rcore-fs-ramfs 0.1.0 (git+https://github.com/rcore-os/rcore-fs?rev=d8d6119)", "rcore-fs-ramfs 0.1.0 (git+https://github.com/rcore-os/rcore-fs?rev=d8d6119)",
"rcore-fs-sfs 0.1.0 (git+https://github.com/rcore-os/rcore-fs?rev=d8d6119)", "rcore-fs-sfs 0.1.0 (git+https://github.com/rcore-os/rcore-fs?rev=d8d6119)",
"rcore-memory 0.1.0", "rcore-memory 0.1.0",
"rcore-thread 0.1.0 (git+https://github.com/rcore-os/rcore-thread?rev=95e716a2)", "rcore-thread 0.1.0 (git+https://github.com/rcore-os/rcore-thread?rev=d727949b)",
"riscv 0.5.0 (git+https://github.com/rcore-os/riscv)", "riscv 0.5.0 (git+https://github.com/rcore-os/riscv)",
"smoltcp 0.5.0 (git+https://github.com/rcore-os/smoltcp?rev=5bd87c7c)", "smoltcp 0.5.0 (git+https://github.com/rcore-os/smoltcp?rev=5bd87c7c)",
"spin 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)", "spin 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)",
@ -436,7 +436,7 @@ dependencies = [
[[package]] [[package]]
name = "rcore-thread" name = "rcore-thread"
version = "0.1.0" version = "0.1.0"
source = "git+https://github.com/rcore-os/rcore-thread?rev=95e716a2#95e716a2d3c315b19dda787cfe7302957b66f80b" source = "git+https://github.com/rcore-os/rcore-thread?rev=d727949b#d727949b8ea20c2e3a94f8ba015652c9eff8b53c"
dependencies = [ dependencies = [
"deque 0.3.2 (git+https://github.com/rcore-os/deque.git?branch=no_std)", "deque 0.3.2 (git+https://github.com/rcore-os/deque.git?branch=no_std)",
"log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)",
@ -707,7 +707,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
"checksum rcore-fs-mountfs 0.1.0 (git+https://github.com/rcore-os/rcore-fs?rev=d8d6119)" = "<none>" "checksum rcore-fs-mountfs 0.1.0 (git+https://github.com/rcore-os/rcore-fs?rev=d8d6119)" = "<none>"
"checksum rcore-fs-ramfs 0.1.0 (git+https://github.com/rcore-os/rcore-fs?rev=d8d6119)" = "<none>" "checksum rcore-fs-ramfs 0.1.0 (git+https://github.com/rcore-os/rcore-fs?rev=d8d6119)" = "<none>"
"checksum rcore-fs-sfs 0.1.0 (git+https://github.com/rcore-os/rcore-fs?rev=d8d6119)" = "<none>" "checksum rcore-fs-sfs 0.1.0 (git+https://github.com/rcore-os/rcore-fs?rev=d8d6119)" = "<none>"
"checksum rcore-thread 0.1.0 (git+https://github.com/rcore-os/rcore-thread?rev=95e716a2)" = "<none>" "checksum rcore-thread 0.1.0 (git+https://github.com/rcore-os/rcore-thread?rev=d727949b)" = "<none>"
"checksum register 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "e10f31b6d2299e5620986ad9fcdd66463e125ad72af4f403f9aedf7592d5ccdb" "checksum register 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "e10f31b6d2299e5620986ad9fcdd66463e125ad72af4f403f9aedf7592d5ccdb"
"checksum riscv 0.5.0 (git+https://github.com/rcore-os/riscv)" = "<none>" "checksum riscv 0.5.0 (git+https://github.com/rcore-os/riscv)" = "<none>"
"checksum rustc_version 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)" = "138e3e0acb6c9fb258b19b67cb8abd63c00679d2851805ea151465464fe9030a" "checksum rustc_version 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)" = "138e3e0acb6c9fb258b19b67cb8abd63c00679d2851805ea151465464fe9030a"

View File

@ -62,7 +62,7 @@ smoltcp = { git = "https://github.com/rcore-os/smoltcp", rev = "5bd87c7c", defau
bitmap-allocator = { git = "https://github.com/rcore-os/bitmap-allocator" } bitmap-allocator = { git = "https://github.com/rcore-os/bitmap-allocator" }
rcore-console = { git = "https://github.com/rcore-os/rcore-console", rev = "b7bacf9", default-features = false } rcore-console = { git = "https://github.com/rcore-os/rcore-console", rev = "b7bacf9", default-features = false }
rcore-memory = { path = "../crate/memory" } rcore-memory = { path = "../crate/memory" }
rcore-thread = { git = "https://github.com/rcore-os/rcore-thread", rev = "95e716a2" } rcore-thread = { git = "https://github.com/rcore-os/rcore-thread", rev = "d727949b" }
rcore-fs = { git = "https://github.com/rcore-os/rcore-fs", rev = "d8d6119" } rcore-fs = { git = "https://github.com/rcore-os/rcore-fs", rev = "d8d6119" }
rcore-fs-sfs = { git = "https://github.com/rcore-os/rcore-fs", rev = "d8d6119" } rcore-fs-sfs = { git = "https://github.com/rcore-os/rcore-fs", rev = "d8d6119" }
rcore-fs-ramfs = { git = "https://github.com/rcore-os/rcore-fs", rev = "d8d6119" } rcore-fs-ramfs = { git = "https://github.com/rcore-os/rcore-fs", rev = "d8d6119" }

View File

@ -69,10 +69,10 @@ impl Log for SimpleLogger {
if let Some(tid) = processor().tid_option() { if let Some(tid) = processor().tid_option() {
print_in_color( print_in_color(
format_args!( format_args!(
"[{:>5}][{}][{}] {}\n", "[{:>5}][{},{}] {}\n",
record.level(), record.level(),
crate::arch::cpu::id(),
tid, tid,
record.target(),
record.args() record.args()
), ),
level_to_color_code(record.level()), level_to_color_code(record.level()),
@ -80,9 +80,9 @@ impl Log for SimpleLogger {
} else { } else {
print_in_color( print_in_color(
format_args!( format_args!(
"[{:>5}][-][{}] {}\n", "[{:>5}][{},-] {}\n",
record.level(), record.level(),
record.target(), crate::arch::cpu::id(),
record.args() record.args()
), ),
level_to_color_code(record.level()), level_to_color_code(record.level()),

View File

@ -90,22 +90,6 @@ static PROCESSORS: [Processor; MAX_CPU_NUM] = [
Processor::new(), Processor::new(),
Processor::new(), Processor::new(),
Processor::new(), Processor::new(),
// Processor::new(), Processor::new(), Processor::new(), Processor::new(),
// Processor::new(), Processor::new(), Processor::new(), Processor::new(),
// Processor::new(), Processor::new(), Processor::new(), Processor::new(),
// Processor::new(), Processor::new(), Processor::new(), Processor::new(),
// Processor::new(), Processor::new(), Processor::new(), Processor::new(),
// Processor::new(), Processor::new(), Processor::new(), Processor::new(),
// Processor::new(), Processor::new(), Processor::new(), Processor::new(),
// Processor::new(), Processor::new(), Processor::new(), Processor::new(),
// Processor::new(), Processor::new(), Processor::new(), Processor::new(),
// Processor::new(), Processor::new(), Processor::new(), Processor::new(),
// Processor::new(), Processor::new(), Processor::new(), Processor::new(),
// Processor::new(), Processor::new(), Processor::new(), Processor::new(),
// Processor::new(), Processor::new(), Processor::new(), Processor::new(),
// Processor::new(), Processor::new(), Processor::new(), Processor::new(),
// Processor::new(), Processor::new(), Processor::new(), Processor::new(),
// Processor::new(), Processor::new(), Processor::new(), Processor::new(),
]; ];
/// Get current thread /// Get current thread
@ -118,6 +102,11 @@ pub unsafe fn current_thread() -> &'static mut Thread {
process process
} }
/// Get global thread manager.
pub fn thread_manager() -> &'static ThreadPool {
processor().manager()
}
// Implement dependencies for std::thread // Implement dependencies for std::thread
#[no_mangle] #[no_mangle]

View File

@ -21,7 +21,7 @@ use crate::memory::{
use crate::sync::{Condvar, SpinNoIrqLock as Mutex}; use crate::sync::{Condvar, SpinNoIrqLock as Mutex};
use super::abi::{self, ProcInitInfo}; use super::abi::{self, ProcInitInfo};
use crate::processor; use crate::process::thread_manager;
use core::mem::MaybeUninit; use core::mem::MaybeUninit;
use rcore_fs::vfs::INode; use rcore_fs::vfs::INode;
@ -420,7 +420,7 @@ impl Process {
pub fn exit(&mut self, exit_code: usize) { pub fn exit(&mut self, exit_code: usize) {
// quit all threads // quit all threads
for tid in self.threads.iter() { for tid in self.threads.iter() {
processor().manager().exit(*tid, 1); thread_manager().exit(*tid, 1);
} }
// notify parent and fill exit code // notify parent and fill exit code
if let Some(parent) = self.parent.upgrade() { if let Some(parent) = self.parent.upgrade() {

View File

@ -38,7 +38,7 @@ pub fn add_user_shell() {
.manager() .manager()
.add(Thread::new_user(&inode, init_shell, init_args, init_envs)); .add(Thread::new_user(&inode, init_shell, init_args, init_envs));
} else { } else {
processor().manager().add(Thread::new_kernel(shell, 0)); thread_manager().add(Thread::new_kernel(shell, 0));
} }
} }
@ -47,7 +47,7 @@ pub fn add_user_shell() {
use crate::drivers::CMDLINE; use crate::drivers::CMDLINE;
let cmdline = CMDLINE.read(); let cmdline = CMDLINE.read();
let inode = ROOT_INODE.lookup(&cmdline).unwrap(); let inode = ROOT_INODE.lookup(&cmdline).unwrap();
processor().manager().add(Thread::new_user( thread_manager().add(Thread::new_user(
&inode, &inode,
&cmdline, &cmdline,
cmdline.split(' ').map(|s| s.into()).collect(), cmdline.split(' ').map(|s| s.into()).collect(),
@ -68,7 +68,7 @@ pub extern "C" fn shell(_arg: usize) -> ! {
} }
let name = cmd.trim().split(' ').next().unwrap(); let name = cmd.trim().split(' ').next().unwrap();
if let Ok(inode) = ROOT_INODE.lookup(name) { if let Ok(inode) = ROOT_INODE.lookup(name) {
let _tid = processor().manager().add(Thread::new_user( let _tid = thread_manager().add(Thread::new_user(
&inode, &inode,
&cmd, &cmd,
cmd.split(' ').map(|s| s.into()).collect(), cmd.split(' ').map(|s| s.into()).collect(),

View File

@ -1,6 +1,6 @@
use super::*; use super::*;
use crate::process::Process; use crate::process::Process;
use crate::process::{current_thread, processor}; use crate::process::{current_thread, thread_manager};
use crate::thread; use crate::thread;
use alloc::collections::VecDeque; use alloc::collections::VecDeque;
use alloc::sync::Arc; use alloc::sync::Arc;
@ -63,19 +63,19 @@ impl Condvar {
let mut lock = condvar.wait_queue.lock(); let mut lock = condvar.wait_queue.lock();
locks.push(lock); locks.push(lock);
} }
processor().manager().sleep(tid, 0); thread_manager().sleep(tid, 0);
locks.clear(); locks.clear();
if let Some(res) = condition() { if let Some(res) = condition() {
let _ = FlagsGuard::no_irq_region(); let _ = FlagsGuard::no_irq_region();
processor().manager().cancel_sleeping(tid); thread_manager().cancel_sleeping(tid);
for condvar in condvars { for condvar in condvars {
let mut lock = condvar.wait_queue.lock(); let mut lock = condvar.wait_queue.lock();
lock.retain(|t| !Arc::ptr_eq(t, &token)); lock.retain(|t| !Arc::ptr_eq(t, &token));
} }
return res; return res;
} }
processor().yield_now(); thread::yield_now();
} }
} }

View File

@ -80,7 +80,7 @@ impl Syscall<'_> {
let begin_time = unsafe { core::arch::x86_64::_rdtsc() }; let begin_time = unsafe { core::arch::x86_64::_rdtsc() };
let cid = cpu::id(); let cid = cpu::id();
let pid = self.process().pid.clone(); let pid = self.process().pid.clone();
let tid = processor().tid(); let tid = thread::current().id();
if !pid.is_init() { if !pid.is_init() {
// we trust pid 0 process // we trust pid 0 process
debug!("{}:{}:{} syscall id {} begin", cid, pid, tid, id); debug!("{}:{}:{} syscall id {} begin", cid, pid, tid, id);

View File

@ -7,8 +7,8 @@ impl Syscall<'_> {
pub fn sys_fork(&mut self) -> SysResult { pub fn sys_fork(&mut self) -> SysResult {
let new_thread = self.thread.fork(self.tf); let new_thread = self.thread.fork(self.tf);
let pid = new_thread.proc.lock().pid.get(); let pid = new_thread.proc.lock().pid.get();
let tid = processor().manager().add(new_thread); let tid = thread_manager().add(new_thread);
processor().manager().detach(tid); thread_manager().detach(tid);
info!("fork: {} -> {}", thread::current().id(), pid); info!("fork: {} -> {}", thread::current().id(), pid);
Ok(pid) Ok(pid)
} }
@ -53,8 +53,8 @@ impl Syscall<'_> {
let new_thread = self let new_thread = self
.thread .thread
.clone(self.tf, newsp, newtls, child_tid as usize); .clone(self.tf, newsp, newtls, child_tid as usize);
let tid = processor().manager().add(new_thread); let tid = thread_manager().add(new_thread);
processor().manager().detach(tid); thread_manager().detach(tid);
info!("clone: {} -> {}", thread::current().id(), tid); info!("clone: {} -> {}", thread::current().id(), tid);
*parent_tid_ref = tid as u32; *parent_tid_ref = tid as u32;
*child_tid_ref = tid as u32; *child_tid_ref = tid as u32;
@ -172,10 +172,10 @@ impl Syscall<'_> {
// Kill other threads // Kill other threads
proc.threads.retain(|&tid| { proc.threads.retain(|&tid| {
if tid != processor().tid() { if tid != thread::current().id() {
processor().manager().exit(tid, 1); thread_manager().exit(tid, 1);
} }
tid == processor().tid() tid == thread::current().id()
}); });
// Read program file // Read program file
@ -280,8 +280,8 @@ impl Syscall<'_> {
drop(proc); drop(proc);
processor().manager().exit(tid, exit_code as usize); thread_manager().exit(tid, exit_code as usize);
processor().yield_now(); thread::yield_now();
unreachable!(); unreachable!();
} }
@ -292,7 +292,7 @@ impl Syscall<'_> {
proc.exit(exit_code); proc.exit(exit_code);
processor().yield_now(); thread::yield_now();
unreachable!(); unreachable!();
} }
@ -306,7 +306,7 @@ impl Syscall<'_> {
pub fn sys_set_priority(&mut self, priority: usize) -> SysResult { pub fn sys_set_priority(&mut self, priority: usize) -> SysResult {
let pid = thread::current().id(); let pid = thread::current().id();
processor().manager().set_priority(pid, priority as u8); thread_manager().set_priority(pid, priority as u8);
Ok(0) Ok(0)
} }

View File

@ -3,7 +3,7 @@ use crate::arch::interrupt::TrapFrame;
use crate::consts::INFORM_PER_MSEC; use crate::consts::INFORM_PER_MSEC;
use crate::process::*; use crate::process::*;
use crate::sync::Condvar; use crate::sync::Condvar;
use log::*; use rcore_thread::std_thread as thread;
pub static mut TICK: usize = 0; pub static mut TICK: usize = 0;
@ -33,7 +33,7 @@ pub fn error(tf: &TrapFrame) -> ! {
let mut proc = current_thread().proc.lock(); let mut proc = current_thread().proc.lock();
proc.exit(0x100); proc.exit(0x100);
} }
processor().yield_now(); thread::yield_now();
unreachable!(); unreachable!();
} }