[Devel] [PATCH RH7] nfs: add some test on blocked locks operations

Cyrill Gorcunov gorcunov at gmail.com
Thu Oct 21 18:51:58 MSK 2021


From: Cyrill Gorcunov <gorcunov at virtuozzo.com>

It looks like plain splinlock guards are not suffice enough to prevent
reuse the same lock entries when several clients issues an operation
simultaneously and one of them are in error path which could possibly
lead to the list corruption. We've been said that this is impossible
but taking into account the backtrace we are receiving it seems such
scenario is still possible.

Thus lets do a trick - add a reference to the blocked lock entry and
when we're in potential race phase lets increment it thus if another
client will try to reuse it before we finished processing the number
of references will be more than one, and finally print a warning if
it happens.

The patch is done on top

branch branch-rh7-3.10.0-1160.41.1.vz7.183.x-ovz
commit 98c614598df6a61ba38af34c370ee44690e785f9
issue https://jira.sw.ru/browse/PSBM-133274

CC: Vasily Averin <vvs at virtuozzo.com>
CC: Kirill Tkhai <ktkhai at virtuozzo.com>
Signed-off-by: Cyrill Gorcunov <gorcunov at gmail.com>
---
 fs/nfsd/nfs4state.c |   38 ++++++++++++++++++++++++++++++++++++++
 fs/nfsd/state.h     |    1 +
 2 files changed, 39 insertions(+)

--- vzkernel7.orig/fs/nfsd/nfs4state.c
+++ vzkernel7/fs/nfsd/nfs4state.c
@@ -211,6 +211,33 @@ static void nfsd4_put_session(struct nfs
 	spin_unlock(&nn->client_lock);
 }
 
+static void nbl_ref_init(struct nfsd4_blocked_lock *nbl)
+{
+	kref_init(&nbl->kref);
+}
+
+static void nbl_ref_release(struct kref *kref)
+{
+	/* We don't need to release anything, the caller will */
+}
+
+static void nbl_ref_put(struct nfsd4_blocked_lock *nbl)
+{
+	kref_put(&nbl->kref, nbl_ref_release);
+}
+
+static void nbl_ref_get(struct nfsd4_blocked_lock *nbl)
+{
+	kref_get(&nbl->kref);
+}
+
+static void nbl_ref_verify(struct nfsd4_blocked_lock *nbl)
+{
+	int c = kref_read(&nbl->kref);
+	if (c > 1)
+		pr_warn_once("nfsd: infly reuse of block %d\n", c);
+}
+
 static struct nfsd4_blocked_lock *
 find_blocked_lock(struct nfs4_lockowner *lo, struct knfsd_fh *fh,
 			struct nfsd_net *nn)
@@ -220,6 +247,7 @@ find_blocked_lock(struct nfs4_lockowner
 	spin_lock(&nn->blocked_locks_lock);
 	list_for_each_entry(cur, &lo->lo_blocked, nbl_list) {
 		if (fh_match(fh, &cur->nbl_fh)) {
+			nbl_ref_verify(cur);
 			list_del_init(&cur->nbl_list);
 			list_del_init(&cur->nbl_lru);
 			found = cur;
@@ -244,6 +272,7 @@ find_or_allocate_block(struct nfs4_locko
 		if (nbl) {
 			INIT_LIST_HEAD(&nbl->nbl_list);
 			INIT_LIST_HEAD(&nbl->nbl_lru);
+			nbl_ref_init(nbl);
 			fh_copy_shallow(&nbl->nbl_fh, fh);
 			locks_init_lock(&nbl->nbl_lock);
 			nfsd4_init_cb(&nbl->nbl_cb, lo->lo_owner.so_client,
@@ -258,6 +287,8 @@ static void
 free_blocked_lock(struct nfsd4_blocked_lock *nbl)
 {
 	locks_release_private(&nbl->nbl_lock);
+	nbl_ref_verify(nbl);
+	nbl_ref_put(nbl);
 	kfree(nbl);
 }
 
@@ -277,6 +308,7 @@ remove_blocked_locks(struct nfs4_lockown
 					nbl_list);
 		list_del_init(&nbl->nbl_list);
 		list_move(&nbl->nbl_lru, &reaplist);
+		nbl_ref_verify(nbl);
 	}
 	spin_unlock(&nn->blocked_locks_lock);
 
@@ -4766,6 +4798,7 @@ nfs4_laundromat(struct nfsd_net *nn)
 		}
 		list_move(&nbl->nbl_lru, &reaplist);
 		list_del_init(&nbl->nbl_list);
+		nbl_ref_verify(nbl);
 	}
 	spin_unlock(&nn->blocked_locks_lock);
 
@@ -5557,6 +5590,7 @@ nfsd4_lm_notify(struct file_lock *fl)
 	if (!list_empty(&nbl->nbl_list)) {
 		list_del_init(&nbl->nbl_list);
 		list_del_init(&nbl->nbl_lru);
+		nbl_ref_verify(nbl);
 		queue = true;
 	}
 	spin_unlock(&nn->blocked_locks_lock);
@@ -5993,6 +6027,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struc
 	if (fl_flags & FL_SLEEP) {
 		nbl->nbl_time = jiffies;
 		spin_lock(&nn->blocked_locks_lock);
+		nbl_ref_get(nbl);
 		list_add_tail(&nbl->nbl_list, &lock_sop->lo_blocked);
 		list_add_tail(&nbl->nbl_lru, &nn->blocked_locks_lru);
 		spin_unlock(&nn->blocked_locks_lock);
@@ -6005,6 +6040,8 @@ nfsd4_lock(struct svc_rqst *rqstp, struc
 		status = 0;
 		break;
 	case FILE_LOCK_DEFERRED:
+		if (fl_flags & FL_SLEEP)
+			nbl_ref_put(nbl);
 		nbl = NULL;
 		/* Fallthrough */
 	case -EAGAIN:		/* conflock holds conflicting lock */
@@ -6025,6 +6062,7 @@ out:
 		/* dequeue it if we queued it before */
 		if (fl_flags & FL_SLEEP) {
 			spin_lock(&nn->blocked_locks_lock);
+			nbl_ref_put(nbl);
 			list_del_init(&nbl->nbl_list);
 			list_del_init(&nbl->nbl_lru);
 			spin_unlock(&nn->blocked_locks_lock);
--- vzkernel7.orig/fs/nfsd/state.h
+++ vzkernel7/fs/nfsd/state.h
@@ -583,6 +583,7 @@ enum nfsd4_cb_op {
 struct nfsd4_blocked_lock {
 	struct list_head	nbl_list;
 	struct list_head	nbl_lru;
+	struct kref		kref;
 	unsigned long		nbl_time;
 	struct file_lock	nbl_lock;
 	struct knfsd_fh		nbl_fh;


More information about the Devel mailing list