Ext2 uses the page cache for block metadata
authorBarret Rhoden <brho@cs.berkeley.edu>
Wed, 29 Sep 2010 01:56:21 +0000 (18:56 -0700)
committerKevin Klues <klueska@cs.berkeley.edu>
Thu, 3 Nov 2011 00:35:54 +0000 (17:35 -0700)
This is a cool commit - everything just worked (or seemed to), including
the untested readpage().  Just say it has a page mapping, then call
pm_load_page(), and you get an incref'd page back.

kern/include/pagemap.h
kern/include/vfs.h
kern/src/blockdev.c
kern/src/ext2fs.c
kern/src/mm.c
kern/src/pagemap.c
kern/src/vfs.c

index 56dd9c2..e302bf3 100644 (file)
@@ -58,5 +58,6 @@ void pm_init(struct page_map *pm, struct page_map_operations *op, void *host);
 struct page *pm_find_page(struct page_map *pm, unsigned long index);
 int pm_insert_page(struct page_map *pm, unsigned long index, struct page *page);
 int pm_remove_page(struct page_map *pm, struct page *page);
+int pm_load_page(struct page_map *pm, unsigned long index, struct page **pp);
 
 #endif /* ROS_KERN_PAGEMAP_H */
index f2b33ec..6b16ead 100644 (file)
@@ -449,7 +449,6 @@ int do_mkdir(char *path, int mode);
 int do_rmdir(char *path);
 struct file *dentry_open(struct dentry *dentry, int flags);
 void file_release(struct kref *kref);
-int file_load_page(struct file *file, unsigned long index, struct page **pp);
 
 /* Process-related File management functions */
 struct file *get_file_from_fd(struct files_struct *open_files, int fd);
index f290290..2d7399e 100644 (file)
@@ -167,6 +167,7 @@ int block_readpage(struct page_map *pm, struct page *page)
        breq->bhs[0] = (struct buffer_head*)page->pg_private;
        breq->nr_bhs = 1;
        retval = make_request(bdev, breq);
+       /* TODO: (BLK) this assumes we slept til the request was done */
        assert(!retval);
        /* after the data is read, we mark it up to date and unlock the page. */
        page->pg_flags |= PG_UPTODATE;
index b3bcda1..e2998bc 100644 (file)
@@ -13,6 +13,7 @@
 #include <kref.h>
 #include <endian.h>
 #include <error.h>
+#include <pmap.h>
 
 /* These structs are declared again and initialized farther down */
 struct page_map_operations ext2_pm_op;
@@ -80,17 +81,55 @@ void *__ext2_read_block(struct block_device *bdev, int block_num, int blocksize)
        return buffer;
 }
 
-/* Free whatever is returned with kfree(), pending proper buffer management.
- * Ext2's superblock is always in the same spot, starting at byte 1024 and is
- * 1024 bytes long. */
-struct ext2_sb *ext2_read_sb(struct block_device *bdev)
+/* TODO: pull these metablock functions out of ext2 */
+/* Makes sure the FS block of metadata is in memory.  This returns a pointer to
+ * the beginning of the requested block.  Release it with put_metablock().
+ * Internally, the kreffing is done on the page. */
+void *__ext2_get_metablock(struct block_device *bdev, unsigned long blk_num,
+                           unsigned int blk_sz)
+{
+       struct page *page;
+       struct page_map *pm = &bdev->b_pm;
+       unsigned int blk_per_pg = PGSIZE / blk_sz;
+       unsigned int blk_offset = (blk_num % blk_per_pg) * blk_sz;
+       int error;
+       assert(blk_offset < PGSIZE);
+       error = pm_load_page(pm, blk_num / blk_per_pg, &page); 
+       if (error) {
+               warn("Failed to read metablock! (%d)", error);
+               return 0;
+       }
+       /* return where we are within the page for the given block */
+       return page2kva(page) + blk_offset;
+}
+
+/* Convenience wrapper */
+void *ext2_get_metablock(struct super_block *sb, unsigned long block_num)
 {
-       return (struct ext2_sb*)__ext2_read_block(bdev, 1, 1024);
+       return __ext2_get_metablock(sb->s_bdev, block_num, sb->s_blocksize);
 }
