public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Dave Hansen <haveblue@us.ibm.com>
To: Mark Fasheh <mark.fasheh@oracle.com>
Cc: linux-kernel@vger.kernel.org, viro@ftp.linux.org.uk,
	herbert@13thfloor.at, hch@infradead.org
Subject: [PATCH] clean up OCFS2 nlink handling
Date: Fri, 04 Aug 2006 14:01:32 -0700	[thread overview]
Message-ID: <1154725292.10109.41.camel@localhost.localdomain> (raw)
In-Reply-To: <20060803002038.GI29686@ca-server1.us.oracle.com>

On Wed, 2006-08-02 at 17:20 -0700, Mark Fasheh wrote:
> > -	saved_nlink = inode->i_nlink;
> > -	if (S_ISDIR(inode->i_mode))
> > -		inode->i_nlink = 0;
> > -	else
> > -		inode->i_nlink--;
> > -
> >  	status = ocfs2_request_unlink_vote(inode, dentry,
> >  					   (unsigned int) inode->i_nlink);
> The network request above needs to send what the new nlink will be set to.
> This is so that the other nodes which have an interest in the inode can
> determine whether the will need to do orphan processing on their last iput.
> 
> Really what they do is just compare against zero and mark the inode with a
> flag indicating that it may have been orphaned. So we could just pass the
> result of inode_is_unlinkable(), but I'd rather preserve the behavior that
> we have today.
> 
> The ugly method is:
> 
> -  	status = ocfs2_request_unlink_vote(inode, dentry,
> -  					   (unsigned int) inode->i_nlink);
> +   	status = ocfs2_request_unlink_vote(inode, dentry,
> +  		S_ISDIR(inode->i_mode) && inode->i_nlink == 2 ? 0 : inode->i_nlink - 1);
> 
> Better yet, we could store the result of the evaluation in a temporary
> variable and pass that through to the request function.

OK, I hope this does it.

-- Dave

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 inode_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.

Signed-off-by: Dave Hansen <haveblue@us.ibm.com>
---

 lxc-dave/fs/ocfs2/namei.c      |   50 
 lxc-dave/fs/ocfs2/namei.c.orig | 2268 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 2294 insertions(+), 24 deletions(-)

diff -puN fs/ocfs2/namei.c~Re-_PATCH_04_28_OCFS2_is_screwy fs/ocfs2/namei.c
--- lxc/fs/ocfs2/namei.c~Re-_PATCH_04_28_OCFS2_is_screwy	2006-08-04 13:53:36.000000000 -0700
+++ lxc-dave/fs/ocfs2/namei.c	2006-08-04 13:59:15.000000000 -0700
@@ -744,11 +744,23 @@ bail:
 	return err;
 }
 
