* Re: - r-o-bind-mount-clean-up-ocfs2-nlink-handling.patch removed from -mm tree [not found] <200609130506.k8D56U3m018878@shell0.pdx.osdl.net> @ 2006-09-13 18:27 ` Mark Fasheh 2006-09-13 18:36 ` Dave Hansen 0 siblings, 1 reply; 4+ messages in thread From: Mark Fasheh @ 2006-09-13 18:27 UTC (permalink / raw) To: linux-kernel; +Cc: haveblue, hch, viro, mm-commits, akpm On Tue, Sep 12, 2006 at 10:06:30PM -0700, akpm@osdl.org wrote: > The patch titled > > r/o bind mounts: clean up OCFS2 nlink handling > > has been removed from the -mm tree. Its filename is > > r-o-bind-mount-clean-up-ocfs2-nlink-handling.patch > > This patch was dropped because git-ocfs2 changes broke it. New patch, please. Yep, that was very likely due to my dentry vote removal changes. Dave, how's this one look? I guess I'll leave the same description message below... Subject: r/o bind mounts: clean up OCFS2 nlink handling From: Dave Hansen <haveblue@us.ibm.com> OCFS2 does some operations on i_nlink, then reverts them if some of its operations fail to complete. This does not fit in well with the drop_nlink() logic where we expect i_nlink to stay at zero once it gets there. So, delay all of the nlink operations until we're sure that the operations have completed. Also, introduce a small helper to check whether an inode has proper "unlinkable" i_nlink counts no matter whether it is a directory or regular inode. This patch is broken out from the others because it does contain some logical changes. Cleanups to apply against current git-ocfs2 changes by Mark Fasheh. Cc: Dave Hansen <haveblue@us.ibm.com> Signed-off-by: Mark Fasheh <mark.fasheh@oracle.com> Cc: Christoph Hellwig <hch@lst.de> Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: Andrew Morton <akpm@osdl.org> diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c index 4cfc061..4f74cd9 100644 --- a/fs/ocfs2/namei.c +++ b/fs/ocfs2/namei.c @@ -797,11 +797,23 @@ static int ocfs2_remote_dentry_delete(st return ret; } +static inline int inode_is_unlinkable(struct inode *inode) +{ + if (S_ISDIR(inode->i_mode)) { + if (inode->i_nlink == 2) + return 1; + return 0; + } + + if (inode->i_nlink == 1) + return 1; + return 0; +} + static int ocfs2_unlink(struct inode *dir, struct dentry *dentry) { int status; - unsigned int saved_nlink = 0; struct inode *inode = dentry->d_inode; struct ocfs2_super *osb = OCFS2_SB(dir->i_sb); u64 blkno; @@ -813,6 +825,7 @@ static int ocfs2_unlink(struct inode *di struct buffer_head *dirent_bh = NULL; char orphan_name[OCFS2_ORPHAN_NAMELEN + 1]; struct buffer_head *orphan_entry_bh = NULL; + unsigned int future_nlink; mlog_entry("(0x%p, 0x%p, '%.*s')\n", dir, dentry, dentry->d_name.len, dentry->d_name.name); @@ -876,15 +889,10 @@ static int ocfs2_unlink(struct inode *di } } - /* There are still a few steps left until we can consider the - * unlink to have succeeded. Save off nlink here before - * modification so we can set it back in case we hit an issue - * before commit. */ - saved_nlink = inode->i_nlink; - if (S_ISDIR(inode->i_mode)) - inode->i_nlink = 0; + if (S_ISDIR(inode->i_mode) && (inode->i_nlink == 2)) + future_nlink = 0; else - inode->i_nlink--; + future_nlink = inode->i_nlink - 1; status = ocfs2_remote_dentry_delete(dentry); if (status < 0) { @@ -894,7 +902,7 @@ static int ocfs2_unlink(struct inode *di goto leave; } - if (!inode->i_nlink) { + if (inode_is_unlinkable(inode)) { status = ocfs2_prepare_orphan_dir(osb, handle, inode, orphan_name, &orphan_entry_bh); @@ -921,7 +929,7 @@ static int ocfs2_unlink(struct inode *di fe = (struct ocfs2_dinode *) fe_bh->b_data; - if (!inode->i_nlink) { + if (inode_is_unlinkable(inode)) { status = ocfs2_orphan_add(osb, handle, inode, fe, orphan_name, orphan_entry_bh); if (status < 0) { @@ -937,10 +945,10 @@ static int ocfs2_unlink(struct inode *di goto leave; } - /* We can set nlink on the dinode now. clear the saved version - * so that it doesn't get set later. */ + if (S_ISDIR(inode->i_mode)) + drop_nlink(inode); + drop_nlink(inode); fe->i_links_count = cpu_to_le16(inode->i_nlink); - saved_nlink = 0; status = ocfs2_journal_dirty(handle, fe_bh); if (status < 0) { @@ -949,7 +957,7 @@ static int ocfs2_unlink(struct inode *di } if (S_ISDIR(inode->i_mode)) { - dir->i_nlink--; + drop_nlink(dir); status = ocfs2_mark_inode_dirty(handle, dir, parent_node_bh); if (status < 0) { @@ -959,9 +967,6 @@ static int ocfs2_unlink(struct inode *di } leave: - if (status < 0 && saved_nlink) - inode->i_nlink = saved_nlink; - if (handle) ocfs2_commit_trans(handle); ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: - r-o-bind-mount-clean-up-ocfs2-nlink-handling.patch removed from -mm tree 2006-09-13 18:27 ` - r-o-bind-mount-clean-up-ocfs2-nlink-handling.patch removed from -mm tree Mark Fasheh @ 2006-09-13 18:36 ` Dave Hansen 2006-09-13 19:12 ` Mark Fasheh 0 siblings, 1 reply; 4+ messages in thread From: Dave Hansen @ 2006-09-13 18:36 UTC (permalink / raw) To: Mark Fasheh; +Cc: linux-kernel, hch, viro, mm-commits, akpm On Wed, 2006-09-13 at 11:27 -0700, Mark Fasheh wrote: > On Tue, Sep 12, 2006 at 10:06:30PM -0700, akpm@osdl.org wrote: > > The patch titled > > > > r/o bind mounts: clean up OCFS2 nlink handling > > > > has been removed from the -mm tree. Its filename is > > > > r-o-bind-mount-clean-up-ocfs2-nlink-handling.patch > > > > This patch was dropped because git-ocfs2 changes broke it. New patch, please. > Yep, that was very likely due to my dentry vote removal changes. > > Dave, how's this one look? I guess I'll leave the same description message > below... I was _just_ fighting with your git tree to see what was conflicting! You have impeccable timing. > static int ocfs2_unlink(struct inode *dir, > struct dentry *dentry) > { > int status; > - unsigned int saved_nlink = 0; > struct inode *inode = dentry->d_inode; > struct ocfs2_super *osb = OCFS2_SB(dir->i_sb); > u64 blkno; > @@ -813,6 +825,7 @@ static int ocfs2_unlink(struct inode *di > struct buffer_head *dirent_bh = NULL; > char orphan_name[OCFS2_ORPHAN_NAMELEN + 1]; > struct buffer_head *orphan_entry_bh = NULL; > + unsigned int future_nlink; > > mlog_entry("(0x%p, 0x%p, '%.*s')\n", dir, dentry, > dentry->d_name.len, dentry->d_name.name); > @@ -876,15 +889,10 @@ static int ocfs2_unlink(struct inode *di > } > } > > - /* There are still a few steps left until we can consider the > - * unlink to have succeeded. Save off nlink here before > - * modification so we can set it back in case we hit an issue > - * before commit. */ > - saved_nlink = inode->i_nlink; > - if (S_ISDIR(inode->i_mode)) > - inode->i_nlink = 0; > + if (S_ISDIR(inode->i_mode) && (inode->i_nlink == 2)) > + future_nlink = 0; > else > - inode->i_nlink--; > + future_nlink = inode->i_nlink - 1; Now that the vote call is gone, I don't think we even use future_nlink. Can we just kill this entire section? -- Dave ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: - r-o-bind-mount-clean-up-ocfs2-nlink-handling.patch removed from -mm tree 2006-09-13 18:36 ` Dave Hansen @ 2006-09-13 19:12 ` Mark Fasheh 2006-09-13 19:18 ` Dave Hansen 0 siblings, 1 reply; 4+ messages in thread From: Mark Fasheh @ 2006-09-13 19:12 UTC (permalink / raw) To: Dave Hansen; +Cc: linux-kernel, hch, viro, mm-commits, akpm On Wed, Sep 13, 2006 at 11:36:36AM -0700, Dave Hansen wrote: > I was _just_ fighting with your git tree to see what was conflicting! > You have impeccable timing. Heh, glad that the timing worked out :) > > - /* There are still a few steps left until we can consider the > > - * unlink to have succeeded. Save off nlink here before > > - * modification so we can set it back in case we hit an issue > > - * before commit. */ > > - saved_nlink = inode->i_nlink; > > - if (S_ISDIR(inode->i_mode)) > > - inode->i_nlink = 0; > > + if (S_ISDIR(inode->i_mode) && (inode->i_nlink == 2)) > > + future_nlink = 0; > > else > > - inode->i_nlink--; > > + future_nlink = inode->i_nlink - 1; > > Now that the vote call is gone, I don't think we even use future_nlink. > Can we just kill this entire section? Yeah, good catch. Lets try this one again... Subject: r/o bind mounts: clean up OCFS2 nlink handling From: Dave Hansen <haveblue@us.ibm.com> OCFS2 does some operations on i_nlink, then reverts them if some of its operations fail to complete. This does not fit in well with the drop_nlink() logic where we expect i_nlink to stay at zero once it gets there. So, delay all of the nlink operations until we're sure that the operations have completed. Also, introduce a small helper to check whether an inode has proper "unlinkable" i_nlink counts no matter whether it is a directory or regular inode. This patch is broken out from the others because it does contain some logical changes. Cc: Dave Hansen <haveblue@us.ibm.com> Signed-off-by: Mark Fasheh <mark.fasheh@oracle.com> Cc: Christoph Hellwig <hch@lst.de> Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: Andrew Morton <akpm@osdl.org> diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c index 4cfc061..1c3d296 100644 --- a/fs/ocfs2/namei.c +++ b/fs/ocfs2/namei.c @@ -797,11 +797,23 @@ static int ocfs2_remote_dentry_delete(st return ret; } +static inline int inode_is_unlinkable(struct inode *inode) +{ + if (S_ISDIR(inode->i_mode)) { + if (inode->i_nlink == 2) + return 1; + return 0; + } + + if (inode->i_nlink == 1) + return 1; + return 0; +} + static int ocfs2_unlink(struct inode *dir, struct dentry *dentry) { int status; - unsigned int saved_nlink = 0; struct inode *inode = dentry->d_inode; struct ocfs2_super *osb = OCFS2_SB(dir->i_sb); u64 blkno; @@ -876,16 +888,6 @@ static int ocfs2_unlink(struct inode *di } } - /* There are still a few steps left until we can consider the - * unlink to have succeeded. Save off nlink here before - * modification so we can set it back in case we hit an issue - * before commit. */ - saved_nlink = inode->i_nlink; - if (S_ISDIR(inode->i_mode)) - inode->i_nlink = 0; - else - inode->i_nlink--; - status = ocfs2_remote_dentry_delete(dentry); if (status < 0) { /* This vote should succeed under all normal @@ -894,7 +896,7 @@ static int ocfs2_unlink(struct inode *di goto leave; } - if (!inode->i_nlink) { + if (inode_is_unlinkable(inode)) { status = ocfs2_prepare_orphan_dir(osb, handle, inode, orphan_name, &orphan_entry_bh); @@ -921,7 +923,7 @@ static int ocfs2_unlink(struct inode *di fe = (struct ocfs2_dinode *) fe_bh->b_data; - if (!inode->i_nlink) { + if (inode_is_unlinkable(inode)) { status = ocfs2_orphan_add(osb, handle, inode, fe, orphan_name, orphan_entry_bh); if (status < 0) { @@ -937,10 +939,10 @@ static int ocfs2_unlink(struct inode *di goto leave; } - /* We can set nlink on the dinode now. clear the saved version - * so that it doesn't get set later. */ + if (S_ISDIR(inode->i_mode)) + drop_nlink(inode); + drop_nlink(inode); fe->i_links_count = cpu_to_le16(inode->i_nlink); - saved_nlink = 0; status = ocfs2_journal_dirty(handle, fe_bh); if (status < 0) { @@ -949,7 +951,7 @@ static int ocfs2_unlink(struct inode *di } if (S_ISDIR(inode->i_mode)) { - dir->i_nlink--; + drop_nlink(dir); status = ocfs2_mark_inode_dirty(handle, dir, parent_node_bh); if (status < 0) { @@ -959,9 +961,6 @@ static int ocfs2_unlink(struct inode *di } leave: - if (status < 0 && saved_nlink) - inode->i_nlink = saved_nlink; - if (handle) ocfs2_commit_trans(handle); ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: - r-o-bind-mount-clean-up-ocfs2-nlink-handling.patch removed from -mm tree 2006-09-13 19:12 ` Mark Fasheh @ 2006-09-13 19:18 ` Dave Hansen 0 siblings, 0 replies; 4+ messages in thread From: Dave Hansen @ 2006-09-13 19:18 UTC (permalink / raw) To: Mark Fasheh; +Cc: linux-kernel, hch, viro, mm-commits, akpm On Wed, 2006-09-13 at 12:12 -0700, Mark Fasheh wrote: > > Now that the vote call is gone, I don't think we even use future_nlink. > > Can we just kill this entire section? > Yeah, good catch. Lets try this one again... Looks good to me. Thanks a bunch for updating this one. Signed-off-by: Dave Hansen <haveblue@us.ibm.com> -- Dave ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2006-09-13 19:19 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <200609130506.k8D56U3m018878@shell0.pdx.osdl.net>
2006-09-13 18:27 ` - r-o-bind-mount-clean-up-ocfs2-nlink-handling.patch removed from -mm tree Mark Fasheh
2006-09-13 18:36 ` Dave Hansen
2006-09-13 19:12 ` Mark Fasheh
2006-09-13 19:18 ` Dave Hansen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®