-       
-/* Raw access to an FS block */
-void *ext2_read_block(struct super_block *sb, unsigned int block_num)
+
+/* Decrefs the buffer from get_metablock().  Call this when you no longer
+ * reference your metadata block/buffer */
+void ext2_put_metablock(void *buffer)
+{
+       page_decref(kva2page(buffer));
+}
+
+/* Will dirty the block/BH/page for the given metadata block/buffer.  Will have
+ * to be careful with the page reclaimer - if someone holds a reference, they
+ * can still dirty it. */
+void ext2_dirty_metablock(void *buffer)
+{
+       struct page *page = kva2page(buffer);
+       /* TODO: race on flag modification, and consider dirtying the BH. */
+       page->pg_flags |= PG_DIRTY;
+}
+
+/* Reads a block of file data.  TODO: Function name and guts will change soon */
+void *ext2_read_fileblock(struct super_block *sb, unsigned int block_num)
 {
+       /* note, we might get rid of this read block, if all files use pages */
        return __ext2_read_block(sb->s_bdev, block_num, sb->s_blocksize);
 }
 
@@ -101,16 +140,15 @@ void *ext2_read_block(struct super_block *sb, unsigned int block_num)
  * indirection tables, and returns where you should be for the next one.  Reach
  * is how many items the current table's *items* can index (so if we're on a
  * 3x indir block, reach should be for the doubly-indirect entries, and
- * rel_inoblk will tell you where within that double block you want).
- * TODO: buffer/page cache this shit. */
+ * rel_inoblk will tell you where within that double block you want). */
 static void ext2_walk_inotable(struct inode *inode, unsigned int *blkid,
                                unsigned int *rel_inoblk, unsigned int reach)
 {
-       uint32_t *blk_buf = ext2_read_block(inode->i_sb, *blkid);
+       uint32_t *blk_buf = ext2_get_metablock(inode->i_sb, *blkid);
        assert(blk_buf);
        *blkid = le32_to_cpu(blk_buf[*rel_inoblk / reach]);
        *rel_inoblk = *rel_inoblk % reach;
-       kfree(blk_buf);
+       ext2_put_metablock(blk_buf);
 }
 
 /* Reads a file's block, determined via walking the inode's tables.  The general
@@ -158,7 +196,7 @@ void *ext2_read_ino_block(struct inode *inode, unsigned int ino_block)
                /* Direct block, straight out of the inode */
                blkid = e2ii->i_block[ino_block];
        }
-       return ext2_read_block(inode->i_sb, blkid);
+       return ext2_read_fileblock(inode->i_sb, blkid);
 }
 
 /* This checks an ext2 disc SB for consistency, optionally printing out its
@@ -260,7 +298,9 @@ struct super_block *ext2_get_sb(struct fs_type *fs, int flags,
        }
        bdev = get_bdev(dev_name);
        assert(bdev);
-       e2sb = ext2_read_sb(bdev);
+       /* Read the SB.  It's always at byte 1024 and 1024 bytes long.  Note we do
+        * not put the metablock (we pin it off the sb later).  Same with e2bg. */
+       e2sb = (struct ext2_sb*)__ext2_get_metablock(bdev, 1, 1024);
        if (!(le16_to_cpu(e2sb->s_magic) == EXT2_SUPER_MAGIC)) {
                warn("EXT2 Not detected when it was expected!");
                return 0;
@@ -268,7 +308,7 @@ struct super_block *ext2_get_sb(struct fs_type *fs, int flags,
        /* Read in the block group descriptor table.  Which block the BG table is on
         * depends on the blocksize */
        unsigned int blksize = 1024 << le32_to_cpu(e2sb->s_log_block_size);
-       e2bg = __ext2_read_block(bdev, blksize == 1024 ? 2 : 1, blksize);
+       e2bg = __ext2_get_metablock(bdev, blksize == 1024 ? 2 : 1, blksize);
        assert(e2bg);
        ext2_check_sb(e2sb, e2bg, FALSE);
 
@@ -389,7 +429,7 @@ void ext2_read_inode(struct inode *inode)
        /* Figure out which FS block of the inode table we want and read in that
         * chunk */
        my_ino_blk = le32_to_cpu(my_bg->bg_inode_table) + bg_idx / ino_per_blk;
-       ino_tbl_chunk = ext2_read_block(inode->i_sb, my_ino_blk);
+       ino_tbl_chunk = ext2_get_metablock(inode->i_sb, my_ino_blk);
        my_ino = &ino_tbl_chunk[bg_idx % ino_per_blk];
 
        /* Have the disk inode now, let's put its info into the VFS inode: */
@@ -431,14 +471,15 @@ void ext2_read_inode(struct inode *inode)
        for (int i = 0; i < 15; i++)
                e2ii->i_block[i] = le32_to_cpu(my_ino->i_block[i]);
        /* TODO: (HASH) unused: inode->i_hash add to hash (saves on disc reading) */
-       /* TODO: (BUF) we could consider saving a pointer to the disk inode and
-        * pinning its buffer in memory, but for now we'll just free it */
-       kfree(ino_tbl_chunk);
+       /* TODO: we could consider saving a pointer to the disk inode and pinning
+        * its buffer in memory, but for now we'll just free it. */
+       ext2_put_metablock(ino_tbl_chunk);
 }
 
 /* called when an inode in memory is modified (journalling FS's care) */
 void ext2_dirty_inode(struct inode *inode)
 {
+       // presumably we'll ext2_dirty_metablock(void *buffer) here
 }
 
 /* write the inode to disk (specifically, to inode inode->i_ino), synchronously
index 5ac0f0d..1fe9d35 100644 (file)
@@ -583,7 +583,7 @@ int __handle_page_fault(struct proc *p, uintptr_t va, int prot)
                 * this stuff so we aren't hold the lock as excessively as we are, and
                 * such that we can block and resume later. */
                f_idx = (va - vmr->vm_base + vmr->vm_foff) >> PGSHIFT;
-               retval = file_load_page(vmr->vm_file, f_idx, &a_page);
+               retval = pm_load_page(vmr->vm_file->f_mapping, f_idx, &a_page);
                if (retval)
                        return retval;
                /* If we want a private map that is writable, we'll preemptively give
index d980f91..09a44a0 100644 (file)
@@ -65,7 +65,9 @@ int pm_remove_page(struct page_map *pm, struct page *page)
        void *retval;
        warn("pm_remove_page() hasn't been thought through or tested.");
        /* TODO: check for dirty pages, don't let them be removed right away.  Need
-        * to schedule them for writeback, and then remove them later (callback). */
+        * to schedule them for writeback, and then remove them later (callback).
+        * Also, need to be careful - anyone holding a reference to a page can dirty
+        * it concurrently. */
        spin_lock(&pm->pm_tree_lock);
        retval = radix_delete(&pm->pm_tree, page->pg_index);
        spin_unlock(&pm->pm_tree_lock);
@@ -74,3 +76,73 @@ int pm_remove_page(struct page_map *pm, struct page *page)
        pm->pm_num_pages--;
        return 0;
 }
