All of lore.kernel.org
 help / color / mirror / Atom feed
* 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 related	[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 related	[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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.