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