All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: linux-afs@lists.infradead.org
Cc: Jonathan Billings <jsbillin@umich.edu>,
	dhowells@redhat.com, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH 07/10] afs: Handle lock rpc ops failing on a file that got deleted
Date: Fri, 15 Mar 2019 23:00:14 +0000	[thread overview]
Message-ID: <155269081456.8537.5454244654980801039.stgit@warthog.procyon.org.uk> (raw)
In-Reply-To: <155269076033.8537.10785090349210421514.stgit@warthog.procyon.org.uk>

Holding a file lock on an AFS file does not prevent it from being deleted
on the server, so we need to handle an error resulting from that when we
try setting, extending or releasing a lock.

Fix this by adding a "deleted" lock state and cancelling the lock extension
process for that file and aborting all waiters for the lock.

Fixes: 0fafdc9f888b ("afs: Fix file locking")
Reported-by: Jonathan Billings <jsbillin@umich.edu>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/afs/flock.c             |   62 ++++++++++++++++++++++++++++++++++++++++++--
 fs/afs/internal.h          |    1 +
 include/trace/events/afs.h |    7 ++++-
 3 files changed, 66 insertions(+), 4 deletions(-)

diff --git a/fs/afs/flock.c b/fs/afs/flock.c
index 2237ab4ea111..08b06f53a375 100644
--- a/fs/afs/flock.c
+++ b/fs/afs/flock.c
@@ -157,6 +157,28 @@ static void afs_next_locker(struct afs_vnode *vnode, int error)
 	_leave("");
 }
 
+/*
+ * Kill off all waiters in the the pending lock queue due to the vnode being
+ * deleted.
+ */
+static void afs_kill_lockers_enoent(struct afs_vnode *vnode)
+{
+	struct file_lock *p;
+
+	afs_set_lock_state(vnode, AFS_VNODE_LOCK_DELETED);
+
+	while (!list_empty(&vnode->pending_locks)) {
+		p = list_entry(vnode->pending_locks.next,
+			       struct file_lock, fl_u.afs.link);
+		list_del_init(&p->fl_u.afs.link);
+		p->fl_u.afs.state = -ENOENT;
+		wake_up(&p->fl_wait);
+	}
+
+	key_put(vnode->lock_key);
+	vnode->lock_key = NULL;
+}
+
 /*
  * Get a lock on a file
  */
@@ -278,13 +300,19 @@ void afs_lock_work(struct work_struct *work)
 		/* attempt to release the server lock; if it fails, we just
 		 * wait 5 minutes and it'll expire anyway */
 		ret = afs_release_lock(vnode, vnode->lock_key);
-		if (ret < 0)
+		if (ret < 0) {
+			trace_afs_flock_ev(vnode, NULL, afs_flock_release_fail,
+					   ret);
 			printk(KERN_WARNING "AFS:"
 			       " Failed to release lock on {%llx:%llx} error %d\n",
 			       vnode->fid.vid, vnode->fid.vnode, ret);
+		}
 
 		spin_lock(&vnode->lock);
-		afs_next_locker(vnode, 0);
+		if (ret == -ENOENT)
+			afs_kill_lockers_enoent(vnode);
+		else
+			afs_next_locker(vnode, 0);
 		spin_unlock(&vnode->lock);
 		return;
 
@@ -304,12 +332,21 @@ void afs_lock_work(struct work_struct *work)
 		ret = afs_extend_lock(vnode, key); /* RPC */
 		key_put(key);
 
-		if (ret < 0)
+		if (ret < 0) {
+			trace_afs_flock_ev(vnode, NULL, afs_flock_extend_fail,
+					   ret);
 			pr_warning("AFS: Failed to extend lock on {%llx:%llx} error %d\n",
 				   vnode->fid.vid, vnode->fid.vnode, ret);
+		}
 
 		spin_lock(&vnode->lock);
 
+		if (ret == -ENOENT) {
+			afs_kill_lockers_enoent(vnode);
+			spin_unlock(&vnode->lock);
+			return;
+		}
+
 		if (vnode->lock_state != AFS_VNODE_LOCK_EXTENDING)
 			goto again;
 		afs_set_lock_state(vnode, AFS_VNODE_LOCK_GRANTED);
@@ -333,6 +370,11 @@ void afs_lock_work(struct work_struct *work)
 		spin_unlock(&vnode->lock);
 		return;
 
+	case AFS_VNODE_LOCK_DELETED:
+		afs_kill_lockers_enoent(vnode);
+		spin_unlock(&vnode->lock);
+		return;
+
 	default:
 		/* Looks like a lock request was withdrawn. */
 		spin_unlock(&vnode->lock);
@@ -435,6 +477,10 @@ static int afs_do_setlk(struct file *file, struct file_lock *fl)
 	spin_lock(&vnode->lock);
 	list_add_tail(&fl->fl_u.afs.link, &vnode->pending_locks);
 
