rmdir() can't remove mountpoints
[akaros.git] / kern / src / vfs.c
index d1e7a01..26ee24d 100644 (file)
@@ -12,6 +12,7 @@
 #include <slab.h>
 #include <kmalloc.h>
 #include <kfs.h>
+#include <ext2fs.h>
 #include <pmap.h>
 #include <umem.h>
 #include <smp.h>
@@ -20,9 +21,6 @@ struct sb_tailq super_blocks = TAILQ_HEAD_INITIALIZER(super_blocks);
 spinlock_t super_blocks_lock = SPINLOCK_INITIALIZER;
 struct fs_type_tailq file_systems = TAILQ_HEAD_INITIALIZER(file_systems);
 struct namespace default_ns;
-// TODO: temp dcache, holds all dentries ever for now
-struct dentry_slist dcache = SLIST_HEAD_INITIALIZER(dcache);
-spinlock_t dcache_lock = SPINLOCK_INITIALIZER;
 
 struct kmem_cache *dentry_kcache; // not to be confused with the dcache
 struct kmem_cache *inode_kcache;
@@ -33,9 +31,9 @@ struct kmem_cache *file_kcache;
  * with multiple namespaces on the same FS yet.  Note if you mount the same FS
  * multiple times, you only have one FS still (and one SB).  If we ever support
  * that... */
-struct vfsmount *mount_fs(struct fs_type *fs, char *dev_name,
-                          struct dentry *mnt_pt, int flags,
-                          struct namespace *ns)
+struct vfsmount *__mount_fs(struct fs_type *fs, char *dev_name,
+                            struct dentry *mnt_pt, int flags,
+                            struct namespace *ns)
 {
        struct super_block *sb;
        struct vfsmount *vmnt = kmalloc(sizeof(struct vfsmount), 0);
@@ -101,12 +99,13 @@ void vfs_init(void)
        /* build list of all FS's in the system.  put yours here.  if this is ever
         * done on the fly, we'll need to lock. */
        TAILQ_INSERT_TAIL(&file_systems, &kfs_fs_type, list);
+       TAILQ_INSERT_TAIL(&file_systems, &ext2_fs_type, list);
        TAILQ_FOREACH(fs, &file_systems, list)
                printk("Supports the %s Filesystem\n", fs->name);
 
        /* mounting KFS at the root (/), pending root= parameters */
        // TODO: linux creates a temp root_fs, then mounts the real root onto that
-       default_ns.root = mount_fs(&kfs_fs_type, "RAM", NULL, 0, &default_ns);
+       default_ns.root = __mount_fs(&kfs_fs_type, "RAM", NULL, 0, &default_ns);
 
        printk("vfs_init() completed\n");
 }
@@ -129,22 +128,46 @@ char *file_name(struct file *file)
        return file->f_dentry->d_name.name;
 }
 
-/* Some issues with this, coupled closely to fs_lookup.  This assumes that
- * negative dentries are not returned (might differ from linux) */
+/* Some issues with this, coupled closely to fs_lookup.
+ *
+ * Note the use of __dentry_free, instead of kref_put.  In those cases, we don't
+ * want to treat it like a kref and we have the only reference to it, so it is
+ * okay to do this.  It makes dentry_release() easier too. */
 static struct dentry *do_lookup(struct dentry *parent, char *name)
 {
-       struct dentry *dentry;
-       /* TODO: look up in the dentry cache first */
-       dentry = get_dentry(parent->d_sb, parent, name);
-       dentry = parent->d_inode->i_op->lookup(parent->d_inode, dentry, 0);
-       /* insert in dentry cache */
+       struct dentry *result, *query;
+       query = get_dentry(parent->d_sb, parent, name);
+       result = dcache_get(parent->d_sb, query); 
+       if (result) {
+               __dentry_free(query);
+               return result;
+       }
+       /* No result, check for negative */
+       if (query->d_flags & DENTRY_NEGATIVE) {
+               __dentry_free(query);
+               return 0;
+       }
+       /* not in the dcache at all, need to consult the FS */
+       result = parent->d_inode->i_op->lookup(parent->d_inode, query, 0);
+       if (!result) {
+               query->d_flags |= DENTRY_NEGATIVE;
+               dcache_put(parent->d_sb, query);
+               kref_put(&query->d_kref);
+               return 0;
+       }
+       dcache_put(parent->d_sb, result);
+       /* This is because KFS doesn't return the same dentry, but ext2 does.  this
+        * is ugly and needs to be fixed. (TODO) */
+       if (result != query)
+               __dentry_free(query);
+
        /* TODO: if the following are done by us, how do we know the i_ino?
         * also need to handle inodes that are already read in!  For now, we're
         * going to have the FS handle it in it's lookup() method: 
         * - get a new inode
         * - read in the inode
         * - put in the inode cache */
-       return dentry;
+       return result;
 }
 
 /* Update ND such that it represents having followed dentry.  IAW the nd
@@ -173,7 +196,7 @@ static int climb_up(struct nameidata *nd)
        printd("CLIMB_UP, from %s\n", nd->dentry->d_name.name);
        /* Top of the world, just return.  Should also check for being at the top of
         * the current process's namespace (TODO) */
