Fix Fifo & Clock SwapManager, pass test.

crate/memory/src/lib.rs

@@ -3,6 +3,11 @@
 
 extern crate alloc;
 
+// To use `println!` in test
+#[cfg(test)]
+#[macro_use]
+extern crate std;
+
 pub mod paging;
 pub mod cow;
 pub mod swap;

crate/memory/src/swap/enhanced_clock.rs

@@ -1,13 +1,15 @@
 use alloc::vec_deque::VecDeque;
 use super::*;
+use paging::Entry;
 
-pub struct EnhancedClockSwapManager<T: 'static + SwappablePageTable> {
+#[derive(Default)]
-    page_table: &'static mut T,
+pub struct EnhancedClockSwapManager {
     clock_ptr: usize,
     deque: VecDeque<VirtAddr>,
 }
 
-impl<T: 'static + SwappablePageTable> SwapManager for EnhancedClockSwapManager<T> {
+// FIXME: It's unusable. But can pass a simple test.
+impl SwapManager for EnhancedClockSwapManager {
     fn tick(&mut self) {
     }
 
@@ -26,19 +28,28 @@ impl<T: 'static + SwappablePageTable> SwapManager for EnhancedClockSwapManager<T
         self.deque.remove(id);
     }
 
-    fn pop(&mut self) -> Option<usize> {
+    fn pop<T, S>(&mut self, page_table: &mut T, _swapper: &mut S) -> Option<VirtAddr>
+        where T: PageTable, S: Swapper
+    {
         loop {
             let addr = self.deque[self.clock_ptr];
-            let entry = self.page_table.get_entry(addr);
+            // FIXME: Once define `slice`, all modifies of `entry` below will fail.
+            //        Then the loop will never stop.
+            //        The reason may be `get_page_slice_mut()` contains unsafe operation,
+            //        which lead the compiler to do a wrong optimization.
+//            let slice = page_table.get_page_slice_mut(addr);
+            let entry = page_table.get_entry(addr);
+//            println!("{:#x} , {}, {}", addr, entry.accessed(), entry.dirty());
 
             match (entry.accessed(), entry.dirty()) {
                 (true, _) => {
                     entry.clear_accessed();
                 },
                 (false, true) => {
-                    if self.page_table.swap_out(addr).is_ok() {
+                    // FIXME:
-                        entry.clear_dirty();
+//                    if let Ok(token) = swapper.swap_out(slice) {
-                    }
+//                        entry.clear_dirty();
+//                    }
                 },
                 _ => {
                     return self.remove_current();
@@ -50,14 +61,7 @@ impl<T: 'static + SwappablePageTable> SwapManager for EnhancedClockSwapManager<T
 }
 
 
-impl<T: 'static + SwappablePageTable> EnhancedClockSwapManager<T> {
+impl EnhancedClockSwapManager {
-    pub fn new(page_table: &'static mut T) -> Self {
-        EnhancedClockSwapManager {
-            page_table,
-            clock_ptr: 0,
-            deque: VecDeque::<VirtAddr>::new()
-        }
-    }
     fn remove_current(&mut self) -> Option<VirtAddr> {
         let addr = self.deque.remove(self.clock_ptr);
         if self.clock_ptr == self.deque.len() {
@@ -76,45 +80,12 @@ impl<T: 'static + SwappablePageTable> EnhancedClockSwapManager<T> {
 #[cfg(test)]
 mod test {
     use super::*;
-    use alloc::{arc::Arc, boxed::Box};
+    use swap::test::*;
-    use core::mem::uninitialized;
-    use core::cell::RefCell;
-    use paging::MockPageTable;
-
-    impl SwappablePageTable for MockPageTable {
-        fn swap_out(&mut self, addr: usize) -> Result<(), ()> {
-            Ok(())
-        }
-    }
-
-    enum MemOp {
-        R(usize), W(usize)
-    }
 
     #[test]
     fn test() {
         use self::MemOp::{R, W};
-        let page_fault_count = Arc::new(RefCell::new(0usize));
+        let ops = [
-
-        let mut pt = Box::new(MockPageTable::new(4));
-        let static_pt = unsafe{ &mut *(pt.as_mut() as *mut MockPageTable) };
-        pt.set_handler(Box::new({
-            let page_fault_count1 = page_fault_count.clone();
-            let mut clock = EnhancedClockSwapManager::new(static_pt);
-
-            move |pt: &mut MockPageTable, addr: VirtAddr| {
-                *page_fault_count1.borrow_mut() += 1;
-
-                if !pt.map(addr) {  // is full?
-                    pt.unmap(clock.pop().unwrap());
-                    pt.map(addr);
-                }
-                clock.push(addr);
-            }
-        }));
-
-
-        let op_seq = [
             R(0x1000), R(0x2000), R(0x3000), R(0x4000),
             R(0x3000), W(0x1000), R(0x4000), W(0x2000), R(0x5000),
             R(0x2000), W(0x1000), R(0x2000), R(0x3000), R(0x4000)];
@@ -122,12 +93,6 @@ mod test {
             1, 2, 3, 4,
             4, 4, 4, 4, 5,
             5, 5, 5, 6, 7];
-        for (op, &count) in op_seq.iter().zip(pgfault_count.iter()) {
+        test_manager(EnhancedClockSwapManager::default(), &ops, &pgfault_count);
-            match op {
-                R(addr) => {pt.read(*addr);},
-                W(addr) => pt.write(*addr, 0),
-            }
-            assert_eq!(*(*page_fault_count).borrow(), count);
-        }
     }
 }

crate/memory/src/swap/fifo.rs

@@ -1,14 +1,13 @@
 use alloc::vec_deque::VecDeque;
 use super::*;
 
+#[derive(Default)]
 pub struct FifoSwapManager {
     deque: VecDeque<VirtAddr>,
 }
 
 impl SwapManager for FifoSwapManager {
-    fn tick(&mut self) {
+    fn tick(&mut self) {}
-
-    }
 
     fn push(&mut self, addr: usize) {
         self.deque.push_back(addr);
@@ -21,52 +20,22 @@ impl SwapManager for FifoSwapManager {
         self.deque.remove(id);
     }
 
-    fn pop(&mut self) -> Option<VirtAddr> {
+    fn pop<T, S>(&mut self, _: &mut T, _: &mut S) -> Option<VirtAddr>
+        where T: PageTable, S: Swapper
+    {
         self.deque.pop_front()
     }
 }
 
-impl FifoSwapManager {
-    pub fn new() -> Self {
-        FifoSwapManager {
-            deque: VecDeque::<VirtAddr>::new()
-        }
-    }
-}
-
 #[cfg(test)]
 mod test {
     use super::*;
-    use alloc::{arc::Arc, boxed::Box};
+    use swap::test::*;
-    use core::cell::RefCell;
-    use paging::MockPageTable;
-
-    enum MemOp {
-        R(usize), W(usize)
-    }
 
     #[test]
     fn test() {
         use self::MemOp::{R, W};
-        let page_fault_count = Arc::new(RefCell::new(0usize));
+        let ops = [
-
-        let mut pt = MockPageTable::new(4);
-        pt.set_handler(Box::new({
-            let page_fault_count1 = page_fault_count.clone();
-            let mut fifo = FifoSwapManager::new();
-
-            move |pt: &mut MockPageTable, addr: VirtAddr| {
-                *page_fault_count1.borrow_mut() += 1;
-
-                if !pt.map(addr) {  // is full?
-                    pt.unmap(fifo.pop().unwrap());
-                    pt.map(addr);
-                }
-                fifo.push(addr);
-            }
-        }));
-
-        let op_seq = [
             R(0x1000), R(0x2000), R(0x3000), R(0x4000),
             W(0x3000), W(0x1000), W(0x4000), W(0x2000), W(0x5000),
             W(0x2000), W(0x1000), W(0x2000), W(0x3000), W(0x4000),
@@ -76,12 +45,6 @@ mod test {
             4, 4, 4, 4, 5,
             5, 6, 7, 8, 9,
             10, 11, 11];
-        for (op, &count) in op_seq.iter().zip(pgfault_count.iter()) {
+        test_manager(FifoSwapManager::default(), &ops, &pgfault_count);
-            match op {
-                R(addr) => {pt.read(*addr);},
-                W(addr) => pt.write(*addr, 0),
-            }
-            assert_eq!(*(*page_fault_count).borrow(), count);
-        }
     }
 }

crate/memory/src/swap/mod.rs

@@ -3,10 +3,10 @@ use super::paging::*;
 use core::ops::{Deref, DerefMut};
 
 //pub use self::fifo::FifoSwapManager;
-//pub use self::enhanced_clock::EnhancedClockSwapManager;
+pub use self::enhanced_clock::EnhancedClockSwapManager;
 
-//mod fifo;
+mod fifo;
-//mod enhanced_clock;
+mod enhanced_clock;
 #[cfg(test)]
 mod mock_swapper;
 
@@ -19,9 +19,12 @@ pub trait SwapManager {
     /// Called to delete the addr entry from the swap manager
     fn remove(&mut self, addr: VirtAddr);
     /// Try to swap out a page, return then victim
-    fn pop(&mut self) -> Option<VirtAddr>;
+    /// (The params is only used by `EnhancedClockSwapManager`)
+    fn pop<T, S>(&mut self, page_table: &mut T, swapper: &mut S) -> Option<VirtAddr>
+        where T: PageTable, S: Swapper;
 }
 
+/// Do swap in & out
 pub trait Swapper {
     /// Allocate space on device and write data to it.
     /// Return a token indicating the location.
@@ -47,9 +50,17 @@ impl<T: PageTable, M: SwapManager, S: Swapper> SwapExt<T, M, S> {
             swapper,
         }
     }
+    pub fn map_to_swappable(&mut self, addr: VirtAddr, target: PhysAddr) -> &mut T::Entry {
+        self.swap_manager.push(addr);
+        self.map(addr, target)
+    }
     /// Swap out any one of the swapped pages, return the released PhysAddr.
     pub fn swap_out_any(&mut self) -> Result<PhysAddr, SwapError> {
-        match self.swap_manager.pop() {
+        let victim = {
+            let Self {ref mut page_table, ref mut swap_manager, ref mut swapper} = self;
+            swap_manager.pop(page_table, swapper)
+        };
+        match victim {
             None => Err(SwapError::NoSwapped),
             Some(addr) => self.swap_out(addr),
         }
@@ -85,16 +96,16 @@ impl<T: PageTable, M: SwapManager, S: Swapper> SwapExt<T, M, S> {
         };
         let data = self.page_table.get_page_slice_mut(addr);
         self.swapper.swap_in(token, data).map_err(|_| SwapError::IOError)?;
+        self.swap_manager.push(addr);
         Ok(())
     }
-    pub fn page_fault_handler(&mut self, addr: VirtAddr, alloc_frame: impl FnOnce() -> PhysAddr) -> bool {
+    pub fn page_fault_handler(&mut self, addr: VirtAddr, alloc_frame: impl FnOnce() -> Option<PhysAddr>) -> bool {
-        {
+        if !self.page_table.get_entry(addr).swapped() {
-            let entry = self.page_table.get_entry(addr);
+            return false;
-            if !entry.swapped() {
-                return false;
-            }
         }
-        self.swap_in(addr, alloc_frame()).ok().unwrap();
+        // Allocate a frame, if failed, swap out a page
+        let frame = alloc_frame().unwrap_or_else(|| self.swap_out_any().ok().unwrap());
+        self.swap_in(addr, frame).ok().unwrap();
         true
     }
 }
@@ -118,4 +129,68 @@ impl<T: PageTable, M: SwapManager, S: Swapper> DerefMut for SwapExt<T, M, S> {
     fn deref_mut(&mut self) -> &mut <Self as Deref>::Target {
         &mut self.page_table
     }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use super::mock_swapper::MockSwapper;
+    use alloc::{arc::Arc, boxed::Box};
+    use core::cell::RefCell;
+    use paging::MockPageTable;
+
+    #[derive(Debug)]
+    pub enum MemOp {
+        R(usize),
+        W(usize),
+    }
+
+    struct FrameAlloc(usize);
+
+    impl FrameAlloc {
+        fn alloc(&mut self) -> Option<PhysAddr> {
+            if self.0 == 0 {
+                return None;
+            }
+            self.0 -= 1;
+            Some((self.0 + 1) * PAGE_SIZE)
+        }
+    }
+
+    unsafe fn clone<'a, 'b, T>(x: &'a mut T) -> &'b mut T {
+        &mut *(x as *mut T)
+    }
+
+    /// Test framework with different SwapManagers.
+    /// See `fifo::test` mod for example.
+    pub fn test_manager(swap_manager: impl 'static + SwapManager, ops: &[MemOp], pgfault_count: &[u8]) {
+        use self::MemOp::{R, W};
+        let page_fault_count = Arc::new(RefCell::new(0u8));
+
+        let mut pt = SwapExt::new(MockPageTable::new(), swap_manager, MockSwapper::default());
+
+        // Move to closure
+        let pt0 = unsafe{ clone(&mut pt) };
+        let page_fault_count1 = page_fault_count.clone();
+        let mut alloc = FrameAlloc(4);
+
+        pt.set_handler(Box::new(move |_, addr: VirtAddr| {
+            *page_fault_count1.borrow_mut() += 1;
+            if pt0.page_fault_handler(addr, || alloc.alloc()) {
+                return;
+            }
+            // The page is not mapped, map it to a new frame, if no more frame, swap out.
+            let target = alloc.alloc().or_else(|| pt0.swap_out_any().ok())
+                .expect("no more frame in both allocator and swap_manager");
+            pt0.map_to_swappable(addr, target);
+        }));
+
+        for (op, &count) in ops.iter().zip(pgfault_count.iter()) {
+            match op {
+                R(addr) => { pt.read(*addr); }
+                W(addr) => pt.write(*addr, 0),
+            }
+            assert_eq!(*(*page_fault_count).borrow(), count);
+        }
+    }
 }