Merge #524

524: Easy 2021 05 01 r=Medowhill a=jeehoonkang

D-easy 다 해결했습니다

Co-authored-by: Jeehoon Kang <[email protected]>

kernel-rs/Cargo.toml

@@ -34,6 +34,7 @@ pin-project = "1.0.7"
 scopeguard = { version = "1.1.0", default-features = false }
 spin = "0.9.0"
 static_assertions = "1.1.0"
+zerocopy = "0.5.0"
 
 # Compiler options for sysroot packages.
 # Cargo currently warns following packages are not dependencies.

kernel-rs/src/arch/memlayout.rs

@@ -15,6 +15,10 @@
 //! 80000000 -- entry.S, then kernel text and data
 //! end -- start of kernel page allocation area
 //! PHYSTOP -- end RAM used by the kernel
+
+// Dead code is allowed in this file because not all components are used in the kernel.
+#![allow(dead_code)]
+
 use crate::arch::addr::{MAXVA, PGSIZE};
 
 /// SiFive Test Finisher. (virt device only)

kernel-rs/src/arch/mod.rs

@@ -1,8 +1,5 @@
 //! Architecture-dependent code.
 
-// TODO(https://github.com/kaist-cp/rv6/issues/120)
-#![allow(dead_code)]
-
 pub mod addr;
 pub mod memlayout;
 pub mod plic;

kernel-rs/src/arch/riscv.rs

@@ -1,3 +1,8 @@
+//! RISC-V instructions.
+
+// Dead code is allowed in this file because not all components are used in the kernel.
+#![allow(dead_code)]
+
 use bitflags::bitflags;
 
 /// Which hart (core) is this?

kernel-rs/src/console.rs