+	ret = -ENOENT;
+	if (vnode->lock_state == AFS_VNODE_LOCK_DELETED)
+		goto error_unlock;
+
 	/* If we've already got a lock on the server then try to move to having
 	 * the VFS grant the requested lock.  Note that this means that other
 	 * clients may get starved out.
@@ -489,6 +535,13 @@ static int afs_do_setlk(struct file *file, struct file_lock *fl)
 		afs_next_locker(vnode, ret);
 		goto error_unlock;
 
+	case -ENOENT:
+		fl->fl_u.afs.state = ret;
+		trace_afs_flock_ev(vnode, fl, afs_flock_fail_other, ret);
+		list_del_init(&fl->fl_u.afs.link);
+		afs_kill_lockers_enoent(vnode);
+		goto error_unlock;
+
 	default:
 		fl->fl_u.afs.state = ret;
 		trace_afs_flock_ev(vnode, fl, afs_flock_fail_other, ret);
@@ -638,6 +691,9 @@ static int afs_do_getlk(struct file *file, struct file_lock *fl)
 
 	_enter("");
 
+	if (vnode->lock_state == AFS_VNODE_LOCK_DELETED)
+		return -ENOENT;
+
 	fl->fl_type = F_UNLCK;
 
 	/* check local lock records first */
diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index 37a5ba550846..d6763e59952d 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -600,6 +600,7 @@ enum afs_lock_state {
 	AFS_VNODE_LOCK_EXTENDING,	/* We're extending a lock on the server */
 	AFS_VNODE_LOCK_NEED_UNLOCK,	/* We need to unlock on the server */
 	AFS_VNODE_LOCK_UNLOCKING,	/* We're telling the server to unlock */
+	AFS_VNODE_LOCK_DELETED,		/* The vnode has been deleted whilst we have a lock */
 };
 
 /*
diff --git a/include/trace/events/afs.h b/include/trace/events/afs.h
index 24c058a93e8f..21b896fabb2f 100644
--- a/include/trace/events/afs.h
+++ b/include/trace/events/afs.h
@@ -156,9 +156,11 @@ enum afs_flock_event {
 	afs_flock_acquired,
 	afs_flock_callback_break,
 	afs_flock_defer_unlock,
+	afs_flock_extend_fail,
 	afs_flock_fail_other,
 	afs_flock_fail_perm,
 	afs_flock_no_lockers,
+	afs_flock_release_fail,
 	afs_flock_timestamp,
 	afs_flock_try_to_lock,
 	afs_flock_vfs_lock,
@@ -323,15 +325,18 @@ enum afs_flock_operation {
 	EM(AFS_VNODE_LOCK_GRANTED,		"GRANTED")		\
 	EM(AFS_VNODE_LOCK_EXTENDING,		"EXTENDING")		\
 	EM(AFS_VNODE_LOCK_NEED_UNLOCK,		"NEED_UNLOCK")		\
-	E_(AFS_VNODE_LOCK_UNLOCKING,		"UNLOCKING")		\
+	EM(AFS_VNODE_LOCK_UNLOCKING,		"UNLOCKING")		\
+	E_(AFS_VNODE_LOCK_DELETED,		"DELETED")
 
 #define afs_flock_events						\
 	EM(afs_flock_acquired,			"Acquired")		\
 	EM(afs_flock_callback_break,		"Callback")		\
 	EM(afs_flock_defer_unlock,		"D-Unlock")		\
+	EM(afs_flock_extend_fail,		"Ext_Fail")		\
 	EM(afs_flock_fail_other,		"ErrOther")		\
 	EM(afs_flock_fail_perm,			"ErrPerm ")		\
 	EM(afs_flock_no_lockers,		"NoLocker")		\
+	EM(afs_flock_release_fail,		"Rel_Fail")		\
 	EM(afs_flock_timestamp,			"Timestmp")		\
 	EM(afs_flock_try_to_lock,		"TryToLck")		\
 	EM(afs_flock_vfs_lock,			"VFSLock ")		\


  parent reply	other threads:[~2019-03-15 23:09 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-15 22:59 [PATCH 00/10] AFS fixes David Howells
2019-03-15 22:59 ` [PATCH 01/10] afs: Split wait from afs_make_call() David Howells
2019-03-15 22:59 ` [PATCH 02/10] afs: Calculate lock extend timer from set/extend reply reception David Howells
2019-03-15 22:59 ` [PATCH 03/10] afs: Fix AFS file locking to allow fine grained locks David Howells
2019-03-15 22:59 ` [PATCH 04/10] afs: Further fix file locking David Howells
2019-03-15 22:59 ` [PATCH 05/10] afs: Add file locking tracepoints David Howells
2019-03-15 23:00 ` [PATCH 06/10] afs: Improve dir check failure reports David Howells
2019-03-15 23:00 ` David Howells [this message]
2019-03-15 23:00 ` [PATCH 08/10] afs: Add directory reload tracepoint David Howells
2019-03-15 23:00 ` [PATCH 09/10] afs: Implement sillyrename for unlink and rename David Howells
2019-03-15 23:00 ` [PATCH 10/10] afs: Add more tracepoints David Howells

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=155269081456.8537.5454244654980801039.stgit@warthog.procyon.org.uk \
    --to=dhowells@redhat.com \
    --cc=jsbillin@umich.edu \
    --cc=linux-afs@lists.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.