-       if (!nd->dentry->d_parent)
+       if (!nd->dentry->d_parent || (nd->dentry->d_parent == nd->dentry))
                return -1;
        /* Check if we are at the top of a mount, if so, we need to follow
         * backwards, and then climb_up from that one.  We might need to climb
@@ -192,9 +215,13 @@ static int climb_up(struct nameidata *nd)
        return 0;
 }
 
+/* nd->dentry might be on a mount point, so we need to move on to the child
+ * mount's root. */
 static int follow_mount(struct nameidata *nd)
 {
-       /* Detect mount, follow, etc... (TODO!) */
+       if (!nd->dentry->d_mount_point)
+               return 0;
+       next_link(nd->dentry->d_mounted_fs->mnt_root, nd);
        return 0;
 }
 
@@ -464,8 +491,43 @@ void path_release(struct nameidata *nd)
        }
 }
 
+/* External version of mount, only call this after having a / mount */
+int mount_fs(struct fs_type *fs, char *dev_name, char *path, int flags)
+{
+       struct nameidata nd_r = {0}, *nd = &nd_r;
+       int retval = 0;
+       retval = path_lookup(path, LOOKUP_DIRECTORY, nd);
+       if (retval)
+               goto out;
+       /* taking the namespace of the vfsmount of path */ 
+       if (!__mount_fs(fs, dev_name, nd->dentry, flags, nd->mnt->mnt_namespace))
+               retval = -EINVAL;
+out:
+       path_release(nd);
+       return retval;
+}
+
 /* Superblock functions */
 
+/* Dentry "hash" function for the hash table to use.  Since we already have the
+ * hash in the qstr, we don't need to rehash.  Also, note we'll be using the
+ * dentry in question as both the key and the value. */
+static size_t __dcache_hash(void *k)
+{
+       return (size_t)((struct dentry*)k)->d_name.hash;
+}
+
+/* Dentry cache hashtable equality function.  This means we need to pass in some
+ * minimal dentry when doing a lookup. */
+static ssize_t __dcache_eq(void *k1, void *k2)
+{
+       if (((struct dentry*)k1)->d_parent != ((struct dentry*)k2)->d_parent)
+               return 0;
+       /* TODO: use the FS-specific string comparison */
+       return !strcmp(((struct dentry*)k1)->d_name.name,
+                      ((struct dentry*)k2)->d_name.name);
+}
+
 /* Helper to alloc and initialize a generic superblock.  This handles all the
  * VFS related things, like lists.  Each FS will need to handle its own things
  * in it's *_get_sb(), usually involving reading off the disc. */
@@ -478,8 +540,13 @@ struct super_block *get_sb(void)
        TAILQ_INIT(&sb->s_inodes);
        TAILQ_INIT(&sb->s_dirty_i);
        TAILQ_INIT(&sb->s_io_wb);
-       SLIST_INIT(&sb->s_anon_d);
+       TAILQ_INIT(&sb->s_lru_d);
        TAILQ_INIT(&sb->s_files);
