All of lore.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Coddington <bcodding@redhat.com>
To: linux-nfs@vger.kernel.org
Cc: jlayton@poochiereds.net, trond.myklebust@primarydata.com,
	anna.schumaker@netapp.com
Subject: [PATCH 10/10] NFS: Deferred unlocks - always unlock on FL_CLOSE
Date: Wed, 14 Oct 2015 14:23:37 -0400	[thread overview]
Message-ID: <93c4aa68a777b232075b9b8252d671635bb1c320.1444846590.git.bcodding@redhat.com> (raw)
In-Reply-To: <cover.1444846590.git.bcodding@redhat.com>
In-Reply-To: <cover.1444846590.git.bcodding@redhat.com>

NFS unlock procedures will wait for IO to complete before sending an unlock.
In the case that this wait is interrupted, an unlock may never be sent if
the unlock is part of cleaning up locks during a close.  This lost lock can
then prevent other clients from locking the file.

Fix this by deferring an unlock that should wait for IO during FL_CLOSE by
copying it to a list on the nfs_lock_context, which can then be used to
release the lock when the IO has completed.

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 fs/nfs/file.c          |   36 +++++++++++++++++++++++++++++++++++-
 fs/nfs/inode.c         |    1 +
 fs/nfs/pagelist.c      |   23 ++++++++++++++++++++---
 include/linux/nfs_fs.h |    7 +++++++
 4 files changed, 63 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index d16c50f..460311a 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -738,6 +738,36 @@ out_noconflict:
 }
 
 static int
+defer_unlk(struct nfs_lock_context *l_ctx, int cmd, struct file_lock *fl)
+{
+	struct inode *inode = d_inode(l_ctx->open_context->dentry);
+	struct nfs_io_counter *c = &l_ctx->io_count;
+	struct nfs_deferred_unlock *dunlk;
+	int status = 0;
+
+	if (atomic_read(&c->io_count) == 0)
+		return 0;
+
+	/* free in nfs_iocounter_dec */
+	dunlk = kmalloc(sizeof(*dunlk), GFP_NOFS);
+	if (dunlk == NULL)
+		return -ENOMEM;
+
+	INIT_LIST_HEAD(&dunlk->list);
+	dunlk->cmd = cmd;
+	memcpy(&dunlk->fl, fl, sizeof(dunlk->fl));
+	spin_lock(&inode->i_lock);
+	if (atomic_read(&c->io_count) != 0) {
+		list_add_tail(&dunlk->list, &l_ctx->dunlk_list);
+		status = -EINPROGRESS;
+	} else {
+		kfree(dunlk);
+	}
+	spin_unlock(&inode->i_lock);
+	return status;
+}
+
+static int
 do_unlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
 {
 	struct inode *inode = filp->f_mapping->host;
@@ -753,7 +783,11 @@ do_unlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
 
 	l_ctx = nfs_get_lock_context(nfs_file_open_context(filp));
 	if (!IS_ERR(l_ctx)) {
-		status = nfs_iocounter_wait(&l_ctx->io_count);
+		if (fl->fl_flags & FL_CLOSE)
+			status = defer_unlk(l_ctx, cmd, fl);
+		else
+			status = nfs_iocounter_wait(&l_ctx->io_count);
+
 		nfs_put_lock_context(l_ctx);
 		if (status < 0)
 			return status;
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 326d9e1..af4f846 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -696,6 +696,7 @@ static void nfs_init_lock_context(struct nfs_lock_context *l_ctx)
 	l_ctx->lockowner.l_owner = current->files;
 	l_ctx->lockowner.l_pid = current->tgid;
 	INIT_LIST_HEAD(&l_ctx->list);
+	INIT_LIST_HEAD(&l_ctx->dunlk_list);
 	nfs_iocounter_init(&l_ctx->io_count);
 }
 
diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
index fe3ddd2..17dd6c0 100644
--- a/fs/nfs/pagelist.c
+++ b/fs/nfs/pagelist.c
@@ -108,9 +108,26 @@ nfs_iocounter_inc(struct nfs_io_counter *c)
 }
 
 static void
