All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steve Dickson <SteveD@redhat.com>
To: Alexander Viro <aviro@redhat.com>, Christoph Hellwig <hch@infradead.org>
Cc: linux-fsdevel@vger.kernel.org,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: [PATCH] NFS: Stop sillyname renames and unmounts from racing
Date: Sat, 03 Nov 2007 07:09:25 -0400	[thread overview]
Message-ID: <472C56E5.1040901@RedHat.com> (raw)

The following patch stops NFS sillyname renames and umounts from racing.
I have a test script does the following:
     1) start nfs server
      2) mount loopback
      3) open file in background
      4) remove file
      5) stop nfs server
      6) kill -9 process which has file open
      7) restart nfs server
      8) umount looback mount.

After umount I got the "VFS: Busy inodes after unmount" message
because the processing of the rename has not finished.

Below is a patch that the uses the new silly_count mechanism to
synchronize sillyname processing and umounts. The patch introduces a
nfs_put_super() routine that waits until the nfsi->silly_count count
is zero.

A side-effect of finding and waiting for all the inode to
find the sillyname processing, is I need to traverse
the sb->s_inodes list in the supper block. To do that
safely the inode_lock spin lock has to be held. So for
modules to be able to "see" that lock I needed to
EXPORT_SYMBOL_GPL() it.

Any objections to exporting the inode_lock spin lock?
If so, how should modules _safely_ access the s_inode list?

steved.


Author: Steve Dickson <steved@redhat.com>
Date:   Wed Oct 31 12:19:26 2007 -0400

     Close a unlink/sillyname rename and umount race by added a
     nfs_put_super routine that will run through all the inode
     currently on the super block, waiting for those that are
     in the middle of a sillyname rename or removal.

     This patch stop the infamous "VFS: Busy inodes after unmount... "
     warning during umounts.

     Signed-off-by: Steve Dickson <steved@redhat.com>

diff --git a/fs/inode.c b/fs/inode.c
index ed35383..da9034a 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -81,6 +81,7 @@ static struct hlist_head *inode_hashtable __read_mostly;
   * the i_state of an inode while it is in use..
   */
  DEFINE_SPINLOCK(inode_lock);
+EXPORT_SYMBOL_GPL(inode_lock);

  /*
   * iprune_mutex provides exclusion between the kswapd or try_to_free_pages
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index fa517ae..2ac3c34 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -48,6 +48,7 @@
  #include <linux/nfs_xdr.h>
  #include <linux/magic.h>
  #include <linux/parser.h>
+#include <linux/writeback.h>

  #include <asm/system.h>
  #include <asm/uaccess.h>
@@ -202,6 +203,7 @@ static int nfs_get_sb(struct file_system_type *,
int, const char *, void *, stru
  static int nfs_xdev_get_sb(struct file_system_type *fs_type,
  		int flags, const char *dev_name, void *raw_data, struct vfsmount *mnt);
  static void nfs_kill_super(struct super_block *);
+static void nfs_put_super(struct super_block *);

  static struct file_system_type nfs_fs_type = {
  	.owner		= THIS_MODULE,
@@ -223,6 +225,7 @@ static const struct super_operations nfs_sops = {
  	.alloc_inode	= nfs_alloc_inode,
  	.destroy_inode	= nfs_destroy_inode,
  	.write_inode	= nfs_write_inode,
+	.put_super	= nfs_put_super,
  	.statfs		= nfs_statfs,
  	.clear_inode	= nfs_clear_inode,
  	.umount_begin	= nfs_umount_begin,
@@ -1767,6 +1770,30 @@ static void nfs4_kill_super(struct super_block *sb)
  	nfs_free_server(server);
  }

+void nfs_put_super(struct super_block *sb)
+{
+	struct inode *inode;
+	struct nfs_inode *nfsi;
+	/*
+	 * Make sure there are no outstanding renames
+	 */
+relock:
+	spin_lock(&inode_lock);
+	list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
+		nfsi = NFS_I(inode);
+		if (atomic_read(&nfsi->silly_count) > 0) {
+			/* Keep this inode around  during the wait */
+			atomic_inc(&inode->i_count);
+			spin_unlock(&inode_lock);
+			wait_event(nfsi->waitqueue,
+				atomic_read(&nfsi->silly_count) == 1);
+			iput(inode);
+			goto relock;
+		}
+	}
+	spin_unlock(&inode_lock);
+}
+
  /*
   * Clone an NFS4 server record on xdev traversal (FSID-change)
   */


             reply	other threads:[~2007-11-03 11:13 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-11-03 11:09 Steve Dickson [this message]
2007-11-06  5:06 ` [PATCH] NFS: Stop sillyname renames and unmounts from racing Andrew Morton
2007-11-06  5:06   ` Andrew Morton
2007-11-06  5:15   ` Alexander Viro
2007-11-06  5:15     ` Alexander Viro
2007-11-08  9:05     ` Steve Dickson
2007-11-08  9:05       ` Steve Dickson
2007-11-06  8:24   ` Benny Halevy
2007-11-06  8:24     ` Benny Halevy
2007-11-06  8:50     ` Alexander Viro
  -- strict thread matches above, loose matches on Subject: below --
2007-10-31 16:40 Steve Dickson

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=472C56E5.1040901@RedHat.com \
    --to=steved@redhat.com \
    --cc=aviro@redhat.com \
    --cc=hch@infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /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 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.