[PATCH 1/5] fs: make a mount waiting on another mount killable

[Josef Bacik]

A superblock is put on the lists by sget_fc() and becomes usable when
vfs_get_tree() sets SB_BORN, after the filesystem has read what it needs
from its device or its server.  If that doesn't answer, this takes forever.

Somebody else mounting the same thing finds that superblock in sget_fc()
and waits in grab_super() for it to be born, to share it, or to die, to
make its own.  It has to.  But it does so in super_lock(), which can't be
killed, so one stuck mount collects every later attempt.

mount(2) and fsconfig(2) can fail at this point, as they already do when
they are killed while waiting for a superblock to die.

Add super_lock_killable(), in which both the wait for SB_BORN and taking
s_umount can be killed, and use it in grab_super().  It tells a superblock
which is going away from a wait which was cut short, so it returns an int:
0 with the lock held, -ENOENT, or the error from the wait.

Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
---
 fs/super.c | 114 ++++++++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 82 insertions(+), 32 deletions(-)

diff --git a/fs/super.c b/fs/super.c
index 36fdf88ef88e..7164c92fe70e 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -62,6 +62,13 @@ static inline void __super_lock(struct super_block *sb, bool excl)
 		down_read(&sb->s_umount);
 }
 
+static inline int __super_lock_killable(struct super_block *sb, bool excl)
+{
+	if (excl)
+		return down_write_killable(&sb->s_umount);
+	return down_read_killable(&sb->s_umount);
+}
+
 static inline void super_unlock(struct super_block *sb, bool excl)
 {
 	if (excl)
@@ -94,28 +101,14 @@ static bool super_flags(const struct super_block *sb, unsigned int flags)
 	return smp_load_acquire(&sb->s_flags) & flags;
 }
 
-/**
- * super_lock - wait for superblock to become ready and lock it
- * @sb: superblock to wait for
- * @excl: whether exclusive access is required
- *
- * If the superblock has neither passed through vfs_get_tree() or
- * generic_shutdown_super() yet wait for it to happen. Either superblock
- * creation will succeed and SB_BORN is set by vfs_get_tree() or we're
- * woken and we'll see SB_DYING.
- *
- * A superblock which is being shut down is not waited for, see
- * super_wait_shutdown() and super_wait_shutdown_killable() for that.
- *
- * The caller must have acquired a temporary reference on @sb->s_passive.
- *
- * Return: The function returns true if SB_BORN was set and with
- *         s_umount held. The function returns false if the superblock
- *         is being shut down or SB_DYING was set and without s_umount
- *         held.
+/*
+ * Returns 0 with s_umount held, -ENOENT if the superblock is going away, or,
+ * if @killable, the error from a wait which was cut short.
  */
-static __must_check bool super_lock(struct super_block *sb, bool excl)
+static int super_lock_common(struct super_block *sb, bool excl, bool killable)
 {
+	int ret = 0;
+
 	lockdep_assert_not_held(&sb->s_umount);
 
 	/*
@@ -125,16 +118,28 @@ static __must_check bool super_lock(struct super_block *sb, bool excl)
 	 * and it can be a long one: the filesystem is written out first.
 	 */
 	if (!atomic_read(&sb->s_active))
-		return false;
+		return -ENOENT;
 
 	/* wait until the superblock is ready or dying */
-	wait_var_event(&sb->s_flags, super_flags(sb, SB_BORN | SB_DYING));
+	if (killable)
+		ret = wait_var_event_killable(&sb->s_flags,
+					      super_flags(sb, SB_BORN | SB_DYING));
+	else
+		wait_var_event(&sb->s_flags,
+			       super_flags(sb, SB_BORN | SB_DYING));
+	if (ret)
+		return ret;
 
 	/* Don't pointlessly acquire s_umount. */
 	if (super_flags(sb, SB_DYING))
-		return false;
+		return -ENOENT;
 
-	__super_lock(sb, excl);
+	if (killable)
+		ret = __super_lock_killable(sb, excl);
+	else
+		__super_lock(sb, excl);
+	if (ret)
+		return ret;
 
 	/*
 	 * Has gone through generic_shutdown_super() in the meantime.
@@ -143,11 +148,54 @@ static __must_check bool super_lock(struct super_block *sb, bool excl)
 	 */
 	if (sb->s_flags & SB_DYING) {
 		super_unlock(sb, excl);
-		return false;
+		return -ENOENT;
 	}
 
 	WARN_ON_ONCE(!(sb->s_flags & SB_BORN));
-	return true;
+	return 0;
+}
+
+/**
+ * super_lock - wait for superblock to become ready and lock it
+ * @sb: superblock to wait for
+ * @excl: whether exclusive access is required
+ *
+ * If the superblock has neither passed through vfs_get_tree() or
+ * generic_shutdown_super() yet wait for it to happen. Either superblock
+ * creation will succeed and SB_BORN is set by vfs_get_tree() or we're
+ * woken and we'll see SB_DYING.
+ *
+ * A superblock which is being shut down is not waited for, see
+ * super_wait_shutdown() and super_wait_shutdown_killable() for that.
+ *
+ * The caller must have acquired a temporary reference on @sb->s_passive.
+ *
+ * Return: The function returns true if SB_BORN was set and with
+ *         s_umount held. The function returns false if the superblock
+ *         is being shut down or SB_DYING was set and without s_umount
+ *         held.
+ */
+static __must_check bool super_lock(struct super_block *sb, bool excl)
+{
+	return !super_lock_common(sb, excl, false);
+}
+
+/**
+ * super_lock_killable - killable super_lock()
+ * @sb: superblock to wait for
+ * @excl: whether exclusive access is required
+ *
+ * Only for callers which give up altogether when they are killed.
+ *
+ * The caller must have acquired a temporary reference on @sb->s_passive.
+ *
+ * Return: 0 if SB_BORN was set and with s_umount held. Without s_umount held,
+ *         -ENOENT if the superblock is being shut down or SB_DYING was set,
+ *         -EINTR or -ERESTARTSYS if the task was killed.
+ */
+static __must_check int super_lock_killable(struct super_block *sb, bool excl)
+{
+	return super_lock_common(sb, excl, true);
 }
 
 /**
@@ -674,25 +722,27 @@ EXPORT_SYMBOL(deactivate_super);
  * sb->kill() and be marked as SB_DEAD.
  *
  * Return: 0 if an active reference could be acquired, -EAGAIN if the
- *         superblock is gone and the caller should look again,
- *         -ERESTARTSYS if the task was killed while waiting for that.
+ *         superblock is gone and the caller should look again, -EINTR or
+ *         -ERESTARTSYS if the task was killed while waiting for either.
  */
 static int grab_super(struct super_block *sb)
 {
-	bool locked;
 	int ret;
 
 	refcount_inc(&sb->s_passive);
 	spin_unlock(&sb_lock);
-	locked = super_lock_excl(sb);
-	if (locked) {
+	ret = super_lock_killable(sb, true);
+	if (!ret) {
 		if (atomic_inc_not_zero(&sb->s_active)) {
 			put_super(sb);
 			return 0;
 		}
 		super_unlock_excl(sb);
+		ret = -ENOENT;
 	}
-	ret = wait_var_event_killable(&sb->s_flags, super_flags(sb, SB_DEAD));
+	if (ret == -ENOENT)
+		ret = wait_var_event_killable(&sb->s_flags,
+					      super_flags(sb, SB_DEAD));
 	put_super(sb);
 	return ret ? ret : -EAGAIN;
 }

-- 
2.55.0