-nfs_iocounter_dec(struct nfs_io_counter *c)
+nfs_iocounter_dec(struct nfs_lock_context *l_ctx)
 {
-	if (atomic_dec_and_test(&c->io_count)) {
+	struct nfs_io_counter *c = &l_ctx->io_count;
+	struct inode *inode = d_inode(l_ctx->open_context->dentry);
+
+	if (atomic_dec_and_lock(&c->io_count, &inode->i_lock)) {
+		if (unlikely(!list_empty(&l_ctx->dunlk_list))) {
+			struct nfs_deferred_unlock *dunlk, *tmp;
+			LIST_HEAD(dunlk_list);
+			list_replace_init(&l_ctx->dunlk_list, &dunlk_list);
+			spin_unlock(&inode->i_lock);
+
+			list_for_each_entry_safe(dunlk, tmp, &dunlk_list, list) {
+				NFS_PROTO(inode)->lock(l_ctx->open_context, dunlk->cmd, &dunlk->fl);
+				locks_release_private(&dunlk->fl);
+				kfree(dunlk);
+			}
+		} else {
+			spin_unlock(&inode->i_lock);
+		}
 		clear_bit(NFS_IO_INPROGRESS, &c->flags);
 		smp_mb__after_atomic();
 		wake_up_bit(&c->flags, NFS_IO_INPROGRESS);
@@ -431,7 +448,7 @@ static void nfs_clear_request(struct nfs_page *req)
 		req->wb_page = NULL;
 	}
 	if (l_ctx != NULL) {
-		nfs_iocounter_dec(&l_ctx->io_count);
+		nfs_iocounter_dec(l_ctx);
 		nfs_put_lock_context(l_ctx);
 		req->wb_lock_context = NULL;
 	}
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index c0e9614..ba36498 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -66,12 +66,19 @@ struct nfs_io_counter {
 	atomic_t io_count;
 };
 
+struct nfs_deferred_unlock {
+	struct list_head list;
+	int cmd;
+	struct file_lock fl;
+};
+
 struct nfs_lock_context {
 	atomic_t count;
 	struct list_head list;
 	struct nfs_open_context *open_context;
 	struct nfs_lockowner lockowner;
 	struct nfs_io_counter io_count;
+	struct list_head dunlk_list;
 };
 
 struct nfs4_state;
-- 
1.7.1


  parent reply	other threads:[~2015-10-14 18:23 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-14 18:23 [PATCH 00/10] locking fixups for NFS Benjamin Coddington
2015-10-14 18:23 ` [PATCH 01/10] NFS: keep nfs4_state for nfs4_lock_state cleanup Benjamin Coddington
2015-10-14 18:23 ` [PATCH 02/10] NFS4: remove a redundant lock range checks Benjamin Coddington
2015-10-14 18:23 ` [PATCH 03/10] NFS: Move the flock open mode check into nfs_flock() Benjamin Coddington
2015-10-14 18:23 ` [PATCH 04/10] NFS: Pass nfs_open_context instead of file to the lock procs Benjamin Coddington
2015-10-14 18:23 ` [PATCH 05/10] NFSv4: Pass nfs_open_context instead of nfs4_state to nfs4_proc_unlck() Benjamin Coddington
2015-10-14 18:23 ` [PATCH 06/10] lockd: Plumb nfs_open_context into nlm client unlock Benjamin Coddington
2015-10-14 18:23 ` [PATCH 07/10] lockd: Send the inode to nlmclnt_setlockargs() Benjamin Coddington
2015-10-14 18:23 ` [PATCH 08/10] NFS: Move do_vfs_lock to shared inline Benjamin Coddington
2015-10-14 19:55   ` Jeff Layton
2015-10-21 21:48     ` Trond Myklebust
2015-10-21 23:49       ` Jeff Layton
2015-10-22  0:11       ` Benjamin Coddington
2015-10-22  8:34       ` Christoph Hellwig
2015-10-22 15:50         ` Benjamin Coddington
2015-10-14 18:23 ` [PATCH 09/10] locks: Use more file_inode and fix a comment Benjamin Coddington
2015-10-14 19:56   ` Jeff Layton
2015-10-14 18:23 ` Benjamin Coddington [this message]
2015-10-14 20:30   ` [PATCH 10/10] NFS: Deferred unlocks - always unlock on FL_CLOSE Jeff Layton
2015-12-07 16:05     ` Benjamin Coddington

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=93c4aa68a777b232075b9b8252d671635bb1c320.1444846590.git.bcodding@redhat.com \
    --to=bcodding@redhat.com \
    --cc=anna.schumaker@netapp.com \
    --cc=jlayton@poochiereds.net \
    --cc=linux-nfs@vger.kernel.org \
    --cc=trond.myklebust@primarydata.com \
    /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.