Revisited dentry and inode creation
authorBarret Rhoden <brho@cs.berkeley.edu>
Fri, 6 Aug 2010 01:39:50 +0000 (18:39 -0700)
committerKevin Klues <klueska@cs.berkeley.edu>
Thu, 3 Nov 2011 00:35:50 +0000 (17:35 -0700)
Pulled out VFS stuff from the alloc_inode, and reworked how it worked
with get_dentry().  get_dentry() will now have a d_ops set for most
dentries (except for the parentless, which needs to be handled by the
mounting).

kern/include/vfs.h
kern/src/kfs.c
kern/src/vfs.c

index 077ac74..d89623f 100644 (file)
@@ -461,6 +461,7 @@ void dcache_put(struct dentry *dentry);
 void dentry_release(struct kref *kref);
 
 /* Inode Functions */
+struct inode *get_inode(struct dentry *dentry);
 int check_perms(struct inode *inode, int access_mode);
 void inode_release(struct kref *kref);
 
index f86a148..7ead9d5 100644 (file)
@@ -149,40 +149,21 @@ int kfs_readpage(struct file *file, struct page *page)
  * an inode that is already on disk, or it can be used when creating. */
 struct inode *kfs_alloc_inode(struct super_block *sb)
 {
-       /* arguably, we can avoid some of this init by using the slab/cache */
        struct inode *inode = kmem_cache_alloc(inode_kcache, 0);
        memset(inode, 0, sizeof(struct inode));
-       TAILQ_INSERT_HEAD(&sb->s_inodes, inode, i_sb_list);
-       TAILQ_INIT(&inode->i_dentry);
-       inode->i_ino = 0;                                       /* set by caller later */
-       kref_init(&inode->i_kref, inode_release, 1);
-       inode->i_blksize = 1;                           /* keep in sync with get_sb() */
-       spinlock_init(&inode->i_lock);
        inode->i_op = &kfs_i_op;
        inode->i_fop = &kfs_f_op;
-       inode->i_sb = sb;
-       inode->i_state = 0;                                     /* need real states, want I_NEW */
-       inode->dirtied_when = 0;
-       atomic_set(&inode->i_writecount, 0);
+       inode->i_pm.pm_op = &kfs_pm_op;
        inode->i_fs_info = kmem_cache_alloc(kfs_i_kcache, 0);
        TAILQ_INIT(&((struct kfs_i_info*)inode->i_fs_info)->children);
        ((struct kfs_i_info*)inode->i_fs_info)->filestart = 0;
-       /* Set up the page_map structures.  Default is to use the embedded one. */
-       inode->i_mapping = &inode->i_pm;
-       inode->i_mapping->pm_host = inode;
-       radix_tree_init(&inode->i_mapping->pm_tree);
-       spinlock_init(&inode->i_mapping->pm_tree_lock);
-       inode->i_mapping->pm_op = &kfs_pm_op;
-       inode->i_mapping->pm_flags = 0;
        return inode;
-       /* caller sets i_ino, i_list set when applicable */
 }
 
 /* deallocs and cleans up after an inode. */
 void kfs_destroy_inode(struct inode *inode)
 {
        kmem_cache_free(kfs_i_kcache, inode->i_fs_info);
-       kmem_cache_free(inode_kcache, inode);
 }
 
 /* reads the inode data on disk specified by inode->i_ino into the inode.
@@ -293,13 +274,10 @@ static struct inode *kfs_create_generic(struct dentry *dentry,
        /* note it is the i_ino that uniquely identifies a file in the system.
         * there's a diff between creating an inode (even for an in-use ino) and
         * then filling it in, and vs creating a brand new one */
-       struct inode *inode = kfs_alloc_inode(dentry->d_sb);
+       struct inode *inode = get_inode(dentry);
+       if (!inode)
+               return 0;
        kref_get(&dentry->d_kref, 1);   /* to pin the dentry in RAM, KFS-style... */