@@ -309,7 +309,7 @@ impl fmt::Write for Printer {
     fn write_str(&mut self, s: &str) -> fmt::Result {
         for c in s.bytes() {
             // TODO(https://github.com/kaist-cp/rv6/issues/267): remove kernel_builder()
-            let kernel = kernel_builder();
+            let kernel = unsafe { kernel_builder() };
             kernel.console.putc_spin(c, &kernel);
         }
         Ok(())

kernel-rs/src/exec.rs

@@ -4,6 +4,7 @@ use core::{cmp, mem};
 
 use bitflags::bitflags;
 use itertools::*;
+use zerocopy::{AsBytes, FromBytes};
 
 use crate::{
     arch::addr::{pgroundup, PAddr, PGSIZE},
@@ -26,6 +27,7 @@ const ELF_PROG_LOAD: u32 = 1;
 // which should follow C(=machine) representation
 // https://github.com/kaist-cp/rv6/issues/52
 #[repr(C)]
+#[derive(AsBytes, FromBytes)]
 struct ElfHdr {
     /// must equal ELF_MAGIC
     magic: u32,
@@ -48,6 +50,7 @@ struct ElfHdr {
 bitflags! {
     /// Flag bits for ProgHdr flags
     #[repr(C)]
+    #[derive(AsBytes, FromBytes)]
     struct ProgFlags: u32 {
         const EXEC = 1;
         const WRITE = 2;
@@ -67,6 +70,7 @@ impl Default for ProgFlags {
 // which should follow C(=machine) representation
 // https://github.com/kaist-cp/rv6/issues/52
 #[repr(C)]
+#[derive(AsBytes, FromBytes)]
 struct ProgHdr {
     typ: u32,
     flags: ProgFlags,
@@ -106,9 +110,7 @@ impl KernelCtx<'_, '_> {
 
         // Check ELF header
         let mut elf: ElfHdr = Default::default();
-        // SAFETY: ElfHdr can be safely transmuted to [u8; _], as it
-        // contains only integers, which do not have internal structures.
-        unsafe { ip.read_kernel(&mut elf, 0, self) }?;
+        ip.read_kernel(&mut elf, 0, self)?;
         if !elf.is_valid() {
             return Err(());
         }
@@ -123,9 +125,7 @@ impl KernelCtx<'_, '_> {
             let off = elf.phoff + i * mem::size_of::<ProgHdr>();
 
             let mut ph: ProgHdr = Default::default();
-            // SAFETY: ProgHdr can be safely transmuted to [u8; _], as it
-            // contains only integers, which do not have internal structures.
-            unsafe { ip.read_kernel(&mut ph, off as _, self) }?;
+            ip.read_kernel(&mut ph, off as _, self)?;
             if ph.is_prog_load() {
                 if ph.memsz < ph.filesz || ph.vaddr % PGSIZE != 0 {
                     return Err(());

kernel-rs/src/file.rs

@@ -220,7 +220,7 @@ impl ArenaObject for File {
                         // TODO(https://github.com/kaist-cp/rv6/issues/290): The inode ip will
                         // be dropped by drop(ip). Deallocation of an inode may cause disk write
                         // operations, so we must begin a transaction here.
-                        let _tx = kref.file_system.begin_transaction();
+                        let _tx = kref.fs().begin_transaction();
                         drop(ip);
                     }
                     _ => (),

kernel-rs/src/fs/inode.rs

@@ -6,7 +6,7 @@
 //! list of blocks holding the file's content.
 //!
 //! The inodes are laid out sequentially on disk at
-//! kernel.file_system.superblock.startinode. Each inode has a number, indicating its
+//! kernel.fs().superblock.startinode. Each inode has a number, indicating its
 //! position on the disk.
 //!
 //! The kernel keeps a table of in-use inodes in memory
@@ -75,6 +75,7 @@ use core::{
 };
 
 use static_assertions::const_assert;
+use zerocopy::{AsBytes, FromBytes};
 
 use super::{FileName, Stat, IPB, MAXFILE, NDIRECT, NINDIRECT};
 use crate::{
@@ -183,7 +184,8 @@ pub struct InodeGuard<'a> {
     pub inode: &'a Inode,
 }
 
-#[derive(Default)]
+#[repr(C)]
+#[derive(Default, AsBytes, FromBytes)]
 pub struct Dirent {
     pub inum: u16,
     name: [u8; DIRSIZ],
@@ -192,9 +194,7 @@ pub struct Dirent {
 impl Dirent {
     fn new(ip: &mut InodeGuard<'_>, off: u32, ctx: &KernelCtx<'_, '_>) -> Result<Dirent, ()> {
         let mut dirent = Dirent::default();
-        // SAFETY: Dirent can be safely transmuted to [u8; _], as it
-        // contains only u16 and u8's, which do not have internal structures.
-        unsafe { ip.read_kernel(&mut dirent, off, ctx) }?;
+        ip.read_kernel(&mut dirent, off, ctx)?;
         Ok(dirent)
     }
 
@@ -324,9 +324,9 @@ impl InodeGuard<'_> {
     /// Must be called after every change to an ip->xxx field
     /// that lives on disk.
     pub fn update(&self, tx: &FsTransaction<'_>, ctx: &KernelCtx<'_, '_>) {
-        let mut bp = ctx.kernel().file_system.log.disk.read(
+        let mut bp = ctx.kernel().fs().log.disk.read(
             self.dev,
-            ctx.kernel().file_system.superblock().iblock(self.inum),
+            ctx.kernel().fs().superblock().iblock(self.inum),
             ctx,
         );
 
@@ -386,7 +386,7 @@ impl InodeGuard<'_> {
         if self.deref_inner().addr_indirect != 0 {
             let mut bp =
                 ctx.kernel()
-                    .file_system
+                    .fs()
                     .log
                     .disk
                     .read(dev, self.deref_inner().addr_indirect, ctx);
@@ -409,22 +409,13 @@ impl InodeGuard<'_> {
 
     /// Copy data into `dst` from the content of inode at offset `off`.
     /// Return Ok(()) on success, Err(()) on failure.
-    ///
-    /// # Safety
-    ///
-    /// `T` can be safely `transmute`d to `[u8; size_of::<T>()]`.
-    pub unsafe fn read_kernel<T>(
+    pub fn read_kernel<T: AsBytes + FromBytes>(
         &mut self,
         dst: &mut T,
         off: u32,
         ctx: &KernelCtx<'_, '_>,
     ) -> Result<(), ()> {
-        let bytes = self.read_bytes_kernel(
-            // SAFETY: the safety assumption of this method.
-            unsafe { core::slice::from_raw_parts_mut(dst as *mut _ as _, mem::size_of::<T>()) },
-            off,
-            ctx,
-        );
+        let bytes = self.read_bytes_kernel(dst.as_bytes_mut(), off, ctx);
         if bytes == mem::size_of::<T>() {
             Ok(())
         } else {
@@ -508,11 +499,12 @@ impl InodeGuard<'_> {
         }
         let mut tot: u32 = 0;
         while tot < n {
-            let bp = k.kernel().file_system.log.disk.read(
-                self.dev,
-                self.bmap(off as usize / BSIZE, &k),
-                &k,
-            );
+            let bp =
+                k.kernel()
+                    .fs()
+                    .log
+                    .disk
+                    .read(self.dev, self.bmap(off as usize / BSIZE, &k), &k);
             let m = core::cmp::min(n - tot, BSIZE as u32 - off % BSIZE as u32);
             let begin = (off % BSIZE as u32) as usize;
             let end = begin + m as usize;
@@ -525,21 +517,14 @@ impl InodeGuard<'_> {
 
     /// Copy data from `src` into the inode at offset `off`.
     /// Return Ok(()) on success, Err(()) on failure.
-    pub fn write_kernel<T>(
+    pub fn write_kernel<T: AsBytes>(
         &mut self,
         src: &T,
         off: u32,
         tx: &FsTransaction<'_>,
         ctx: &KernelCtx<'_, '_>,
     ) -> Result<(), ()> {
-        let bytes = self.write_bytes_kernel(
-            // SAFETY: src is a valid reference to T and
-            // u8 does not have any internal structure.
-            unsafe { core::slice::from_raw_parts(src as *const _ as _, mem::size_of::<T>()) },
-            off,
-            tx,
-            ctx,
-        )?;
+        let bytes = self.write_bytes_kernel(src.as_bytes(), off, tx, ctx)?;
         if bytes == mem::size_of::<T>() {
             Ok(())
         } else {
@@ -631,7 +616,7 @@ impl InodeGuard<'_> {
         }
         let mut tot: u32 = 0;
         while tot < n {
-            let mut bp = k.kernel().file_system.log.disk.read(
+            let mut bp = k.kernel().fs().log.disk.read(
                 self.dev,
                 self.bmap_or_alloc(off as usize / BSIZE, tx, &k),
                 &k,
@@ -703,12 +688,7 @@ impl InodeGuard<'_> {
                 self.deref_inner_mut().addr_indirect = indirect;
             }
 
-            let mut bp = ctx
-                .kernel()
-                .file_system
-                .log
-                .disk
-                .read(self.dev, indirect, ctx);
+            let mut bp = ctx.kernel().fs().log.disk.read(self.dev, indirect, ctx);
             let (prefix, data, _) = unsafe { bp.deref_inner_mut().data.align_to_mut::<u32>() };
             debug_assert_eq!(prefix.len(), 0, "bmap: Buf data unaligned");
             let mut addr = data[bn];
@@ -726,9 +706,8 @@ impl InodeGuard<'_> {
     pub fn is_dir_empty(&mut self, ctx: &KernelCtx<'_, '_>) -> bool {
         let mut de: Dirent = Default::default();
         for off in (2 * DIRENT_SIZE as u32..self.deref_inner().size).step_by(DIRENT_SIZE) {
-            // SAFETY: Dirent can be safely transmuted to [u8; _], as it
-            // contains only u16 and u8's, which do not have internal structures.
-            unsafe { self.read_kernel(&mut de, off, ctx) }.expect("is_dir_empty: read_kernel");
+            self.read_kernel(&mut de, off, ctx)
+                .expect("is_dir_empty: read_kernel");
             if de.inum != 0 {
                 return false;
             }
@@ -771,9 +750,7 @@ impl ArenaObject for Inode {
                 // resulting FsTransaction value is never used. Such transactions
                 // can be found in finalize in file.rs, sys_chdir in sysfile.rs,
                 // close_files in proc.rs, and exec in exec.rs.
-                let tx = mem::ManuallyDrop::new(FsTransaction {
-                    fs: &kernel.file_system,
-                });
+                let tx = mem::ManuallyDrop::new(FsTransaction { fs: &kernel.fs() });
 
                 // self->ref == 1 means no other process can have self locked,
                 // so this acquiresleep() won't block (or deadlock).
@@ -808,9 +785,9 @@ impl Inode {
     pub fn lock(&self, ctx: &KernelCtx<'_, '_>) -> InodeGuard<'_> {
         let mut guard = self.inner.lock();
         if !guard.valid {
-            let mut bp = ctx.kernel().file_system.log.disk.read(
+            let mut bp = ctx.kernel().fs().log.disk.read(
                 self.dev,
-                ctx.kernel().file_system.superblock().iblock(self.inum),
+                ctx.kernel().fs().superblock().iblock(self.inum),
                 ctx,
             );
 
@@ -880,6 +857,7 @@ impl Inode {
                 InodeType::Device { .. } => 3,
             },
             nlink: inner.nlink,
+            _padding: 0,
             size: inner.size as usize,
         }
     }
@@ -915,10 +893,10 @@ impl Itable {
         tx: &FsTransaction<'_>,
         ctx: &KernelCtx<'_, '_>,
     ) -> RcInode {
-        for inum in 1..ctx.kernel().file_system.superblock().ninodes {
-            let mut bp = ctx.kernel().file_system.log.disk.read(
+        for inum in 1..ctx.kernel().fs().superblock().ninodes {
+            let mut bp = ctx.kernel().fs().log.disk.read(
                 dev,
-                ctx.kernel().file_system.superblock().iblock(inum),
+                ctx.kernel().fs().superblock().iblock(inum),
                 ctx,
             );
 

kernel-rs/src/fs/mod.rs

@@ -99,7 +99,7 @@ impl FsTransaction<'_> {
     /// commit()/write_log() will do the disk write.
     ///
     /// write() replaces write(); a typical use is:
-    ///   bp = kernel.file_system.disk.read(...)
+    ///   bp = kernel.fs().disk.read(...)
     ///   modify bp->data[]
     ///   write(bp)
     fn write(&self, b: Buf) {