+       sb->s_dcache = create_hashtable(100, __dcache_hash, __dcache_eq);
+       sb->s_icache = create_hashtable(100, __generic_hash, __generic_eq);
+       spinlock_init(&sb->s_lru_lock);
+       spinlock_init(&sb->s_dcache_lock);
+       spinlock_init(&sb->s_icache_lock);
        sb->s_fs_info = 0; // can override somewhere else
        return sb;
 }
@@ -513,6 +580,7 @@ void init_sb(struct super_block *sb, struct vfsmount *vmnt,
        /* TODO: do we need to read in the inode?  can we do this on demand? */
        /* if this FS is already mounted, we'll need to do something different. */
        sb->s_op->read_inode(inode);
+       icache_put(sb, inode);
        /* Link the dentry and SB to the VFS mount */
        vmnt->mnt_root = d_root;                                /* ref comes from get_dentry */
        vmnt->mnt_sb = sb;
@@ -521,11 +589,13 @@ void init_sb(struct super_block *sb, struct vfsmount *vmnt,
        if (vmnt->mnt_mountpoint) {
                kref_get(&vmnt->mnt_mountpoint->d_kref, 1);     /* held by d_root */
                d_root->d_parent = vmnt->mnt_mountpoint;        /* dentry of the root */
+       } else {
+               d_root->d_parent = d_root;                      /* set root as its own parent */
        }
        /* insert the dentry into the dentry cache.  when's the earliest we can?
         * when's the earliest we should?  what about concurrent accesses to the
         * same dentry?  should be locking the dentry... */
-       dcache_put(d_root); // TODO: should set a d_flag too
+       dcache_put(sb, d_root);
        kref_put(&inode->i_kref);               /* give up the ref from get_inode() */
 }
 
@@ -564,9 +634,8 @@ struct dentry *get_dentry(struct super_block *sb, struct dentry *parent,
                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_flags = DENTRY_USED;
        dentry->d_fs_info = 0;
-       SLIST_INIT(&dentry->d_bucket);
        if (name_len < DNAME_INLINE_LEN) {
                strncpy(dentry->d_iname, name, name_len);
                dentry->d_iname[name_len] = '\0';
@@ -583,31 +652,56 @@ struct dentry *get_dentry(struct super_block *sb, struct dentry *parent,
        return dentry;
 }
 
-/* Adds a dentry to the dcache. */
-void dcache_put(struct dentry *dentry)
+/* Called when the dentry is unreferenced (after kref == 0).  This works closely
+ * with the resurrection in dcache_get().
+ *
+ * The dentry is still in the dcache, but needs to be un-USED and added to the
+ * LRU dentry list.  Even dentries that were used in a failed lookup need to be
+ * cached - they ought to be the negative dentries.  Note that all dentries have
+ * parents, even negative ones (it is needed to find it in the dcache). */
+void dentry_release(struct kref *kref)
 {
-#if 0 /* pending a more thorough review of the dcache */
-       /* TODO: should set a d_flag too */
-       spin_lock(&dcache_lock);
-       SLIST_INSERT_HEAD(&dcache, dentry, d_hash);
-       spin_unlock(&dcache_lock);
-#endif
+       struct dentry *dentry = container_of(kref, struct dentry, d_kref);
+
+       printd("'Releasing' dentry %08p: %s\n", dentry, dentry->d_name.name);
+       /* DYING dentries (recently unlinked / rmdir'd) just get freed */
+       if (dentry->d_flags & DENTRY_DYING) {
+               __dentry_free(dentry);
+               return;
+       }
+       /* This lock ensures the USED state and the TAILQ membership is in sync.
+        * Also used to check the refcnt, though that might not be necessary. */
+       spin_lock(&dentry->d_lock);
+       /* While locked, we need to double check the kref, in case someone already
+        * reup'd it.  Re-up? you're crazy!  Reee-up, you're outta yo mind! */
+       if (!kref_refcnt(&dentry->d_kref)) {
+               /* and make sure it wasn't USED, then UNUSED again */
+               /* TODO: think about issues with this */
+               if (dentry->d_flags & DENTRY_USED) {
+                       dentry->d_flags &= ~DENTRY_USED;
+                       spin_lock(&dentry->d_sb->s_lru_lock);
+                       TAILQ_INSERT_TAIL(&dentry->d_sb->s_lru_d, dentry, d_lru);
+                       spin_unlock(&dentry->d_sb->s_lru_lock);
+               } else {
+                       warn("This should be rare.  Tell brho this happened.");
+               }
+       }
+       spin_unlock(&dentry->d_lock);
 }
 
-/* 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.  Also, if i_nlink == 0, never cache it.
- * 
+/* Called when we really dealloc and get rid of a dentry (like when it is
+ * removed from the dcache, either for memory or correctness reasons)
+ *
  * 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.
  *
  * 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 /
  * weak references to the dentry, which are not refcounted. */
-void dentry_release(struct kref *kref)
+void __dentry_free(struct dentry *dentry)
 {
-       struct dentry *dentry = container_of(kref, struct dentry, d_kref);
-       printd("Freeing dentry %08p: %s\n", dentry, dentry->d_name.name);
+       if (dentry->d_inode)
+               printk("Freeing dentry %08p: %s\n", dentry, dentry->d_name.name);
        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. */
@@ -646,6 +740,93 @@ struct dentry *lookup_dentry(char *path, int flags)
        return dentry;
 }
 
+/* Get a dentry from the dcache.  At a minimum, we need the name hash and parent
+ * in what_i_want, though most uses will probably be from a get_dentry() call.
+ * We pass in the SB in the off chance that we don't want to use a get'd dentry.
+ *
+ * The unusual variable name (instead of just "key" or something) is named after
+ * ex-SPC Castro's porn folder.  Caller deals with the memory for what_i_want.
+ *
+ * If the dentry is negative, we don't return the actual result - instead, we
+ * set the negative flag in 'what i want'.  The reason is we don't want to
+ * kref_get() and then immediately put (causing dentry_release()).  This also
+ * means that dentry_release() should never get someone who wasn't USED (barring
+ * the race, which it handles).  And we don't need to ever have a dentry set as
+ * USED and NEGATIVE (which is always wrong, but would be needed for a cleaner
+ * dentry_release()).
+ *
+ * This is where we do the "kref resurrection" - we are returning a kref'd
+ * object, even if it wasn't kref'd before.  This means the dcache does NOT hold
+ * krefs (it is a weak/internal ref), but it is a source of kref generation.  We
+ * sync up with the possible freeing of the dentry by locking the table.  See
+ * Doc/kref for more info. */
+struct dentry *dcache_get(struct super_block *sb, struct dentry *what_i_want)
+{
+       struct dentry *found;
+       /* This lock protects the hash, as well as ensures the returned object
+        * doesn't get deleted/freed out from under us */
+       spin_lock(&sb->s_dcache_lock);
+       found = hashtable_search(sb->s_dcache, what_i_want);
+       if (found) {
+               if (found->d_flags & DENTRY_NEGATIVE) {
+                       what_i_want->d_flags |= DENTRY_NEGATIVE;
+                       spin_unlock(&sb->s_dcache_lock);
+                       return 0;
+               }
+               spin_lock(&found->d_lock);
+               __kref_get(&found->d_kref, 1);  /* prob could be done outside the lock*/
+               /* If we're here (after kreffing) and it is not USED, we are the one who
+                * should resurrect */
+               if (!(found->d_flags & DENTRY_USED)) {
+                       found->d_flags |= DENTRY_USED;
+                       spin_lock(&sb->s_lru_lock);
+                       TAILQ_REMOVE(&sb->s_lru_d, found, d_lru);
+                       spin_unlock(&sb->s_lru_lock);
+               }
+               spin_unlock(&found->d_lock);
+       }
+       spin_unlock(&sb->s_dcache_lock);
+       return found;
+}
+
+/* Adds a dentry to the dcache.  Note the *dentry is both the key and the value.
+ * If the value was already in there (which can happen iff it was negative), for
+ * now we'll remove it and put the new one in there. */
+void dcache_put(struct super_block *sb, struct dentry *key_val)
+{
+       struct dentry *old;
+       int retval;
+       spin_lock(&sb->s_dcache_lock);
+       old = hashtable_remove(sb->s_dcache, key_val);
+       if (old) {
+               assert(old->d_flags & DENTRY_NEGATIVE);
+               assert(!(old->d_flags & DENTRY_USED));
+               assert(!kref_refcnt(&old->d_kref));
+               spin_lock(&sb->s_lru_lock);
+               TAILQ_REMOVE(&sb->s_lru_d, old, d_lru);
+               spin_unlock(&sb->s_lru_lock);
+               __dentry_free(old);
+       }
+       /* this returns 0 on failure (TODO: Fix this ghetto shit) */
+       retval = hashtable_insert(sb->s_dcache, key_val, key_val);
+       assert(retval);
+       spin_unlock(&sb->s_dcache_lock);
+}
+
+/* Will remove and return the dentry.  Caller deallocs the key, but the retval
+ * won't have a reference.  * Returns 0 if it wasn't found.  Callers can't
+ * assume much - they should not use the reference they *get back*, (if they
+ * already had one for key, they can use that).  There may be other users out
+ * there. */
+struct dentry *dcache_remove(struct super_block *sb, struct dentry *key)
+{
+       struct dentry *retval;
+       spin_lock(&sb->s_dcache_lock);
+       retval = hashtable_remove(sb->s_dcache, key);
+       spin_unlock(&sb->s_dcache_lock);
+       return retval;
+}
+
 /* Inode Functions */
 
 /* Creates and initializes a new inode.  Generic fields are filled in.
@@ -674,7 +855,10 @@ struct inode *get_inode(struct dentry *dentry)
        inode->i_ino = 0;                                       /* set by caller later */
        inode->i_blksize = sb->s_blocksize;
        spinlock_init(&inode->i_lock);
+       kref_get(&sb->s_kref, 1);                       /* could allow the dentry to pin it */
        inode->i_sb = sb;
+       inode->i_rdev = 0;                                      /* this has no real meaning yet */
+       inode->i_bdev = sb->s_bdev;                     /* storing an uncounted ref */
        inode->i_state = 0;                                     /* need real states, like I_NEW */
        inode->dirtied_when = 0;
        inode->i_flags = 0;
@@ -691,6 +875,30 @@ struct inode *get_inode(struct dentry *dentry)
        return inode;
 }
 
+/* Helper: loads/ reads in the inode numbered ino and attaches it to dentry */
+void load_inode(struct dentry *dentry, unsigned int ino)
+{
+       struct inode *inode;
+
+       /* look it up in the inode cache first */
+       inode = icache_get(dentry->d_sb, ino);
+       if (inode) {
+               /* connect the dentry to its inode */
+               TAILQ_INSERT_TAIL(&inode->i_dentry, dentry, d_alias);
+               dentry->d_inode = inode;        /* storing the ref we got from icache_get */
+               return;
+       }
+       /* otherwise, we need to do it manually */
+       inode = get_inode(dentry);
+       inode->i_ino = ino;
+       dentry->d_sb->s_op->read_inode(inode);
+       /* TODO: race here, two creators could miss in the cache, and then get here.
+        * need a way to sync across a blocking call.  needs to be either at this
+        * point in the code or per the ino (dentries could be different) */
+       icache_put(dentry->d_sb, inode);
+       kref_put(&inode->i_kref);
+}
+
 /* Helper op, used when creating regular files, directories, symlinks, etc.
  * Note we make a distinction between the mode and the file type (for now).
  * After calling this, call the FS specific version (create or mkdir), which
@@ -727,16 +935,13 @@ static struct inode *create_inode(struct dentry *dentry, int mode)
 
 /* Create a new disk inode in dir associated with dentry, with the given mode.
  * called when creating a regular file.  dir is the directory/parent.  dentry is
- * the dentry of the inode we are creating.  Note the lack of the nd...
- * Also, we do the nlink++ in here, since we want to give the FS's a chance to
- * fail. */
+ * the dentry of the inode we are creating.  Note the lack of the nd... */
 int create_file(struct inode *dir, struct dentry *dentry, int mode)
 {
        struct inode *new_file = create_inode(dentry, mode);
        if (!new_file)
                return -1;
        dir->i_op->create(dir, dentry, mode, 0);
-       dir->i_nlink++;
        kref_put(&new_file->i_kref);
        return 0;
 }
@@ -749,7 +954,7 @@ int create_dir(struct inode *dir, struct dentry *dentry, int mode)
        if (!new_dir)
                return -1;
        dir->i_op->mkdir(dir, dentry, mode);
-       dir->i_nlink++;
+       dir->i_nlink++;         /* Directories get a hardlink for every child dir */
        /* Make sure my parent tracks me.  This is okay, since no directory (dir)
         * can have more than one dentry */
        struct dentry *parent = TAILQ_FIRST(&dir->i_dentry);
@@ -769,7 +974,6 @@ int create_symlink(struct inode *dir, struct dentry *dentry,
        if (!new_sym)
                return -1;
        dir->i_op->symlink(dir, dentry, symname);
-       dir->i_nlink++;                 /* TODO: race with this, among other things */
        kref_put(&new_sym->i_kref);
        return 0;
 }
@@ -789,12 +993,16 @@ void inode_release(struct kref *kref)
 {
        struct inode *inode = container_of(kref, struct inode, i_kref);
        TAILQ_REMOVE(&inode->i_sb->s_inodes, inode, i_sb_list);
-       /* 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
+       icache_remove(inode->i_sb, inode->i_ino);
+       /* Might need to write back or delete the file/inode */
+       if (inode->i_nlink) {
+               if (inode->i_state & I_STATE_DIRTY)
+                       inode->i_sb->s_op->write_inode(inode, TRUE);
+       } else {
                inode->i_sb->s_op->delete_inode(inode);
+       }
+       /* Either way, we dealloc the in-memory version */
+       inode->i_sb->s_op->dealloc_inode(inode);        /* FS-specific clean-up */
        kref_put(&inode->i_sb->s_kref);
        assert(inode->i_mapping == &inode->i_pm);
        kmem_cache_free(inode_kcache, inode);
@@ -820,6 +1028,43 @@ void stat_inode(struct inode *inode, struct kstat *kstat)
        kstat->st_ctime = inode->i_ctime;
 }
 
+/* Inode Cache management.  In general, search on the ino, get a refcnt'd value
+ * back.  Remove does not give you a reference back - it should only be called
+ * in inode_release(). */
+struct inode *icache_get(struct super_block *sb, unsigned int ino)
+{
+       /* This is the same style as in pid2proc, it's the "safely create a strong
+        * reference from a weak one, so long as other strong ones exist" pattern */
+       spin_lock(&sb->s_icache_lock);
+       struct inode *inode = hashtable_search(sb->s_icache, (void*)ino);
+       if (inode)
+               if (!kref_get_not_zero(&inode->i_kref, 1))
+                       inode = 0;
+       spin_unlock(&sb->s_icache_lock);
+       return inode;
+}
+
+void icache_put(struct super_block *sb, struct inode *inode)
+{
+       spin_lock(&sb->s_icache_lock);
+       /* there's a race in load_ino() that could trigger this */
+       assert(!hashtable_search(sb->s_icache, (void*)inode->i_ino));
+       hashtable_insert(sb->s_icache, (void*)inode->i_ino, inode);
+       spin_unlock(&sb->s_icache_lock);
+}
+
+struct inode *icache_remove(struct super_block *sb, unsigned int ino)
+{
+       struct inode *inode;
+       /* Presumably these hashtable removals could be easier since callers
+        * actually know who they are (same with the pid2proc hash) */
+       spin_lock(&sb->s_icache_lock);
+       inode = hashtable_remove(sb->s_icache, (void*)ino);
+       spin_unlock(&sb->s_icache_lock);
+       assert(inode && !kref_refcnt(&inode->i_kref));
+       return inode;
+}
+
 /* File functions */
 
 /* Read count bytes from the file into buf, starting at *offset, which is increased
@@ -929,36 +1174,41 @@ ssize_t generic_dir_read(struct file *file, char *u_buf, size_t count,
                          off_t *offset)
 {
        struct kdirent dir_r = {0}, *dirent = &dir_r;
-       unsigned int num_dirents = count / sizeof(struct kdirent);
        int retval = 1;
        size_t amt_copied = 0;
        char *buf_end = u_buf + count;
 
-       if (!count)
-               return 0;
-       if (*offset % sizeof(struct kdirent)) {
-               printk("[kernel] the f_pos for a directory should be dirent-aligned\n");
-               set_errno(EINVAL);
+       if (!S_ISDIR(file->f_dentry->d_inode->i_mode)) {
+               set_errno(ENOTDIR);
                return -1;
        }
-       /* for now, we need to tell readdir which dirent we want */
-       dirent->d_off = *offset / sizeof(struct kdirent);
+       if (!count)
+               return 0;
+       /* start readdir from where it left off: */
+       dirent->d_off = *offset;
        for (; (u_buf < buf_end) && (retval == 1); u_buf += sizeof(struct kdirent)){
                /* TODO: UMEM/KFOP (pin the u_buf in the syscall, ditch the local copy,
                 * get rid of this memcpy and reliance on current, etc).  Might be
-                * tricky with the dirent->d_off */
+                * tricky with the dirent->d_off and trust issues */
                retval = file->f_op->readdir(file, dirent);
-               if (retval < 0)
+               if (retval < 0) {
+                       set_errno(-retval);
                        break;
+               }
+               /* Slight info exposure: could be extra crap after the name in the
+                * dirent (like the name of a deleted file) */
                if (current) {
                        memcpy_to_user(current, u_buf, dirent, sizeof(struct dirent));
                } else {
                        memcpy(u_buf, dirent, sizeof(struct dirent));
                }
                amt_copied += sizeof(struct dirent);
-               dirent->d_off++;
        }
-       *offset += amt_copied;
+       /* Next time read is called, we pick up where we left off */
+       *offset = dirent->d_off;        /* UMEM */
+       /* important to tell them how much they got.  they often keep going til they
+        * get 0 back (in the case of ls).  it's also how much has been read, but it
+        * isn't how much the f_pos has moved (which is opaque to the VFS). */
        return amt_copied;
 }
 
@@ -1019,7 +1269,7 @@ struct file *do_file_open(char *path, int flags, int mode)
                 * but we apply it immediately. */
                if (create_file(parent_i, file_d, mode))        /* sets errno */
                        goto out_file_d;
-               dcache_put(file_d);
+               dcache_put(file_d->d_sb, file_d);
        } else {        /* something already exists */
                /* this can happen due to concurrent access, but needs to be thought
                 * through */
@@ -1077,7 +1327,7 @@ int do_symlink(char *path, const char *symname, int mode)
        parent_i = nd->dentry->d_inode;
        if (create_symlink(parent_i, sym_d, symname, mode))
                goto out_sym_d;
-       dcache_put(sym_d);
+       dcache_put(sym_d->d_sb, sym_d);
        retval = 0;                             /* Note the fall through to the exit paths */
 out_sym_d:
        kref_put(&sym_d->d_kref);
@@ -1142,9 +1392,8 @@ int do_link(char *old_path, char *new_path)
        kref_get(&inode->i_kref, 1);
        link_d->d_inode = inode;
        inode->i_nlink++;
-       parent_dir->i_nlink++;
        TAILQ_INSERT_TAIL(&inode->i_dentry, link_d, d_alias);   /* weak ref */
-       dcache_put(link_d);
+       dcache_put(link_d->d_sb, link_d);
        retval = 0;                             /* Note the fall through to the exit paths */
 out_both_ds:
        kref_put(&old_d->d_kref);
@@ -1189,8 +1438,11 @@ int do_unlink(char *path)
                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 */
+       /* Now that our parent doesn't track us, we need to make sure we aren't
+        * findable via the dentry cache.  DYING, so we will be freed in
+        * dentry_release() */
+       dentry->d_flags |= DENTRY_DYING;
+       dcache_remove(dentry->d_sb, dentry);
        dentry->d_inode->i_nlink--;     /* TODO: race here, esp with a decref */
        /* 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
@@ -1267,7 +1519,7 @@ int do_mkdir(char *path, int mode)
        parent_i = nd->dentry->d_inode;
        if (create_dir(parent_i, dentry, mode))
                goto out_dentry;
-       dcache_put(dentry);
+       dcache_put(dentry->d_sb, dentry);
        retval = 0;                             /* Note the fall through to the exit paths */
 out_dentry:
        kref_put(&dentry->d_kref);
@@ -1304,20 +1556,23 @@ int do_rmdir(char *path)
                set_errno(ENOTDIR);
                goto out_dentry;
        }