-       dentry->d_inode = inode;                /* inode ref from alloc() stored here */
-       TAILQ_INSERT_TAIL(&inode->i_dentry, dentry, d_alias); /* weak dentry ref*/
-       /* Need to finish the dentry */
-       dentry->d_op = &kfs_d_op;
-       dentry->d_fs_info = 0;
        inode->i_mode = mode;
        inode->i_ino = kfs_get_free_ino();
        inode->i_nlink = 1;
@@ -334,6 +312,7 @@ int kfs_create(struct inode *dir, struct dentry *dentry, int mode,
                          dentry, d_subdirs_link);
        /* fs_info->filestart is set by the caller, or else when first written (for
         * new files.  it was set to 0 in alloc_inode(). */
+       kref_put(&inode->i_kref);
        return 0;
 }
 
@@ -421,9 +400,11 @@ int kfs_mkdir(struct inode *dir, struct dentry *dentry, int mode)
        inode->i_type = FS_I_DIR;
        /* parent dentry tracks dentry as a subdir, weak reference */
        TAILQ_INSERT_TAIL(&parent->d_subdirs, dentry, d_subdirs_link);
+
        /* get ready to have our own kids */
        TAILQ_INIT(&((struct kfs_i_info*)inode->i_fs_info)->children);
        ((struct kfs_i_info*)inode->i_fs_info)->filestart = 0;
+       kref_put(&inode->i_kref);
        return 0;
 }
 
