lk: drop parent's nlink during directory unlink

Alex Chiang achiang at hp.com
Mon May 5 11:52:42 PDT 2008


* Zach Brown <zach.brown at oracle.com>:
> 
> > My doubt comes from, should we be doing a simple drop_nlink()
> > on our parent directory, or should we be expressing that as a
> > change?
> 
> It's fine to modify the vfs structure and then use the change
> func to update the dirty crfs item in the metadata cache from
> the vfs structure.  crfs_change_cinode() does this for the
> parent inode when crfs_meta_change_commit() is called, and it
> updates nlink, so yeah, that part of the patch is fine.

Ah right. I was confusing myself when reading the code thinking
that crfs_change_cinode() was called when we assigned the
function pointer, but forgot that we don't actually *do* the
changes until crfs_meta_change_commit() gets called. ;)

Thanks for the reminder.

> > +	if (S_ISDIR(inode->i_mode))
> > +		drop_nlink(dir);
> 
> I think we also want a drop_nlink(inode) for "." when the inode
> is a directory, too.  And once we have those you might as well
> drop the stale comment about them on the next line down :).

Ok, how about this:

From: Alex Chiang <achiang at hp.com>
Subject: lk: fix '.' and '..' accounting for directories in crfs_unlink

When unlinking a directory, we need to drop_nlink() on both the
inode and the parent directory; otherwise we get -ENOTEMPTY when
we attempt to unlink the parent directory.

Tested with crfs test suite.

Signed-off-by: Alex Chiang <achiang at hp.com>
---
diff -r 77848719c4ba lk/namei.c
--- a/lk/namei.c	Tue Apr 22 14:03:17 2008 -0700
+++ b/lk/namei.c	Mon May 05 12:22:21 2008 -0600
@@ -645,7 +645,10 @@ static int crfs_unlink(struct inode *dir
 	dir->i_mtime = CURRENT_TIME;
 	dir->i_ctime = dir->i_mtime;
 	drop_nlink(inode);
-	/* XXX I'm not sure when we drop nlink for . and .. */
+	if (S_ISDIR(inode->i_mode)) {
+		drop_nlink(inode);
+		drop_nlink(dir);
+	}
 
 	/* now call all the change functions to update the crfs items */
 	crfs_meta_change_commit(info, set, NULL);



More information about the crfs-devel mailing list