-       /* TODO: make sure we aren't a mount or processes root (EBUSY) */
-       /* make sure we are empty.  TODO: Race with this, and anything touching
-        * i_nlink! */
-       if (dentry->d_inode->i_nlink != 1) {
-               set_errno(ENOTEMPTY);
+       if (dentry->d_mount_point) {
+               set_errno(EBUSY);
                goto out_dentry;
        }
-       /* now for the removal */
+       /* TODO: make sure we aren't a mount or processes root (EBUSY) */
+       /* Now for the removal.  the FSs will check if they are empty */
        parent_i = nd->dentry->d_inode;
        error = parent_i->i_op->rmdir(parent_i, dentry);
        if (error < 0) {
                set_errno(-error);
                goto out_dentry;
        }
+       /* Now that our parent doesn't track us, we need to make sure we aren't
+        * findable via the dentry cache.  DYING, so we will be freed in
+        * dentry_release() */
+       dentry->d_flags |= DENTRY_DYING;
+       dcache_remove(dentry->d_sb, dentry);
        /* Decref ourselves, so inode_release() knows we are done */
        dentry->d_inode->i_nlink--;
        TAILQ_REMOVE(&nd->dentry->d_subdirs, dentry, d_subdirs_link);
@@ -1715,10 +1970,9 @@ char *do_getcwd(struct fs_struct *fs_env, char **kfree_this, size_t cwd_l)
 static void print_dir(struct dentry *dentry, char *buf, int depth)
 {
        struct dentry *child_d;
-       struct dirent next;
+       struct dirent next = {0};
        struct file *dir;
        int retval;
-       int child_num = 0;
 
        if (!S_ISDIR(dentry->d_inode->i_mode)) {
                warn("Thought this was only directories!!");
@@ -1727,6 +1981,9 @@ static void print_dir(struct dentry *dentry, char *buf, int depth)
        /* Print this dentry */
        printk("%s%s/ nlink: %d\n", buf, dentry->d_name.name,
               dentry->d_inode->i_nlink);
+       if (dentry->d_mount_point) {
+               dentry = dentry->d_mounted_fs->mnt_root;
+       }
        if (depth >= 32)
                return;
        /* Set buffer for our kids */
@@ -1736,9 +1993,12 @@ static void print_dir(struct dentry *dentry, char *buf, int depth)
                panic("Filesystem seems inconsistent - unable to open a dir!");
        /* Process every child, recursing on directories */
        while (1) {
-               next.d_off = child_num++;
                retval = dir->f_op->readdir(dir, &next);
                if (retval >= 0) {
+                       /* Skip .., ., and empty entries */
+                       if (!strcmp("..", next.d_name) || !strcmp(".", next.d_name) ||
+                           next.d_ino == 0)
+                               goto loop_next;
                        /* there is an entry, now get its dentry */
                        child_d = do_lookup(dentry, next.d_name);
                        if (!child_d)
@@ -1769,6 +2029,7 @@ static void print_dir(struct dentry *dentry, char *buf, int depth)
                        }
                        kref_put(&child_d->d_kref);     
                }
+loop_next:
                if (retval <= 0)
                        break;
        }