index 33216bd..c0141b4 100644 (file)
@@ -356,10 +356,13 @@ void init_sb(struct super_block *sb, struct vfsmount *vmnt,
         * by vfsmount's mnt_root.  The parent is dealt with later. */
        struct dentry *d_root = get_dentry(sb, 0,  "/");        /* probably right */
 
-       /* a lot of here on down is normally done in lookup() */
+       /* a lot of here on down is normally done in lookup() or create, since
+        * get_dentry isn't a fully usable dentry.  The two FS-specific settings are
+        * normally inherited from a parent within the same FS in get_dentry, but we
+        * have none here. */
        d_root->d_op = d_op;
        d_root->d_fs_info = d_fs_info;
-       struct inode *inode = sb->s_op->alloc_inode(sb);
+       struct inode *inode = get_inode(d_root);
        if (!inode)
                panic("This FS sucks!");
        d_root->d_inode = inode;                                /* storing the inode's kref here */
@@ -387,9 +390,9 @@ void init_sb(struct super_block *sb, struct vfsmount *vmnt,
 /* Dentry Functions */
 
 /* Helper to alloc and initialize a generic dentry.  The following needs to be
- * set still: d_op, d_fs_info (opt), d_inode, connect the inode to the dentry
- * (and up the d_kref again), maybe dcache_put().  The FS related things need to
- * be done in fs_create and fs_mkdir.
+ * set still: d_op (if no parent), d_fs_info (opt), d_inode, connect the inode
+ * to the dentry (and up the d_kref again), maybe dcache_put().  The inode
+ * stitching is done in get_inode() or lookup (depending on the FS).
  *
  * If the name is longer than the inline name, it will kmalloc a buffer, so
  * don't worry about the storage for *name after calling this. */
@@ -410,8 +413,10 @@ struct dentry *get_dentry(struct super_block *sb, struct dentry *parent,
        dentry->d_sb = sb;                                      /* storing a ref here... */
        dentry->d_mount_point = FALSE;
        dentry->d_mounted_fs = 0;
-       if (parent)                                                     /* no parent for rootfs mount */
+       if (parent)     {                                               /* no parent for rootfs mount */
                kref_get(&parent->d_kref, 1);
+               dentry->d_op = parent->d_op;    /* d_op set in init_sb for parentless */
+       }
        dentry->d_parent = parent;
        dentry->d_flags = 0;                            /* related to its dcache state */
        dentry->d_fs_info = 0;
@@ -428,8 +433,6 @@ struct dentry *get_dentry(struct super_block *sb, struct dentry *parent,
                qstr_builder(dentry, l_name);
        }
        /* Catch bugs by aggressively zeroing this (o/w we use old stuff) */
-       dentry->d_op = 0;
-       dentry->d_fs_info = 0;
        dentry->d_inode = 0;
        return dentry;
 }
@@ -450,8 +453,7 @@ void dcache_put(struct dentry *dentry)
  * since we don't have a dcache.
  * 
  * This has to handle two types of dentries: full ones (ones that had been used)
- * and ones that had been just for lookups - hence the checks for d_op and
- * d_inode.
+ * and ones that had been just for lookups - hence the check for d_inode.
  *
  * Note that dentries pin and kref their inodes.  When all the dentries are
  * gone, we want the inode to be released via kref.  The inode has internal /
@@ -460,9 +462,8 @@ void dentry_release(struct kref *kref)
 {
        struct dentry *dentry = container_of(kref, struct dentry, d_kref);
        printd("Freeing dentry %08p: %s\n", dentry, dentry->d_name.name);
-       /* Might not have a d_op, if it was involved in a failed operation */
-       if (dentry->d_op && dentry->d_op->d_release)
-               dentry->d_op->d_release(dentry);
+       assert(dentry->d_op);   /* catch bugs.  a while back, some lacked d_op */
+       dentry->d_op->d_release(dentry);
        /* TODO: check/test the boundaries on this. */
        if (dentry->d_name.len > DNAME_INLINE_LEN)
                kfree((void*)dentry->d_name.name);
@@ -478,6 +479,46 @@ void dentry_release(struct kref *kref)
 
 /* Inode Functions */
 
+/* Creates and initializes a new inode.  Generic fields are filled in.
+ * FS-specific fields are filled in by the callout.  Specific fields are filled
+ * in in read_inode() based on what's on the disk for a given i_no, or when the
+ * inode is created (for new objects).
+ *
+ * i_no is set by the caller.  Note that this means this inode can be for an
+ * inode that is already on disk, or it can be used when creating. */
+struct inode *get_inode(struct dentry *dentry)
+{
+       struct super_block *sb = dentry->d_sb;
+       /* FS allocs and sets the following: i_op, i_fop, i_pm.pm_op, and any FS
+        * specific stuff. */
+       struct inode *inode = sb->s_op->alloc_inode(sb);
+       if (!inode)
+               return 0;
+       TAILQ_INSERT_HEAD(&sb->s_inodes, inode, i_sb_list);             /* weak inode ref */
+       TAILQ_INIT(&inode->i_dentry);
+       TAILQ_INSERT_TAIL(&inode->i_dentry, dentry, d_alias);   /* weak dentry ref*/
+       /* one for the dentry->d_inode, one passed out */
+       kref_init(&inode->i_kref, inode_release, 2);
+       dentry->d_inode = inode;
+       inode->i_ino = 0;                                       /* set by caller later */
+       inode->i_blksize = sb->s_blocksize;
+       spinlock_init(&inode->i_lock);
+       inode->i_sb = sb;
+       inode->i_state = 0;                                     /* need real states, like I_NEW */
+       inode->dirtied_when = 0;
+       atomic_set(&inode->i_writecount, 0);
+       /* Set up the page_map structures.  Default is to use the embedded one.
+        * Might push some of this back into specific FSs.  For now, the FS tells us
+        * what pm_op they want via i_pm.pm_op, which we use when we point i_mapping
+        * to i_pm. */
+       inode->i_mapping = &inode->i_pm;
+       inode->i_mapping->pm_host = inode;
+       radix_tree_init(&inode->i_mapping->pm_tree);
+       spinlock_init(&inode->i_mapping->pm_tree_lock);
+       inode->i_mapping->pm_flags = 0;
+       return inode;
+}
+
 /* Returns 0 if the given mode is acceptable for the inode, and an appropriate
  * error code if not.  Needs to be writen, based on some sensible rules, and
  * will also probably use 'current' */
@@ -495,6 +536,7 @@ void inode_release(struct kref *kref)
        inode->i_sb->s_op->destroy_inode(inode);
        kref_put(&inode->i_sb->s_kref);
        assert(inode->i_mapping == &inode->i_pm);
+       kmem_cache_free(inode_kcache, inode);
        /* TODO: (BDEV) */
        // kref_put(inode->i_bdev->kref); /* assuming it's a bdev */
 }