fix after review

Author: Medowhill

kernel-rs/src/kalloc.rs

@@ -4,7 +4,6 @@
 use core::{mem, pin::Pin};
 
 use pin_project::pin_project;
-use static_assertions::const_assert;
 
 use crate::{
     list::{List, ListEntry, ListNode},
@@ -56,6 +55,12 @@ unsafe impl ListNode for Run {
 /// # Safety
 ///
 /// The address of each `Run` in `runs` can become a `Page` by `Page::from_usize`.
+// This implementation defers from xv6. Kmem of xv6 uses intrusive singly linked list, while this
+// Kmem uses List, which is a intrusive doubly linked list type of rv6. In a intrusive singly
+// linked list, it is impossible to automatically remove an entry from a list when it is dropped.
+// Therefore, it is nontrivial to make a general intrusive singly linked list type in a safe way.
+// For this reason, we use a doubly linked list instead. It adds runtime overhead, but the overhead
+// seems negligible.
 #[pin_project]
 pub struct Kmem {
     #[pin]
@@ -100,11 +105,7 @@ impl Kmem {
         // Fill with junk to catch dangling refs.
         page.write_bytes(1);
 
-        const_assert!(mem::size_of::<Run>() <= PGSIZE);
-        const_assert!(PGSIZE % mem::align_of::<Run>() == 0);
-
-        // SAFETY: the above `const_assert`s.
-        let run = unsafe { page.as_uninit_mut() };
+        let run = page.as_uninit_mut();
         // SAFETY: `run` will be initialized by the following `init`.
         let run = run.write(unsafe { Run::new() });
         let mut run = unsafe { Pin::new_unchecked(run) };

kernel-rs/src/page.rs

@@ -73,11 +73,20 @@ impl Page {
         }
     }
 
-    /// # Safety
-    ///
-    /// * `mem::size_of::<T>() <= PGSIZE`
-    /// * `PGSIZE % mem::align_of::<T>() == 0`
-    pub unsafe fn as_uninit_mut<T>(&mut self) -> &mut MaybeUninit<T> {
+    pub fn as_uninit_mut<T>(&mut self) -> &mut MaybeUninit<T> {
+        // TODO(https://github.com/kaist-cp/rv6/issues/471):
+        // Use const_assert! (or equivalent) instead. Currently, use of T inside const_assert!
+        // incurs a compile error: "can't use generic parameters from outer function".
+        // Also, there's a workaround using feature(const_generics) and
+        // feature(const_evaluatable_checked). However, using them makes the compiler panic.
+        // When the compiler becomes updated, we will fix the following lines to use static checks.
+        assert!(mem::size_of::<T>() <= PGSIZE);
+        assert_eq!(PGSIZE % mem::align_of::<T>(), 0);
+
+        // SAFETY: self.inner is an array of length PGSIZE aligned with PGSIZE bytes.
+        // The above assertions show that it can contain a value of T. As it contains arbitrary
+        // data, we cannot treat it as &mut T. Instead, we use &mut MaybeUninit<T>. It's ok because
+        // T and MaybeUninit<T> have the same size, alignment, and ABI.
         unsafe { &mut *(self.inner.as_ptr() as *mut MaybeUninit<T>) }
     }
 }

kernel-rs/src/pipe.rs

@@ -136,14 +136,10 @@ impl Kernel {
     pub fn allocate_pipe(&self) -> Result<(RcFile, RcFile), ()> {
         let page = self.kmem.alloc().ok_or(())?;
         let mut page = scopeguard::guard(page, |page| self.kmem.free(page));
+        let ptr = page.as_uninit_mut();
 
-        const_assert!(mem::size_of::<Pipe>() <= PGSIZE);
-        const_assert!(PGSIZE % mem::align_of::<Pipe>() == 0);
-
-        // SAFETY: the above `const_assert!`s.
-        let ptr = unsafe { page.as_uninit_mut() };
-
-        //TODO(https://github.com/kaist-cp/rv6/issues/367): Since Pipe is a huge struct, need to check whether stack is used to fill `*ptr`.
+        // TODO(https://github.com/kaist-cp/rv6/issues/367):
+        // Since Pipe is a huge struct, need to check whether stack is used to fill `*ptr`.
         let ptr = NonNull::from(ptr.write(Pipe {
             inner: Spinlock::new(
                 "pipe",