sys_unlink()
authorBarret Rhoden <brho@cs.berkeley.edu>
Thu, 19 Aug 2010 19:47:35 +0000 (12:47 -0700)
committerKevin Klues <klueska@cs.berkeley.edu>
Thu, 3 Nov 2011 00:35:52 +0000 (17:35 -0700)
Note that this isn't fully testable yet, since KFS pins all dentries
forever, which also means our {dentry,inode}_release paths are mostly
unused.  Eagle-eyed readers should know when those functions are called.

Documentation/vfs.txt
kern/include/vfs.h
kern/src/kfs.c
kern/src/syscall.c
kern/src/vfs.c
tests/file_test.c

index b2b20be..108b17a 100644 (file)
@@ -267,3 +267,15 @@ like getting rid of follow_link and put_link from the FS specific sections.  Our
 FSs just need to know how to return a char* for a symname - and not do any of
 the actual link following.  Or any of the other stuff they do.  We'll see if
 that turns out to be an issue or not...
+
+Unlinking
+-------------------------
+Unlinking is just disconnecting a dentry-inode pair from the directory tree, and
+decreasing the inode's i_nlink.  Nothing else happens yet, since we need to keep
+the FS-file (controlled by the dentry/inode) so long as any OS-files have it
+open.  They have it opened via open or mmap - any way that there is a reference
+to a file, which then pins the dentry and inode.  When the OS-files close,
+eventually the dentry's refcnt hits 0.  When it does, it normally would be up
+for caching, but we can check nlinks and just drop it.  When that happens, it
+releases the inode, which will see its nlinks is 0.  That will trigger the
+underlying FS to clear out the FS-file.
index b088bab..bbd78f6 100644 (file)
@@ -215,7 +215,7 @@ struct super_block {
 
 struct super_operations {
        struct inode *(*alloc_inode) (struct super_block *sb);
-       void (*destroy_inode) (struct inode *);         /* dealloc.  might need more */
+       void (*dealloc_inode) (struct inode *);
        void (*read_inode) (struct inode *);
        void (*dirty_inode) (struct inode *);
        void (*write_inode) (struct inode *, bool);
@@ -505,6 +505,7 @@ ssize_t generic_dir_read(struct file *file, char *u_buf, size_t count,
 struct file *do_file_open(char *path, int flags, int mode);
 int do_symlink(char *path, const char *symname, int mode);
 int do_link(char *old_path, char *new_path);
+int do_unlink(char *path);
 int do_file_access(char *path, int mode);
 struct file *dentry_open(struct dentry *dentry, int flags);
 void file_release(struct kref *kref);
index 53babd8..cc16db5 100644 (file)
@@ -164,8 +164,11 @@ struct inode *kfs_alloc_inode(struct super_block *sb)
 }
 
 /* deallocs and cleans up after an inode. */
-void kfs_destroy_inode(struct inode *inode)
+void kfs_dealloc_inode(struct inode *inode)
 {
+       /* If we're a symlink, give up our storage for the symname */
+       if (inode->i_type == FS_I_SYMLINK)
+               kfree(((struct kfs_i_info*)inode->i_fs_info)->filestart);
        kmem_cache_free(kfs_i_kcache, inode->i_fs_info);
 }
 
@@ -231,7 +234,8 @@ void kfs_drop_inode(struct inode *inode)
 void kfs_delete_inode(struct inode *inode)
 {
        // would remove from "disk" here
-       kfs_destroy_inode(inode);
+       kfs_dealloc_inode(inode);
+       /* TODO: give up our i_ino */
 }
 
 /* unmount and release the super block */
@@ -361,7 +365,10 @@ int kfs_link(struct dentry *old_dentry, struct inode *dir,
 /* Removes the link from the dentry in the directory */
 int kfs_unlink(struct inode *dir, struct dentry *dentry)
 {
-       return -1;
+       /* Stop tracking our child */
+       TAILQ_REMOVE(&((struct kfs_i_info*)dir->i_fs_info)->children, dentry,
+                    d_subdirs_link);
+       return 0;
 }
 
 /* Creates a new inode for a symlink dir, linking to / containing the name
@@ -644,7 +651,7 @@ struct page_map_operations kfs_pm_op = {
 
 struct super_operations kfs_s_op = {
        kfs_alloc_inode,
-       kfs_destroy_inode,
+       kfs_dealloc_inode,
        kfs_read_inode,
        kfs_dirty_inode,
        kfs_write_inode,
index d3999bb..5c74c1b 100644 (file)
@@ -976,7 +976,6 @@ static intreg_t sys_access(struct proc *p, const char *path, size_t path_l,
                            int mode)
 {
        int retval;
-
        char *t_path = user_strdup_errno(p, path, path_l);
        if (!t_path)
                return -1;
@@ -1038,12 +1037,13 @@ intreg_t sys_link(struct proc *p, char *old_path, size_t old_l,
 
 intreg_t sys_unlink(struct proc *p, const char *path, size_t path_l)
 {
-       char* fn = user_strdup_errno(p,path,PGSIZE);
-       if(fn == NULL)
+       int retval;
+       char *t_path = user_strdup_errno(p, path, path_l);
+       if (!t_path)
                return -1;
-       int ret = ufe(unlink,PADDR(fn),0,0,0);
-       user_memdup_free(p,fn);
-       return ret;
+       retval = do_unlink(t_path);
+       user_memdup_free(p, t_path);
+       return retval;
 }
 
 intreg_t sys_symlink(struct proc *p, char *old_path, size_t old_l,
index 33e8396..d3334bc 100644 (file)
@@ -573,7 +573,7 @@ void dcache_put(struct dentry *dentry)
 
 /* Cleans up the dentry (after ref == 0).  We still may want it, and this is
  * where we should add it to the dentry cache.  (TODO).  For now, we do nothing,
- * since we don't have a dcache.
+ * since we don't have a dcache.  Also, if i_nlink == 0, never cache it.
  * 
  * 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 check for d_inode.
@@ -756,7 +756,12 @@ int check_perms(struct inode *inode, int access_mode)
 void inode_release(struct kref *kref)
 {
        struct inode *inode = container_of(kref, struct inode, i_kref);
-       inode->i_sb->s_op->destroy_inode(inode);
+       /* If we still have links, just dealloc the in-memory inode.  if we have no
+        * links, we need to delete it too (which calls destroy). */
+       if (inode->i_nlink)
+               inode->i_sb->s_op->dealloc_inode(inode);
+       else
+               inode->i_sb->s_op->delete_inode(inode);
        kref_put(&inode->i_sb->s_kref);
        assert(inode->i_mapping == &inode->i_pm);
        kmem_cache_free(inode_kcache, inode);
@@ -1112,6 +1117,54 @@ out_path_only:
        return retval;
 }
 
+int do_unlink(char *path)
+{
+       struct dentry *dentry;
+       struct inode *parent_dir;
+       struct nameidata nd_r = {0}, *nd = &nd_r;
+       int error;
+       int retval = -1;
+
+       /* get the parent of the target, and don't follow a final link */
+       error = path_lookup(path, LOOKUP_PARENT, nd);
+       if (error) {
+               set_errno(-error);
+               goto out_path_only;
+       }
+       parent_dir = nd->dentry->d_inode;
+       /* make sure the target is there */
+       dentry = do_lookup(nd->dentry, nd->last.name); 
+       if (!dentry) {
+               set_errno(ENOENT);
+               goto out_path_only;
+       }
+       /* Make sure the target is not a directory */
+       if (dentry->d_inode->i_type == FS_I_DIR) {
+               set_errno(EISDIR);
+               goto out_dentry;
+       }
+       /* Remove the dentry from its parent */
+       error = parent_dir->i_op->unlink(parent_dir, dentry);
+       if (error) {
+               set_errno(-error);
+               goto out_dentry;
+       }
+       kref_put(&dentry->d_parent->d_kref);
+       dentry->d_parent = 0;           /* so we don't double-decref it later */
+       dentry->d_inode->i_nlink--;     /* TODO: race here */
+       /* At this point, the dentry is unlinked from the FS, and the inode has one
+        * less link.  When the in-memory objects (dentry, inode) are going to be
+        * released (after all open files are closed, and maybe after entries are
+        * evicted from the cache), then nlinks will get checked and the FS-file
+        * will get removed from the disk */
+       retval = 0;                             /* Note the fall through to the exit paths */
+out_dentry:
+       kref_put(&dentry->d_kref);
+out_path_only:
+       path_release(nd);
+       return retval;
+}
+
 /* Checks to see if path can be accessed via mode.  Doesn't do much now.  This
  * is an example of decent error propagation from the lower levels via int
  * retvals. */
index d05494a..5ddd81c 100644 (file)
@@ -99,6 +99,22 @@ int main()
        retval = link("/bin/hello", "/dir1/hardhello");
        if (retval < 0)
                printf("WARNING! Link failed!\n");
-
+       //breakpoint();
+       printf("Now unlinking /dir1/hardhello\n");
+       retval = unlink("/dir1/hardhello");
+       if (retval < 0)
+               printf("WARNING! Unlink failed!\n");
+       printf("Linking to /bin/hello at /bin/hardhello2\n");
+       retval = link("/bin/hello", "/bin/hardhello2");
+       if (retval < 0)
+               printf("WARNING! Link failed!\n");
+       printf("Now unlinking symlink /dir1/test.txt\n");
+       retval = unlink("/dir1/test.txt");
+       if (retval < 0)
+               printf("WARNING! Unlink failed!\n");
+       printf("Now unlinking /dir2/test2.txt\n");
+       retval = unlink("/dir2/test2.txt");
+       if (retval < 0)
+               printf("WARNING! Unlink failed!\n");
        breakpoint();
 }