+
+/* Makes sure the index'th page of the mapped object is loaded in the page cache
+ * and returns its location via **pp.  Note this will give you a refcnt'd
+ * reference to the page.  This may block! TODO: (BLK) */
+int pm_load_page(struct page_map *pm, unsigned long index, struct page **pp)
+{
+       struct page *page;
+       int error;
+       bool page_was_mapped = TRUE;
+
+       page = pm_find_page(pm, index);
+       while (!page) {
+               /* kpage_alloc, since we want the page to persist after the proc
+                * dies (can be used by others, until the inode shuts down). */
+               if (kpage_alloc(&page))
+                       return -ENOMEM;
+               /* might want to initialize other things, perhaps in page_alloc() */
+               page->pg_flags = 0;
+               error = pm_insert_page(pm, index, page);
+               switch (error) {
+                       case 0:
+                               page_was_mapped = FALSE;
+                               break;
+                       case -EEXIST:
+                               /* the page was mapped already (benign race), just get rid of
+                                * our page and try again (the only case that uses the while) */
+                               page_decref(page);
+                               page = pm_find_page(pm, index);
+                               break;
+                       default:
+                               /* something is wrong, bail out! */
+                               page_decref(page);
+                               return error;
+               }
+       }
+       assert(page && kref_refcnt(&page->pg_kref));
+       /* At this point, page is a refcnt'd page, and we return the reference.
+        * Also, there's an unlikely race where we're not in the page cache anymore,
+        * and this all is useless work. */
+       *pp = page;
+       /* if the page was in the map, we need to do some checks, and might have to
+        * read in the page later.  If the page was freshly inserted to the pm by
+        * us, we skip this since we are the one doing the readpage(). */
+       if (page_was_mapped) {
+               /* is it already here and up to date?  if so, we're done */
+               if (page->pg_flags & PG_UPTODATE)
+                       return 0;
+               /* if not, try to lock the page (could BLOCK) */
+               lock_page(page);
+               /* we got it, is our page still in the cache?  check the mapping.  if
+                * not, start over, perhaps with EAGAIN and outside support */
+               if (!page->pg_mapping)
+                       panic("Page is not in the mapping!  Haven't implemented this!");
+               /* double check, are we up to date?  if so, we're done */
+               if (page->pg_flags & PG_UPTODATE) {
+                       unlock_page(page);
+                       return 0;
+               }
+       }
+       /* if we're here, the page is locked by us, and it needs to be read in */
+       assert(page->pg_mapping == pm);
+       error = pm->pm_op->readpage(pm, page);
+       assert(!error);
+       /* Try to sleep on the IO.  The page will be unlocked when the IO is done */
+       /* TODO: (BLK) this assumes we never slept til we got here */
+       lock_page(page);
+       unlock_page(page);
+       assert(page->pg_flags & PG_UPTODATE);
+       return 0;
+}
index 9e0f87a..eff652d 100644 (file)
@@ -1134,7 +1134,7 @@ ssize_t generic_file_read(struct file *file, char *buf, size_t count,
        /* For each file page, make sure it's in the page cache, then copy it out.
         * TODO: will probably need to consider concurrently truncated files here.*/
        for (int i = first_idx; i <= last_idx; i++) {
-               error = file_load_page(file, i, &page);
+               error = pm_load_page(file->f_mapping, i, &page);
                assert(!error); /* TODO: handle ENOMEM and friends */
                copy_amt = MIN(PGSIZE - page_off, buf_end - buf);
                /* TODO: (UMEM) think about this.  if it's a user buffer, we're relying
@@ -1184,7 +1184,7 @@ ssize_t generic_file_write(struct file *file, const char *buf, size_t count,
        buf_end = buf + count;
        /* For each file page, make sure it's in the page cache, then write it.*/
        for (int i = first_idx; i <= last_idx; i++) {
-               error = file_load_page(file, i, &page);
+               error = pm_load_page(file->f_mapping, i, &page);
                assert(!error); /* TODO: handle ENOMEM and friends */
                copy_amt = MIN(PGSIZE - page_off, buf_end - buf);
                /* TODO: (UMEM) (KFOP) think about this.  if it's a user buffer, we're
@@ -1701,75 +1701,6 @@ void file_release(struct kref *kref)
        kmem_cache_free(file_kcache, file);
 }
 
-/* Makes sure the index'th page from file is loaded in the page cache and
- * returns its location via **pp.  Note this will give you a refcnt'd reference.
- * This may block! TODO: (BLK) */
-int file_load_page(struct file *file, unsigned long index, struct page **pp)
-{
-       struct page_map *pm = file->f_mapping;
-       struct page *page;
-       int error;
-       bool page_was_mapped = TRUE;
-
-       page = pm_find_page(pm, index);
-       while (!page) {
-               /* kpage_alloc, since we want the page to persist after the proc
-                * dies (can be used by others, until the inode shuts down). */
-               if (kpage_alloc(&page))
-                       return -ENOMEM;
-               /* might want to initialize other things, perhaps in page_alloc() */
-               page->pg_flags = 0;
-               error = pm_insert_page(pm, index, page);
-               switch (error) {
-                       case 0:
-                               page_was_mapped = FALSE;
-                               break;
-                       case -EEXIST:
-                               /* the page was mapped already (benign race), just get rid of
-                                * our page and try again (the only case that uses the while) */
-                               page_decref(page);
-                               page = pm_find_page(pm, index);
-                               break;
-                       default:
-                               /* something is wrong, bail out! */
-                               page_decref(page);
-                               return error;
-               }
-       }
-       /* At this point, page is a refcnt'd page, and we return the reference.
-        * Also, there's an unlikely race where we're not in the page cache anymore,
-        * and this all is useless work. */
-       *pp = page;
-       /* if the page was in the map, we need to do some checks, and might have to
-        * read in the page later.  If the page was freshly inserted to the pm by
-        * us, we skip this since we are the one doing the readpage(). */
-       if (page_was_mapped) {
-               /* is it already here and up to date?  if so, we're done */
-               if (page->pg_flags & PG_UPTODATE)
-                       return 0;
-               /* if not, try to lock the page (could BLOCK) */
-               lock_page(page);
-               /* we got it, is our page still in the cache?  check the mapping.  if
-                * not, start over, perhaps with EAGAIN and outside support */
-               if (!page->pg_mapping)
-                       panic("Page is not in the mapping!  Haven't implemented this!");
-               /* double check, are we up to date?  if so, we're done */
-               if (page->pg_flags & PG_UPTODATE) {
-                       unlock_page(page);
-                       return 0;
-               }
-       }
-       /* if we're here, the page is locked by us, and it needs to be read in */
-       assert(page->pg_mapping == pm);
-       error = pm->pm_op->readpage(pm, page);
-       assert(!error);
-       /* Try to sleep on the IO.  The page will be unlocked when the IO is done */
-       lock_page(page);
-       unlock_page(page);
-       assert(page->pg_flags & PG_UPTODATE);
-       return 0;
-}
-
 /* Process-related File management functions */
 
 /* Given any FD, get the appropriate file, 0 o/w */