All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stanislav Kinsbursky <skinsbursky@parallels.com>
To: bfields@fieldses.org
Cc: linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org,
	devel@openvz.org
Subject: [PATCH] NFSd: fix locking in nfsd_forget_delegations()
Date: Tue, 22 May 2012 14:25:14 +0400	[thread overview]
Message-ID: <20120522102444.6988.45434.stgit@localhost.localdomain> (raw)

This patch adds recall_lock hold to nfsd_forget_delegations() to protect
nfsd_process_n_delegations() call.
Also, looks like it would be better to collect delegations to some local
on-stack list, and then unhash collected list. This split allows to simplify
locking, because delegation traversing is protected by recall_lock, when
delegation unhash is protected by client_mutex.

Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
---
 fs/nfsd/nfs4state.c |   32 ++++++++++++++++++++++++--------
 1 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 21266c7..f004e61 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2597,7 +2597,7 @@ out:
 	return ret;
 }
 
-static void nfsd_break_one_deleg(struct nfs4_delegation *dp)
+static void nfsd_break_one_deleg(struct nfs4_delegation *dp, void *data)
 {
 	/* We're assuming the state code never drops its reference
 	 * without first removing the lease.  Since we're in this lease
@@ -2633,7 +2633,7 @@ static void nfsd_break_deleg_cb(struct file_lock *fl)
 	spin_lock(&recall_lock);
 	fp->fi_had_conflict = true;
 	list_for_each_entry(dp, &fp->fi_delegations, dl_perfile)
-		nfsd_break_one_deleg(dp);
+		nfsd_break_one_deleg(dp, NULL);
 	spin_unlock(&recall_lock);
 }
 
@@ -4694,7 +4694,7 @@ void nfsd_forget_openowners(u64 num)
 	printk(KERN_INFO "NFSD: Forgot %d open owners", count);
 }
 
-int nfsd_process_n_delegations(u64 num, void (*deleg_func)(struct nfs4_delegation *))
+int nfsd_process_n_delegations(u64 num, void (*deleg_func)(struct nfs4_delegation *, void *), void *data)
 {
 	int i, count = 0;
 	struct nfs4_file *fp, *fnext;
@@ -4703,7 +4703,7 @@ int nfsd_process_n_delegations(u64 num, void (*deleg_func)(struct nfs4_delegatio
 	for (i = 0; i < FILE_HASH_SIZE; i++) {
 		list_for_each_entry_safe(fp, fnext, &file_hashtbl[i], fi_hash) {
 			list_for_each_entry_safe(dp, dnext, &fp->fi_delegations, dl_perfile) {
-				deleg_func(dp);
+				deleg_func(dp, data);
 				if (++count == num)
 					return count;
 			}
@@ -4713,15 +4713,31 @@ int nfsd_process_n_delegations(u64 num, void (*deleg_func)(struct nfs4_delegatio
 	return count;
 }
 
+/* Called under the recall_lock spinlock. */
+static void
+collect_delegation(struct nfs4_delegation *dp, void *data)
+{
+	struct list_head *list = data;
+
+	list_move(&dp->dl_perfile, list);
+}
+
 void nfsd_forget_delegations(u64 num)
 {
 	unsigned int count;
+	struct nfs4_delegation *dp, *dnext;
+	LIST_HEAD(unhash_list);
 
-	nfs4_lock_state();
-	count = nfsd_process_n_delegations(num, unhash_delegation);
-	nfs4_unlock_state();
+	spin_lock(&recall_lock);
+	count = nfsd_process_n_delegations(num, collect_delegation, &unhash_list);
+	spin_unlock(&recall_lock);
 
 	printk(KERN_INFO "NFSD: Forgot %d delegations", count);
+
+	nfs4_lock_state();
+	list_for_each_entry_safe(dp, dnext, &unhash_list, dl_perfile)
+		unhash_delegation(dp);
+	nfs4_unlock_state();
 }
 
 void nfsd_recall_delegations(u64 num)
@@ -4730,7 +4746,7 @@ void nfsd_recall_delegations(u64 num)
 
 	nfs4_lock_state();
 	spin_lock(&recall_lock);
-	count = nfsd_process_n_delegations(num, nfsd_break_one_deleg);
+	count = nfsd_process_n_delegations(num, nfsd_break_one_deleg, NULL);
 	spin_unlock(&recall_lock);
 	nfs4_unlock_state();
 


             reply	other threads:[~2012-05-22 10:25 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-22 10:25 Stanislav Kinsbursky [this message]
2012-05-23 21:31 ` [PATCH] NFSd: fix locking in nfsd_forget_delegations() J. Bruce Fields
2012-05-24  4:41   ` Stanislav Kinsbursky
2012-05-24 10:56     ` J. Bruce Fields
2012-05-24 11:09       ` Stanislav Kinsbursky

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=20120522102444.6988.45434.stgit@localhost.localdomain \
    --to=skinsbursky@parallels.com \
    --cc=bfields@fieldses.org \
    --cc=devel@openvz.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@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.