+static inline 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;
@@ -760,6 +772,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);
@@ -823,18 +836,11 @@ 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--;
-
-	status = ocfs2_request_unlink_vote(inode, dentry,
-					   (unsigned int) inode->i_nlink);
+		future_nlink = inode->i_nlink - 1;
+	status = ocfs2_request_unlink_vote(inode, dentry, future_nlink);
 	if (status < 0) {
 		/* This vote should succeed under all normal
 		 * circumstances. */
@@ -842,7 +848,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);
@@ -869,7 +875,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) {
@@ -885,10 +891,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))
+		inode_drop_nlink(inode);
+	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) {
@@ -897,19 +903,15 @@ static int ocfs2_unlink(struct inode *di
 	}
 
 	if (S_ISDIR(inode->i_mode)) {
-		dir->i_nlink--;
 		status = ocfs2_mark_inode_dirty(handle, dir,
 						parent_node_bh);
-		if (status < 0) {
+		if (status < 0)
 			mlog_errno(status);
-			inc_nlink(dir);
-		}
+		else
+			inode_drop_nlink(dir);
 	}
 
 leave:
-	if (status < 0 && saved_nlink)
-		inode->i_nlink = saved_nlink;
-
 	if (handle)
 		ocfs2_commit_trans(handle);
 


  reply	other threads:[~2006-08-04 21:01 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-08-01 23:52 [PATCH 00/28] Mount writer count and read-only bind mounts (v5) Dave Hansen
2006-08-01 23:52 ` [PATCH 01/28] prepare for write access checks: collapse if() Dave Hansen
2006-08-03 14:32   ` Christoph Hellwig
2006-08-01 23:52 ` [PATCH 02/28] r/o bind mount prepwork: move open_namei()'s vfs_create() Dave Hansen
2006-08-03 14:33   ` Christoph Hellwig
2006-08-01 23:52 ` [PATCH 03/28] unlink: monitor i_nlink Dave Hansen
2006-08-03 14:35   ` Christoph Hellwig
2006-08-01 23:52 ` [PATCH 05/28] monitor zeroing of i_nlink Dave Hansen
2006-08-01 23:52 ` [PATCH 04/28] OCFS2 is screwy Dave Hansen
2006-08-02  2:14   ` Mark Fasheh
2006-08-02  3:19     ` [PATCH 04/28] OCFS2 is (not) screwy Dave Hansen
2006-08-02  3:21     ` [PATCH 04/28] OCFS2 is screwy Dave Hansen
2006-08-02  4:34       ` Mark Fasheh
2006-08-03  0:20       ` Mark Fasheh
2006-08-04 21:01         ` Dave Hansen [this message]
2006-08-04 21:38           ` [PATCH] clean up OCFS2 nlink handling Mark Fasheh
2006-08-01 23:52 ` [PATCH 06/28] reintroduce list of vfsmounts over superblock Dave Hansen
2006-08-03 14:39   ` Christoph Hellwig
2006-08-04 21:47     ` Dave Hansen
2006-08-01 23:52 ` [PATCH 07/28] Add vfsmount writer count Dave Hansen
2006-08-01 23:52 ` [PATCH 08/28] record when sb_writer_count elevated for inode Dave Hansen
2006-08-01 23:52 ` [PATCH 09/28] kill open files traverse on remount ro Dave Hansen
2006-08-01 23:52 ` [PATCH 10/28] increment sb writer count when nlink hits zero Dave Hansen
2006-08-01 23:52 ` [PATCH 11/28] elevate writer count for chown and friends Dave Hansen
2006-08-01 23:52 ` [PATCH 12/28] elevate mnt writers for callers of vfs_mkdir() Dave Hansen
2006-08-01 23:52 ` [PATCH 14/28] sys_symlinkat() elevate write count around vfs_symlink() Dave Hansen
2006-08-01 23:52 ` [PATCH 13/28] elevate write count during entire ncp_ioctl() Dave Hansen
2006-08-01 23:52 ` [PATCH 15/28] elevate mount count for extended attributes Dave Hansen
2006-08-01 23:52 ` [PATCH 16/28] sys_linkat(): elevate write count around vfs_link() Dave Hansen
2006-08-01 23:52 ` [PATCH 18/28] unix_find_other() elevate write count for touch_atime() Dave Hansen
2006-08-01 23:52 ` [PATCH 17/28] mount_is_safe(): add comment Dave Hansen
2006-08-01 23:52 ` [PATCH 19/28] elevate write count over calls to vfs_rename() Dave Hansen
2006-08-01 23:52 ` [PATCH 20/28] tricky: elevate write count files are open()ed Dave Hansen
2006-08-01 23:52 ` [PATCH 22/28] elevate write count for do_utimes() Dave Hansen
2006-08-01 23:52 ` [PATCH 21/28] elevate writer count for do_sys_truncate() Dave Hansen
2006-08-01 23:52 ` [PATCH 23/28] elevate write count for do_sys_utime() and touch_atime() Dave Hansen
2006-08-01 23:52 ` [PATCH 24/28] sys_mknodat(): elevate write count for vfs_mknod/create() Dave Hansen
2006-08-01 23:52 ` [PATCH 26/28] do_rmdir(): elevate write count Dave Hansen
2006-08-01 23:52 ` [PATCH 25/28] elevate mnt writers for vfs_unlink() callers Dave Hansen
2006-08-01 23:53 ` [PATCH 27/28] elevate writer count for custom 'struct file' Dave Hansen
2006-08-03 14:42   ` Christoph Hellwig
2006-08-11 20:31     ` Dave Hansen
2006-08-01 23:53 ` [PATCH 28/28] honor r/w changes at do_remount() time Dave Hansen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1154725292.10109.41.camel@localhost.localdomain \
    --to=haveblue@us.ibm.com \
    --cc=hch@infradead.org \
    --cc=herbert@13thfloor.at \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.fasheh@oracle.com \
    --cc=viro@ftp.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox