[PATCH 4/5] super: make sync(2)'s wait for s_umount killable

[Christian Brauner <brauner@kernel.org>]

iterate_supers_sync() waits killably for a superblock that's being shut
down, but it takes s_umount of a live superblock uninterruptibly. So a
freeze_super() or remount that's stuck under s_umount, in
sync_filesystem() or ->freeze_fs() say, still hangs every sync(2) on
the machine without anyone being able to kill it.

That's inconsistent with fs_bdev_sync() which already uses a killable
super_lock() for both waits. And the reasoning is the same: if the task
is killed the walk stops and ksys_sync() returns, there's nobody left to
make a promise to. The emergency sync and the sync before suspend and
hibernation run from a workqueue where that can't happen. The PROM sync
on sparc32 runs in whatever task it interrupted but that one could
already be cut short while it waited for a superblock being shut down.

Let __iterate_supers() pass SUPER_LOCK_KILLABLE together with
SUPER_LOCK_WAIT_SHUTDOWN for SUPER_ITER_WAIT_SHUTDOWN. Only
iterate_supers_sync() passes that flag. The wait for SB_BORN doesn't
matter for sync(2) as iterate_supers_sync() skips unborn superblocks
anyway.

This can be reproduced by freezing a filesystem on a suspended dm-linear
device and calling sync(2) behind the freeze:

  truncate -s 512M img
  DEV=$(losetup -f --show img)
  mke2fs -q $DEV
  dmsetup create t --table "0 $(blockdev --getsz $DEV) linear $DEV 0"
  mount -t ext2 /dev/mapper/t /mnt
  dd if=/dev/urandom of=/mnt/f bs=1M count=64
  dmsetup suspend --nolockfs t
  fsfreeze --freeze /mnt &
  sleep 3
  sync &
  sleep 3
  kill -9 $!

fsfreeze sits in sync_filesystem() with s_umount held for writing.
Without this patch the sync stays in down_read() after the SIGKILL. With
it the sync goes away.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/super.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/fs/super.c b/fs/super.c
index 0f3308fb99aa..a0f9eadf0da7 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1010,9 +1010,13 @@ static inline struct super_block *next_super(struct super_block *sb,
 static int __iterate_supers(void (*f)(struct super_block *, void *), void *arg,
 			    enum super_iter_flags_t flags)
 {
+	enum super_lock_flags_t lock_flags = 0;
 	struct super_block *sb, *p = NULL;
 	int ret = 0;
 
+	if (flags & SUPER_ITER_WAIT_SHUTDOWN)
+		lock_flags |= SUPER_LOCK_KILLABLE | SUPER_LOCK_WAIT_SHUTDOWN;
+
 	spin_lock(&sb_lock);
 
 	for (sb = first_super(flags);
@@ -1032,12 +1036,14 @@ static int __iterate_supers(void (*f)(struct super_block *, void *), void *arg,
 
 		if (flags & SUPER_ITER_UNLOCKED) {
 			f(sb, arg);
-		} else if (!super_lock(sb, 0)) {
-			f(sb, arg);
-			super_unlock_shared(sb);
-		} else if (flags & SUPER_ITER_WAIT_SHUTDOWN) {
-			/* whoever shuts it down writes it out, wait for that */
-			ret = super_wait(sb, SB_DYING, TASK_KILLABLE);
+		} else {
+			ret = super_lock(sb, lock_flags);
+			if (!ret) {
+				f(sb, arg);
+				super_unlock_shared(sb);
+			} else if (ret == -ENOENT) {
+				ret = 0;
+			}
 		}
 
 		if (p)
@@ -1085,8 +1091,8 @@ void iterate_supers_born(void (*f)(struct super_block *, void *), void *arg)
  * A superblock which is still being set up isn't passed to @f either, and
  * isn't waited for.  Nobody can have written to it yet.
  *
- * Return: 0 if all the superblocks were seen, -ERESTARTSYS if the task was
- *         killed while it waited for one.
+ * Return: 0 if all the superblocks were seen, -EINTR or -ERESTARTSYS if the
+ *         task was killed while it waited for one.
  */
 int iterate_supers_sync(void (*f)(struct super_block *, void *), void *arg)
 {

-- 
2.53.0