Add Pipe::Drop.

kernel-rs/src/file.rs

@@ -178,7 +178,7 @@ impl ArenaObject for File {
         A::reacquire_after(guard, || {
             let typ = mem::replace(&mut self.typ, FileType::None);
             match typ {
-                FileType::Pipe { mut pipe } => unsafe { pipe.close(self.writable) },
+                FileType::Pipe { pipe } => pipe.close(self.writable),
                 FileType::Inode { ip, .. } | FileType::Device { ip, .. } => {
                     // TODO(rv6)
                     // The inode ip will be dropped by drop(ip). Deallocation

kernel-rs/src/kernel.rs

@@ -59,8 +59,6 @@ pub struct Kernel {
 
     /// Memory for virtio descriptors `&c` for queue 0.
     ///
-    /// This is a global instead of allocated because it must be multiple contiguous pages, which
-    /// `kernel().alloc()` doesn't support, and page aligned.
     // TODO(efenniht): I moved out pages from Disk. Did I changed semantics (pointer indirection?)
     virtqueue: [RawPage; 2],
 

kernel-rs/src/pipe.rs

@@ -84,7 +84,7 @@ impl Pipe {
         }
     }
 
-    unsafe fn close(&self, writable: bool) -> bool {
+    pub fn close(&self, writable: bool) {
         let mut inner = self.inner.lock();
 
         if writable {
@@ -94,22 +94,24 @@ impl Pipe {
             inner.readopen = false;
             self.write_waitchannel.wakeup();
         }
+    }
+
+    fn closed(&self) -> bool {
+        let inner = self.inner.lock();
 
-        // Return whether pipe would be freed or not
+        // Return whether pipe would be freed or not.
         !inner.readopen && !inner.writeopen
     }
 }
 
-// TODO: Remove Copy and Clone
-#[derive(Copy, Clone)]
 pub struct AllocatedPipe {
-    ptr: *mut Pipe,
+    pipe: &'static Pipe,
 }
 
 impl Deref for AllocatedPipe {
     type Target = Pipe;
     fn deref(&self) -> &Self::Target {
-        unsafe { &*self.ptr }
+        self.pipe
     }
 }
 
@@ -118,7 +120,7 @@ impl AllocatedPipe {
         let page = kernel().alloc().ok_or(())?;
         let ptr = page.into_usize() as *mut Pipe;
 
-        // `Pipe` must be align with `Page`
+        // `Pipe` must be aligned with `Page`.
         const_assert!(mem::size_of::<Pipe>() <= PGSIZE);
 
         //TODO(rv6): Since Pipe is a huge struct, need to check whether stack is used to fill `*ptr`
@@ -145,25 +147,29 @@ impl AllocatedPipe {
         };
         let f0 = kernel()
             .ftable
-            .alloc_file(FileType::Pipe { pipe: Self { ptr } }, true, false)
+            // Safe since we already wrote a new `Pipe` to `ptr` using `ptr::write()`.
+            .alloc_file(FileType::Pipe { pipe: unsafe { Self { pipe: &*ptr } } }, true, false)
             // It is safe because ptr is an address of page, which obtained by alloc()
             .map_err(|_| kernel().free(unsafe { Page::from_usize(ptr as _) }))?;
         let f1 = kernel()
             .ftable
-            .alloc_file(FileType::Pipe { pipe: Self { ptr } }, false, true)
+            // Safe since we already wrote a new `Pipe` to `ptr` using `ptr::write()`.
+            .alloc_file(FileType::Pipe { pipe: unsafe { Self { pipe: &*ptr } } }, false, true)
             // It is safe because ptr is an address of page, which obtained by alloc()
             .map_err(|_| kernel().free(unsafe { Page::from_usize(ptr as _) }))?;
 
         Ok((f0, f1))
     }
+}
 
-    // TODO: use `Drop` instead of `close`
-    // TODO: use `self` instead of `&mut self`
-    // `&mut self` is used because `Drop` of `File` uses AllocatedPipe inside File.
-    // https://github.com/kaist-cp/rv6/pull/211#discussion_r491671723
-    pub unsafe fn close(&mut self, writable: bool) {
-        if (*self.ptr).close(writable) {
-            kernel().free(Page::from_usize(self.ptr as *mut Pipe as _));
+impl Drop for AllocatedPipe {
+    fn drop(&mut self) {
+        if self.pipe.closed() {
+            unsafe {
+                // Safe since this `self.pipe` is stored at a page
+                // that was allocated through `kernel().alloc()`.
+                kernel().free(Page::from_usize(self.pipe as *const Pipe as _));
+            }
         }
     }
 }