S5 Fixes (#6)

* S5 Fixes

* Remove #ifdef OLD

* Fix comments for s5_get_disk_block

* Add info for newp return parameter

* Add stencil comment to s5_file_block_to_disk_block

* Remove ifdefno

---------

Co-authored-by: Ayma-n <aymanbt14@gmail.com>

1 files changed, 12 insertions, 55 deletions

diff --git a/kernel/fs/s5fs/s5fs.c b/kernel/fs/s5fs/s5fs.c
index fe2a16c..d1df3a5 100644
--- a/kernel/fs/s5fs/s5fs.c
+++ b/kernel/fs/s5fs/s5fs.c
@@ -195,11 +195,11 @@ long s5fs_mount(fs_t *fs)
  *     - FS_TO_S5FS to obtain the s5fs object
  *     - S5_INODE_BLOCK(vn->v_vno) to determine the block number of the block that
  *       contains the inode info
- *     - s5_get_disk_block and s5_release_disk_block to handle the disk block
+ *     - s5_get_meta_disk_block and s5_release_disk_block to handle the disk block
  *     - S5_INODE_OFFSET to find the desired inode within the disk block
  *       containing it (returns the offset that the inode is stored within the block)
  *  - You should initialize the s5_node_t's inode field by reading directly from
- *    the inode on disk by using the page frame returned from s5_get_disk_block. Also 
+ *    the inode on disk by using the page frame returned from s5_get_meta_disk_block. Also 
  *    make sure to initialize the dirtied_inode field.
  *  - Using the inode info, you need to initialize the following vnode fields:
  *    vn_len, vn_mode, and vn_ops using the fields found in the s5_inode struct.
@@ -268,23 +268,17 @@ static long s5fs_umount(fs_t *fs)
 
 static void s5fs_sync(fs_t *fs)
 {
-#ifdef FIXME
     s5fs_t *s5fs = FS_TO_S5FS(fs);
-    #ifdef OLD
-    mobj_t *mobj = S5FS_TO_VMOBJ(s5fs);
-    #endif
-    mobj_t *mobj = 0;  // XXX FIX ME
-
-    mobj_lock(mobj);
+    mobj_t *mobj = &s5fs->s5f_mobj;
 
     pframe_t *pf;
-    mobj_get_pframe(mobj, S5_SUPER_BLOCK, 1, &pf);
+    s5_get_meta_disk_block(s5fs, S5_SUPER_BLOCK, 1, &pf);
     memcpy(pf->pf_addr, &s5fs->s5f_super, sizeof(s5_super_t));
-    pframe_release(&pf);
+    s5_release_disk_block(&pf);
 
-    mobj_flush(S5FS_TO_VMOBJ(s5fs));
-    mobj_unlock(S5FS_TO_VMOBJ(s5fs));
-#endif
+    mobj_lock(&s5fs->s5f_mobj);
+    mobj_flush(mobj);
+    mobj_unlock(&s5fs->s5f_mobj);
 }
 
 /* Wrapper around s5_read_file. */
@@ -577,21 +571,6 @@ static void s5fs_truncate_file(vnode_t *file)
     vunlock(file); 
 }
 
-#ifdef OLD
-/*
- * Wrapper around mobj_get_pframe. Remember to lock the memory object around
- * the call to mobj_get_pframe. Assert that the get_pframe does not fail.
- */
-inline void s5_get_disk_block(s5fs_t *s5fs, blocknum_t blocknum, long forwrite,
-                              pframe_t **pfp)
-{
-    mobj_lock(S5FS_TO_VMOBJ(s5fs));
-    long ret = mobj_get_pframe(S5FS_TO_VMOBJ(s5fs), blocknum, forwrite, pfp);
-    mobj_unlock(S5FS_TO_VMOBJ(s5fs));
-    KASSERT(!ret && *pfp);
-}
-#endif
-
 /*
  * Wrapper around device's read_block function; first looks up block in file-system cache.
  * If not there, allocates and fills a page frame. 
@@ -605,6 +584,7 @@ inline void s5_get_meta_disk_block(s5fs_t *s5fs, uint64_t blocknum, long forwrit
     if (*pfp)
     {
         // block is cached
+        (*pfp)->pf_dirty |= forwrite;
         mobj_unlock(&s5fs->s5f_mobj);
         return;
     }
@@ -615,7 +595,7 @@ inline void s5_get_meta_disk_block(s5fs_t *s5fs, uint64_t blocknum, long forwrit
 
     blockdev_t *bd = s5fs->s5f_bdev;
     long ret = bd->bd_ops->read_block(bd, pf->pf_addr, (blocknum_t)pf->pf_loc, 1);
-    pf->pf_dirty |= forwrite;  // needed?
+    pf->pf_dirty |= forwrite;  // yes, needed
     KASSERT (!ret);
     mobj_unlock(&s5fs->s5f_mobj);
     KASSERT(!ret && *pfp);
@@ -637,7 +617,7 @@ static inline void s5_get_file_disk_block(vnode_t *vnode, uint64_t blocknum, uin
     KASSERT(pf->pf_addr);
     blockdev_t *bd = VNODE_TO_S5FS(vnode)->s5f_bdev;
     long ret = bd->bd_ops->read_block(bd, pf->pf_addr, pf->pf_loc, 1);
-    pf->pf_dirty |= forwrite;  // needed?
+    pf->pf_dirty |= forwrite; // yes, needed
     KASSERT (!ret);
 }
 
@@ -680,36 +660,13 @@ inline void s5_release_disk_block(pframe_t **pfp) { pframe_release(pfp); }
 static long s5fs_get_pframe(vnode_t *vnode, uint64_t pagenum, long forwrite,
                             pframe_t **pfp)
 {
-#ifdef OLD
-    if (vnode->vn_len <= pagenum * PAGE_SIZE)
-        return -EINVAL;
-    long loc =
-        s5_file_block_to_disk_block(VNODE_TO_S5NODE(vnode), pagenum, forwrite);
-    if (loc < 0)
-        return loc;
-    if (loc)
-    {
-        mobj_find_pframe(&vnode->vn_mobj, pagenum, pfp);
-        if (*pfp)
-        {
-            mobj_free_pframe(&vnode->vn_mobj, pfp);
-        }
-        s5_get_disk_block(VNODE_TO_S5FS(vnode), (blocknum_t)loc, forwrite, pfp);
-        return 0;
-    }
-    else
-    {
-        KASSERT(!forwrite);
-        return mobj_default_get_pframe(&vnode->vn_mobj, pagenum, forwrite, pfp);
-    }
-#endif
-
     if (vnode->vn_len <= pagenum * PAGE_SIZE)
         return -EINVAL;
     mobj_find_pframe(&vnode->vn_mobj, pagenum, pfp);
     if (*pfp)
     {
         // block is cached
+        (*pfp)->pf_dirty |= forwrite;
         return 0;
     }
     int new;