Everyone who needs an unmount to be over before they can carry on currently calls super_lock() and, if that fails, calls super_wait_shutdown() or super_wait_shutdown_killable() after it. That's six callers open-coding the same two step dance, and they have to make sure to pick the shutdown wait that matches the lock they took. Add SUPER_LOCK_WAIT_SHUTDOWN which makes super_lock() wait for SB_DYING when it finds the superblock going away. It still returns -ENOENT after the wait. It sleeps in the same state as the rest of super_lock(), so SUPER_LOCK_KILLABLE now covers all of the waiting. get_active_super() passes the flags through for the freezing callers. Then remove super_wait_shutdown() and super_wait_shutdown_killable(). __iterate_supers() is the odd one out. It takes s_umount uninterruptibly but waits for the shutdown killably. Keep it that way for now and let it call super_wait() directly. Note that the freezing callers used to wait for the shutdown even if super_lock() succeeded and atomic_inc_not_zero() in get_active_super() failed. That can't happen. The last active reference is dropped with s_umount held and s_umount stays held until SB_DYING is set. So once super_lock() succeeded there's always an active reference to take. No functional changes. Signed-off-by: Christian Brauner (Amutable) --- fs/super.c | 118 +++++++++++++++++++++++-------------------------------------- 1 file changed, 45 insertions(+), 73 deletions(-) diff --git a/fs/super.c b/fs/super.c index 0185c1f16ba3..0f3308fb99aa 100644 --- a/fs/super.c +++ b/fs/super.c @@ -58,6 +58,7 @@ static char *sb_writers_name[SB_FREEZE_LEVELS] = { enum super_lock_flags_t { SUPER_LOCK_EXCL = (1U << 0), SUPER_LOCK_KILLABLE = (1U << 1), + SUPER_LOCK_WAIT_SHUTDOWN = (1U << 2), }; static inline int __super_lock(struct super_block *sb, @@ -130,13 +131,19 @@ static int super_wait(struct super_block *sb, unsigned int sb_flags, * 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. - * * With SUPER_LOCK_EXCL s_umount is taken for writing, otherwise for reading. - * With SUPER_LOCK_KILLABLE both the wait and taking s_umount can be cut short - * by a fatal signal. That is only for callers which give up altogether when - * they are killed. + * + * A superblock which is being shut down is not waited for unless + * SUPER_LOCK_WAIT_SHUTDOWN is passed. That is for those who have nothing to + * do on a superblock which is going away, but who promise their caller that + * what it had to write has been written, or who must not carry on while the + * filesystem is still being written out. The wait is over once + * generic_shutdown_super() is done with @sb, and -ENOENT is still returned. + * + * With SUPER_LOCK_KILLABLE all the waiting can be cut short by a fatal + * signal. That is only for callers which give up altogether when they are + * killed. If what comes after the wait happens either way, the wait must + * not be cut short. * * The caller must have acquired a temporary reference on @sb->s_passive. * @@ -162,7 +169,7 @@ static __must_check int super_lock(struct super_block *sb, * and it can be a long one: the filesystem is written out first. */ if (!atomic_read(&sb->s_active)) - return -ENOENT; + goto dying; /* wait until the superblock is ready or dying */ ret = super_wait(sb, SB_BORN | SB_DYING, state); @@ -171,7 +178,7 @@ static __must_check int super_lock(struct super_block *sb, /* Don't pointlessly acquire s_umount. */ if (super_flags(sb, SB_DYING)) - return -ENOENT; + goto dying; ret = __super_lock(sb, flags); if (ret) @@ -184,45 +191,19 @@ static __must_check int super_lock(struct super_block *sb, */ if (sb->s_flags & SB_DYING) { super_unlock(sb, flags); - return -ENOENT; + goto dying; } WARN_ON_ONCE(!(sb->s_flags & SB_BORN)); return 0; -} -/** - * super_wait_shutdown_killable - wait for a superblock being shut down - * @sb: superblock super_lock() failed on - * - * For those who have nothing to do on a superblock which is going away, but - * who promise their caller that what it had to write has been written. - * - * Only for callers which give up altogether when they are killed. If what - * comes after the wait happens either way, the wait must not be cut short. - * - * The caller must have acquired a temporary reference on @sb->s_passive. - * - * Return: 0 once generic_shutdown_super() is done with @sb, -ERESTARTSYS if - * the task was killed first. - */ -static int super_wait_shutdown_killable(struct super_block *sb) -{ - return super_wait(sb, SB_DYING, TASK_KILLABLE); -} - -/** - * super_wait_shutdown - wait for a superblock being shut down - * @sb: superblock super_lock() failed on - * - * For callers of which the rest happens whether they are killed or not, and - * must not happen while the filesystem is still being written out. - * - * The caller must have acquired a temporary reference on @sb->s_passive. - */ -static void super_wait_shutdown(struct super_block *sb) -{ - super_wait(sb, SB_DYING, TASK_UNINTERRUPTIBLE); +dying: + if (flags & SUPER_LOCK_WAIT_SHUTDOWN) { + ret = super_wait(sb, SB_DYING, state); + if (ret) + return ret; + } + return -ENOENT; } /* wake waiters */ @@ -1055,7 +1036,8 @@ static int __iterate_supers(void (*f)(struct super_block *, void *), void *arg, f(sb, arg); super_unlock_shared(sb); } else if (flags & SUPER_ITER_WAIT_SHUTDOWN) { - ret = super_wait_shutdown_killable(sb); + /* whoever shuts it down writes it out, wait for that */ + ret = super_wait(sb, SB_DYING, TASK_KILLABLE); } if (p) @@ -1285,11 +1267,9 @@ static void do_emergency_remount_callback(struct super_block *sb, void *unused) { struct fs_context *fc = NULL; - if (super_lock(sb, SUPER_LOCK_EXCL)) { - /* the unmount writes the filesystem out, wait for that */ - super_wait_shutdown(sb); + /* the unmount writes the filesystem out, wait for that */ + if (super_lock(sb, SUPER_LOCK_EXCL | SUPER_LOCK_WAIT_SHUTDOWN)) return; - } /* Make sure no reference put under s_umount is the last one. */ atomic_inc(&sb->s_active); @@ -1329,11 +1309,12 @@ void emergency_remount(void) } } -static inline bool get_active_super(struct super_block *sb) +static inline bool get_active_super(struct super_block *sb, + enum super_lock_flags_t flags) { bool active = false; - if (!super_lock(sb, SUPER_LOCK_EXCL)) { + if (!super_lock(sb, SUPER_LOCK_EXCL | flags)) { active = atomic_inc_not_zero(&sb->s_active); super_unlock_excl(sb); } @@ -1342,7 +1323,7 @@ static inline bool get_active_super(struct super_block *sb) static void do_thaw_all_callback(struct super_block *sb, void *unused) { - if (!get_active_super(sb)) + if (!get_active_super(sb, 0)) return; /* fs_bdev_thaw() acquires s_umount so it must not be held here */ @@ -1388,11 +1369,9 @@ static void filesystems_freeze_callback(struct super_block *sb, void *freeze_all if (!freeze_all_ptr && !(sb->s_type->fs_flags & FS_POWER_FREEZE)) return; - if (!get_active_super(sb)) { - /* an unmount from a workqueue isn't stopped by the freezer */ - super_wait_shutdown(sb); + /* an unmount from a workqueue isn't stopped by the freezer */ + if (!get_active_super(sb, SUPER_LOCK_WAIT_SHUTDOWN)) return; - } if (sb->s_op->freeze_super) sb->s_op->freeze_super(sb, FREEZE_EXCL | FREEZE_HOLDER_KERNEL, @@ -1419,7 +1398,7 @@ static void filesystems_thaw_callback(struct super_block *sb, void *unused) if (!sb->s_op->freeze_fs && !sb->s_op->freeze_super) return; - if (!get_active_super(sb)) + if (!get_active_super(sb, 0)) return; if (sb->s_op->thaw_super) @@ -1660,12 +1639,9 @@ static void fs_bdev_mark_dead(struct block_device *bdev, bool surprise) mutex_unlock(&bdev->bd_holder_lock); for_each_dev_super(sb, &cursor) { - if (super_lock(sb, 0)) { - /* don't take the device away from under an unmount */ - if (!surprise) - super_wait_shutdown(sb); + /* don't take the device away from under an unmount */ + if (super_lock(sb, surprise ? 0 : SUPER_LOCK_WAIT_SHUTDOWN)) continue; - } if (sb->s_root && (sb->s_flags & SB_ACTIVE)) { if (!sb->s_op->remove_bdev || sb->s_op->remove_bdev(sb, bdev)) { @@ -1691,20 +1667,18 @@ static void fs_bdev_sync(struct block_device *bdev) for_each_dev_super_killable(sb, &cursor) { int ret; - ret = super_lock(sb, SUPER_LOCK_KILLABLE); - if (!ret) { - if (sb->s_root && (sb->s_flags & SB_ACTIVE)) - sync_filesystem(sb); - super_unlock_shared(sb); - continue; - } + ret = super_lock(sb, SUPER_LOCK_KILLABLE | + SUPER_LOCK_WAIT_SHUTDOWN); if (ret == -ENOENT) - ret = super_wait_shutdown_killable(sb); + continue; if (ret) { super_dev_walk_end(&cursor); put_super(sb); return; } + if (sb->s_root && (sb->s_flags & SB_ACTIVE)) + sync_filesystem(sb); + super_unlock_shared(sb); } } @@ -1733,11 +1707,9 @@ static int fs_bdev_freeze(struct block_device *bdev) mutex_unlock(&bdev->bd_holder_lock); for_each_dev_super(sb, &cursor) { - if (!get_active_super(sb)) { - /* an unmount writes to the device until it is done */ - super_wait_shutdown(sb); + /* an unmount writes to the device until it is done */ + if (!get_active_super(sb, SUPER_LOCK_WAIT_SHUTDOWN)) continue; - } err = fs_super_freeze(sb); if (err && !error) error = err; @@ -1791,7 +1763,7 @@ static int fs_bdev_thaw(struct block_device *bdev) mutex_unlock(&bdev->bd_holder_lock); for_each_dev_super(sb, &cursor) { - if (!get_active_super(sb)) + if (!get_active_super(sb, 0)) continue; err = fs_super_thaw(sb); if (err && !error) -- 2.53.0