* [PATCH 0/5] super: rework super_lock()
@ 2026-10-23 9:41 Christian Brauner
2026-10-23 9:41 ` [PATCH 1/5] super: add super_wait() Christian Brauner
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Christian Brauner @ 2026-10-23 9:41 UTC (permalink / raw)
To: Jan Kara
Cc: Alexander Viro, Josef Bacik, linux-fsdevel, linux-kernel,
Christian Brauner (Amutable)
Josef's two series (cf. [1], [2]) left us with four ways to lock a
superblock and two ways to wait for one that's being shut down.
super_lock(), super_lock_shared() and super_lock_excl() return true on
success while super_lock_killable() returns 0. And callers that need an
unmount to be over pair super_lock() with super_wait_shutdown() or
super_wait_shutdown_killable() and have to pick the one that matches
the lock they took. Jan pointed out the bool vs int mess (cf. [3]).
So fold it all into a single super_lock() that takes SUPER_LOCK_EXCL,
SUPER_LOCK_KILLABLE and SUPER_LOCK_WAIT_SHUTDOWN and returns 0 with
s_umount held or a negative error code. The first three patches don't
change behavior.
The fourth patch makes sync(2) killable while it waits for s_umount of
a live superblock. Right now a freeze or remount that's stuck under
s_umount hangs every sync(2) on the machine and nobody can kill it.
fs_bdev_sync() already works that way.
The last patch adds a guard for super_lock() for the callers that take
and drop s_umount in the same scope.
This sits on top of vfs-7.5.super which carries Josef's series.
[1]: https://lore.kernel.org/20261021-b4-super-lock-dying-v1-0-2b35e86706ff@toxicpanda.com
[2]: https://lore.kernel.org/20261021-b4-super-lock-unborn-v1-0-0265eddb6dee@toxicpanda.com
[3]: https://lore.kernel.org/tmw22jj2hn6g6lklme2j3rsfjvmstzpoicvibvmmv6sivd66oj@nhc4lng25gti
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
Christian Brauner (5):
super: add super_wait()
super: let super_lock() take flags
super: let super_lock() wait for a superblock being shut down
super: make sync(2)'s wait for s_umount killable
super: add a guard for super_lock()
fs/super.c | 340 ++++++++++++++++++++++++++++---------------------------------
1 file changed, 156 insertions(+), 184 deletions(-)
---
base-commit: e6d8f4efc8e2301e15de7bcf9e8d3a2134fb058a
change-id: 20261023-work-super-lock_helpers-23d8d257c1b4
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/5] super: add super_wait()
2026-10-23 9:41 [PATCH 0/5] super: rework super_lock() Christian Brauner
@ 2026-10-23 9:41 ` Christian Brauner
2026-10-23 9:41 ` [PATCH 2/5] super: let super_lock() take flags Christian Brauner
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Christian Brauner @ 2026-10-23 9:41 UTC (permalink / raw)
To: Jan Kara
Cc: Alexander Viro, Josef Bacik, linux-fsdevel, linux-kernel,
Christian Brauner (Amutable)
We wait for SB_BORN, SB_DYING and SB_DEAD in five places now, each with
an open-coded wait_var_event(), wait_var_event_killable() or
wait_var_event_state() on sb->s_flags and super_lock_common() picking
between the first two depending on @killable. Add super_wait() which
takes the flags to wait for and the task state to sleep in and use it
everywhere.
No functional changes.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
fs/super.c | 30 +++++++++++++++++-------------
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/fs/super.c b/fs/super.c
index 63e85794ea4d..16b9d7ac381c 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -102,6 +102,17 @@ static bool super_flags(const struct super_block *sb, unsigned int flags)
return smp_load_acquire(&sb->s_flags) & flags;
}
+/*
+ * Wait for one of @sb_flags to be set on @sb, sleeping in @state. Returns 0
+ * or -ERESTARTSYS if @state allowed the wait to be cut short.
+ */
+static int super_wait(struct super_block *sb, unsigned int sb_flags,
+ unsigned int state)
+{
+ return wait_var_event_state(&sb->s_flags, super_flags(sb, sb_flags),
+ state);
+}
+
/*
* 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.
@@ -122,12 +133,8 @@ static int super_lock_common(struct super_block *sb, bool excl, bool killable)
return -ENOENT;
/* wait until the superblock is ready or 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));
+ ret = super_wait(sb, SB_BORN | SB_DYING,
+ killable ? TASK_KILLABLE : TASK_UNINTERRUPTIBLE);
if (ret)
return ret;
@@ -216,7 +223,7 @@ static __must_check int super_lock_killable(struct super_block *sb, bool excl)
*/
static int super_wait_shutdown_killable(struct super_block *sb)
{
- return wait_var_event_killable(&sb->s_flags, super_flags(sb, SB_DYING));
+ return super_wait(sb, SB_DYING, TASK_KILLABLE);
}
/**
@@ -230,7 +237,7 @@ static int super_wait_shutdown_killable(struct super_block *sb)
*/
static void super_wait_shutdown(struct super_block *sb)
{
- wait_var_event(&sb->s_flags, super_flags(sb, SB_DYING));
+ super_wait(sb, SB_DYING, TASK_UNINTERRUPTIBLE);
}
/* wait and try to acquire read-side of @sb->s_umount */
@@ -606,9 +613,7 @@ static struct super_block *super_dev_next(struct super_dev *cursor,
return sb;
}
}
- ret = wait_var_event_state(&sb->s_flags,
- super_flags(sb, SB_BORN | SB_DYING),
- state);
+ ret = super_wait(sb, SB_BORN | SB_DYING, state);
put_super(sb);
if (ret) {
super_dev_walk_end(cursor);
@@ -731,8 +736,7 @@ static int grab_super(struct super_block *sb)
ret = -ENOENT;
}
if (ret == -ENOENT)
- ret = wait_var_event_killable(&sb->s_flags,
- super_flags(sb, SB_DEAD));
+ ret = super_wait(sb, SB_DEAD, TASK_KILLABLE);
put_super(sb);
return ret ? ret : -EAGAIN;
}
--
2.53.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/5] super: let super_lock() take flags
2026-10-23 9:41 [PATCH 0/5] super: rework super_lock() Christian Brauner
2026-10-23 9:41 ` [PATCH 1/5] super: add super_wait() Christian Brauner
@ 2026-10-23 9:41 ` Christian Brauner
2026-10-23 9:41 ` [PATCH 3/5] super: let super_lock() wait for a superblock being shut down Christian Brauner
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Christian Brauner @ 2026-10-23 9:41 UTC (permalink / raw)
To: Jan Kara
Cc: Alexander Viro, Josef Bacik, linux-fsdevel, linux-kernel,
Christian Brauner (Amutable)
We have super_lock() returning a bool, super_lock_killable() returning
an int and both forwarding to super_lock_common() which takes two bools.
On top of that super_lock_shared() and super_lock_excl() return true on
success while super_lock_killable() returns 0 on success. So whether
"if (super_lock*(sb))" means we got the lock depends on which one is
called. And the next patch wants super_lock() to optionally wait for a
superblock that's being shut down.
So instead of growing yet another variant, add SUPER_LOCK_EXCL and
SUPER_LOCK_KILLABLE and pass them all the way down. __super_lock() and
super_unlock() take the flags instead of a bool, which also gets rid of
__super_lock_killable(). super_lock() takes the flags and returns 0,
-ENOENT or the error from the wait. Then remove super_lock_killable(),
super_lock_common(), super_lock_shared() and super_lock_excl() and
convert everyone to super_lock().
No functional changes.
Suggested-by: Jan Kara <jack@suse.cz>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
fs/super.c | 163 +++++++++++++++++++++++++------------------------------------
1 file changed, 67 insertions(+), 96 deletions(-)
diff --git a/fs/super.c b/fs/super.c
index 16b9d7ac381c..0185c1f16ba3 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -55,24 +55,31 @@ static char *sb_writers_name[SB_FREEZE_LEVELS] = {
"sb_internal",
};
-static inline void __super_lock(struct super_block *sb, bool excl)
+enum super_lock_flags_t {
+ SUPER_LOCK_EXCL = (1U << 0),
+ SUPER_LOCK_KILLABLE = (1U << 1),
+};
+
+static inline int __super_lock(struct super_block *sb,
+ enum super_lock_flags_t flags)
{
- if (excl)
+ if (flags & SUPER_LOCK_KILLABLE) {
+ if (flags & SUPER_LOCK_EXCL)
+ return down_write_killable(&sb->s_umount);
+ return down_read_killable(&sb->s_umount);
+ }
+
+ if (flags & SUPER_LOCK_EXCL)
down_write(&sb->s_umount);
else
down_read(&sb->s_umount);
+ return 0;
}
-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)
+static inline void super_unlock(struct super_block *sb,
+ enum super_lock_flags_t flags)
{
- if (excl)
+ if (flags & SUPER_LOCK_EXCL)
up_write(&sb->s_umount);
else
up_read(&sb->s_umount);
@@ -80,17 +87,17 @@ static inline void super_unlock(struct super_block *sb, bool excl)
static inline void __super_lock_excl(struct super_block *sb)
{
- __super_lock(sb, true);
+ __super_lock(sb, SUPER_LOCK_EXCL);
}
static inline void super_unlock_excl(struct super_block *sb)
{
- super_unlock(sb, true);
+ super_unlock(sb, SUPER_LOCK_EXCL);
}
static inline void super_unlock_shared(struct super_block *sb)
{
- super_unlock(sb, false);
+ super_unlock(sb, 0);
}
static bool super_flags(const struct super_block *sb, unsigned int flags)
@@ -113,13 +120,38 @@ static int super_wait(struct super_block *sb, unsigned int sb_flags,
state);
}
-/*
- * 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.
+/**
+ * super_lock - wait for superblock to become ready and lock it
+ * @sb: superblock to wait for
+ * @flags: SUPER_LOCK_* flags
+ *
+ * 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.
+ *
+ * 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.
+ *
+ * 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 int super_lock_common(struct super_block *sb, bool excl, bool killable)
+static __must_check int super_lock(struct super_block *sb,
+ enum super_lock_flags_t flags)
{
- int ret = 0;
+ unsigned int state = TASK_UNINTERRUPTIBLE;
+ int ret;
+
+ if (flags & SUPER_LOCK_KILLABLE)
+ state = TASK_KILLABLE;
lockdep_assert_not_held(&sb->s_umount);
@@ -133,8 +165,7 @@ static int super_lock_common(struct super_block *sb, bool excl, bool killable)
return -ENOENT;
/* wait until the superblock is ready or dying */
- ret = super_wait(sb, SB_BORN | SB_DYING,
- killable ? TASK_KILLABLE : TASK_UNINTERRUPTIBLE);
+ ret = super_wait(sb, SB_BORN | SB_DYING, state);
if (ret)
return ret;
@@ -142,10 +173,7 @@ static int super_lock_common(struct super_block *sb, bool excl, bool killable)
if (super_flags(sb, SB_DYING))
return -ENOENT;
- if (killable)
- ret = __super_lock_killable(sb, excl);
- else
- __super_lock(sb, excl);
+ ret = __super_lock(sb, flags);
if (ret)
return ret;
@@ -155,7 +183,7 @@ static int super_lock_common(struct super_block *sb, bool excl, bool killable)
* grab a reference to this. Tell them so.
*/
if (sb->s_flags & SB_DYING) {
- super_unlock(sb, excl);
+ super_unlock(sb, flags);
return -ENOENT;
}
@@ -163,49 +191,6 @@ static int super_lock_common(struct super_block *sb, bool excl, bool killable)
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);
-}
-
/**
* super_wait_shutdown_killable - wait for a superblock being shut down
* @sb: superblock super_lock() failed on
@@ -240,18 +225,6 @@ static void super_wait_shutdown(struct super_block *sb)
super_wait(sb, SB_DYING, TASK_UNINTERRUPTIBLE);
}
-/* wait and try to acquire read-side of @sb->s_umount */
-static inline bool super_lock_shared(struct super_block *sb)
-{
- return super_lock(sb, false);
-}
-
-/* wait and try to acquire write-side of @sb->s_umount */
-static inline bool super_lock_excl(struct super_block *sb)
-{
- return super_lock(sb, true);
-}
-
/* wake waiters */
#define SUPER_WAKE_FLAGS (SB_BORN | SB_DYING | SB_DEAD)
static void super_wake(struct super_block *sb, unsigned int flag)
@@ -726,7 +699,7 @@ static int grab_super(struct super_block *sb)
refcount_inc(&sb->s_passive);
spin_unlock(&sb_lock);
- ret = super_lock_killable(sb, true);
+ ret = super_lock(sb, SUPER_LOCK_EXCL | SUPER_LOCK_KILLABLE);
if (!ret) {
if (atomic_inc_not_zero(&sb->s_active)) {
put_super(sb);
@@ -1078,7 +1051,7 @@ static int __iterate_supers(void (*f)(struct super_block *, void *), void *arg,
if (flags & SUPER_ITER_UNLOCKED) {
f(sb, arg);
- } else if (super_lock_shared(sb)) {
+ } else if (!super_lock(sb, 0)) {
f(sb, arg);
super_unlock_shared(sb);
} else if (flags & SUPER_ITER_WAIT_SHUTDOWN) {
@@ -1155,8 +1128,6 @@ void iterate_supers_type(struct file_system_type *type,
spin_lock(&sb_lock);
hlist_for_each_entry(sb, &type->fs_supers, s_instances) {
- bool locked;
-
if (super_flags(sb, SB_DYING))
continue;
@@ -1165,8 +1136,7 @@ void iterate_supers_type(struct file_system_type *type,
spin_unlock(&sb_lock);
- locked = super_lock_shared(sb);
- if (locked) {
+ if (!super_lock(sb, 0)) {
f(sb, arg);
super_unlock_shared(sb);
}
@@ -1191,7 +1161,8 @@ struct super_block *user_get_super(dev_t dev, bool excl)
for_each_dev_super_killable(sb, &cursor) {
int ret;
- ret = super_lock_killable(sb, excl);
+ ret = super_lock(sb, (excl ? SUPER_LOCK_EXCL : 0) |
+ SUPER_LOCK_KILLABLE);
if (ret == -ENOENT)
continue;
super_dev_walk_end(&cursor);
@@ -1314,7 +1285,7 @@ static void do_emergency_remount_callback(struct super_block *sb, void *unused)
{
struct fs_context *fc = NULL;
- if (!super_lock_excl(sb)) {
+ if (super_lock(sb, SUPER_LOCK_EXCL)) {
/* the unmount writes the filesystem out, wait for that */
super_wait_shutdown(sb);
return;
@@ -1362,7 +1333,7 @@ static inline bool get_active_super(struct super_block *sb)
{
bool active = false;
- if (super_lock_excl(sb)) {
+ if (!super_lock(sb, SUPER_LOCK_EXCL)) {
active = atomic_inc_not_zero(&sb->s_active);
super_unlock_excl(sb);
}
@@ -1379,7 +1350,7 @@ static void do_thaw_all_callback(struct super_block *sb, void *unused)
while (sb->s_bdev && !bdev_thaw(sb->s_bdev))
pr_warn("Emergency Thaw on %pg\n", sb->s_bdev);
- if (super_lock_excl(sb))
+ if (!super_lock(sb, SUPER_LOCK_EXCL))
thaw_super_locked(sb, FREEZE_HOLDER_USERSPACE, NULL);
deactivate_super(sb);
}
@@ -1668,7 +1639,7 @@ static int fs_super_thaw(struct super_block *sb)
}
/* The caller holds an active reference. */
- if (WARN_ON_ONCE(!super_lock_excl(sb)))
+ if (WARN_ON_ONCE(super_lock(sb, SUPER_LOCK_EXCL)))
return 0;
/* FITHAW or an emergency thaw may have dropped the freeze already. */
@@ -1689,7 +1660,7 @@ 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_shared(sb)) {
+ if (super_lock(sb, 0)) {
/* don't take the device away from under an unmount */
if (!surprise)
super_wait_shutdown(sb);
@@ -1720,7 +1691,7 @@ static void fs_bdev_sync(struct block_device *bdev)
for_each_dev_super_killable(sb, &cursor) {
int ret;
- ret = super_lock_killable(sb, false);
+ ret = super_lock(sb, SUPER_LOCK_KILLABLE);
if (!ret) {
if (sb->s_root && (sb->s_flags & SB_ACTIVE))
sync_filesystem(sb);
@@ -2517,7 +2488,7 @@ int freeze_super(struct super_block *sb, enum freeze_holder who, const void *fre
{
int ret;
- if (!super_lock_excl(sb)) {
+ if (super_lock(sb, SUPER_LOCK_EXCL)) {
WARN_ONCE(1, "Dying superblock while freezing!");
return -EINVAL;
}
@@ -2684,7 +2655,7 @@ static int thaw_super_locked(struct super_block *sb, enum freeze_holder who,
int thaw_super(struct super_block *sb, enum freeze_holder who,
const void *freeze_owner)
{
- if (!super_lock_excl(sb)) {
+ if (super_lock(sb, SUPER_LOCK_EXCL)) {
WARN_ONCE(1, "Dying superblock while thawing!");
return -EINVAL;
}
--
2.53.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/5] super: let super_lock() wait for a superblock being shut down
2026-10-23 9:41 [PATCH 0/5] super: rework super_lock() Christian Brauner
2026-10-23 9:41 ` [PATCH 1/5] super: add super_wait() Christian Brauner
2026-10-23 9:41 ` [PATCH 2/5] super: let super_lock() take flags Christian Brauner
@ 2026-10-23 9:41 ` Christian Brauner
2026-10-23 9:41 ` [PATCH 4/5] super: make sync(2)'s wait for s_umount killable Christian Brauner
2026-10-23 9:41 ` [PATCH 5/5] super: add a guard for super_lock() Christian Brauner
4 siblings, 0 replies; 6+ messages in thread
From: Christian Brauner @ 2026-10-23 9:41 UTC (permalink / raw)
To: Jan Kara
Cc: Alexander Viro, Josef Bacik, linux-fsdevel, linux-kernel,
Christian Brauner (Amutable)
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) <brauner@kernel.org>
---
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 4/5] super: make sync(2)'s wait for s_umount killable
2026-10-23 9:41 [PATCH 0/5] super: rework super_lock() Christian Brauner
` (2 preceding siblings ...)
2026-10-23 9:41 ` [PATCH 3/5] super: let super_lock() wait for a superblock being shut down Christian Brauner
@ 2026-10-23 9:41 ` Christian Brauner
2026-10-23 9:41 ` [PATCH 5/5] super: add a guard for super_lock() Christian Brauner
4 siblings, 0 replies; 6+ messages in thread
From: Christian Brauner @ 2026-10-23 9:41 UTC (permalink / raw)
To: Jan Kara
Cc: Alexander Viro, Josef Bacik, linux-fsdevel, linux-kernel,
Christian Brauner (Amutable)
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 5/5] super: add a guard for super_lock()
2026-10-23 9:41 [PATCH 0/5] super: rework super_lock() Christian Brauner
` (3 preceding siblings ...)
2026-10-23 9:41 ` [PATCH 4/5] super: make sync(2)'s wait for s_umount killable Christian Brauner
@ 2026-10-23 9:41 ` Christian Brauner
4 siblings, 0 replies; 6+ messages in thread
From: Christian Brauner @ 2026-10-23 9:41 UTC (permalink / raw)
To: Jan Kara
Cc: Alexander Viro, Josef Bacik, linux-fsdevel, linux-kernel,
Christian Brauner (Amutable)
Six callers take s_umount and drop it again in the same scope and have
to pick the unlock that matches the lock they took. Add a guard for
super_lock() that unlocks with the flags it locked with and convert
them. Similar to irqdesc_lock let's build it from
__DEFINE_UNLOCK_GUARD() as super_lock() takes its flags at runtime.
The device walks use ACQUIRE() instead of scoped_guard() so a continue
still moves on to the next superblock. __iterate_supers() uses ACQUIRE()
as well because, like fs_bdev_sync(), it needs ACQUIRE_ERR() to tell
-ENOENT from a fatal signal. The callers that hand the lock off stay as
they are.
No functional changes.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
fs/super.c | 83 ++++++++++++++++++++++++++++++++++++++------------------------
1 file changed, 51 insertions(+), 32 deletions(-)
diff --git a/fs/super.c b/fs/super.c
index a0f9eadf0da7..aff2169ca94d 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -206,6 +206,31 @@ static __must_check int super_lock(struct super_block *sb,
return -ENOENT;
}
+static inline void super_unlock_guard(struct super_block *sb,
+ enum super_lock_flags_t flags)
+{
+ if (!IS_ERR_OR_NULL(sb))
+ super_unlock(sb, flags);
+}
+
+/* Unlocks with the flags it locked with. */
+__DEFINE_CLASS_IS_CONDITIONAL(super_lock, true);
+__DEFINE_UNLOCK_GUARD(super_lock, struct super_block,
+ super_unlock_guard(_T->lock, _T->flags),
+ enum super_lock_flags_t flags);
+
+static inline class_super_lock_t
+class_super_lock_constructor(struct super_block *sb, enum super_lock_flags_t flags)
+{
+ class_super_lock_t _t = { .lock = sb, .flags = flags };
+ int ret;
+
+ ret = super_lock(sb, flags);
+ if (ret)
+ _t.lock = ERR_PTR(ret);
+ return _t;
+}
+
/* wake waiters */
#define SUPER_WAKE_FLAGS (SB_BORN | SB_DYING | SB_DEAD)
static void super_wake(struct super_block *sb, unsigned int flag)
@@ -1037,13 +1062,13 @@ static int __iterate_supers(void (*f)(struct super_block *, void *), void *arg,
if (flags & SUPER_ITER_UNLOCKED) {
f(sb, arg);
} else {
- ret = super_lock(sb, lock_flags);
- if (!ret) {
+ ACQUIRE(super_lock, lock)(sb, lock_flags);
+
+ ret = ACQUIRE_ERR(super_lock, &lock);
+ if (!ret)
f(sb, arg);
- super_unlock_shared(sb);
- } else if (ret == -ENOENT) {
+ else if (ret == -ENOENT)
ret = 0;
- }
}
if (p)
@@ -1124,10 +1149,8 @@ void iterate_supers_type(struct file_system_type *type,
spin_unlock(&sb_lock);
- if (!super_lock(sb, 0)) {
+ scoped_guard(super_lock, sb, 0)
f(sb, arg);
- super_unlock_shared(sb);
- }
if (p)
put_super(p);
@@ -1274,18 +1297,18 @@ static void do_emergency_remount_callback(struct super_block *sb, void *unused)
struct fs_context *fc = NULL;
/* 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);
- if (sb->s_bdev && !sb_rdonly(sb)) {
- fc = fs_context_for_reconfigure(sb->s_root,
- SB_RDONLY | SB_FORCE, SB_RDONLY);
- if (!IS_ERR(fc) && parse_monolithic_mount_data(fc, NULL) == 0)
- (void)reconfigure_super(fc);
+ scoped_cond_guard(super_lock, return, sb,
+ SUPER_LOCK_EXCL | SUPER_LOCK_WAIT_SHUTDOWN) {
+ /* Make sure no reference put under s_umount is the last one. */
+ atomic_inc(&sb->s_active);
+ if (sb->s_bdev && !sb_rdonly(sb)) {
+ fc = fs_context_for_reconfigure(sb->s_root,
+ SB_RDONLY | SB_FORCE, SB_RDONLY);
+ if (!IS_ERR(fc) &&
+ parse_monolithic_mount_data(fc, NULL) == 0)
+ (void)reconfigure_super(fc);
+ }
}
- super_unlock_excl(sb);
/*
* Our active reference may be the last one by now, so only drop it
@@ -1318,13 +1341,9 @@ void emergency_remount(void)
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 | flags)) {
- active = atomic_inc_not_zero(&sb->s_active);
- super_unlock_excl(sb);
- }
- return active;
+ scoped_guard(super_lock, sb, SUPER_LOCK_EXCL | flags)
+ return atomic_inc_not_zero(&sb->s_active);
+ return false;
}
static void do_thaw_all_callback(struct super_block *sb, void *unused)
@@ -1646,7 +1665,9 @@ static void fs_bdev_mark_dead(struct block_device *bdev, bool surprise)
for_each_dev_super(sb, &cursor) {
/* don't take the device away from under an unmount */
- if (super_lock(sb, surprise ? 0 : SUPER_LOCK_WAIT_SHUTDOWN))
+ ACQUIRE(super_lock, lock)(sb, surprise ? 0 :
+ SUPER_LOCK_WAIT_SHUTDOWN);
+ if (ACQUIRE_ERR(super_lock, &lock))
continue;
if (sb->s_root && (sb->s_flags & SB_ACTIVE)) {
if (!sb->s_op->remove_bdev ||
@@ -1659,7 +1680,6 @@ static void fs_bdev_mark_dead(struct block_device *bdev, bool surprise)
sb->s_op->shutdown(sb);
}
}
- super_unlock_shared(sb);
}
}
@@ -1671,10 +1691,10 @@ static void fs_bdev_sync(struct block_device *bdev)
mutex_unlock(&bdev->bd_holder_lock);
for_each_dev_super_killable(sb, &cursor) {
- int ret;
+ ACQUIRE(super_lock, lock)(sb, SUPER_LOCK_KILLABLE |
+ SUPER_LOCK_WAIT_SHUTDOWN);
+ int ret = ACQUIRE_ERR(super_lock, &lock);
- ret = super_lock(sb, SUPER_LOCK_KILLABLE |
- SUPER_LOCK_WAIT_SHUTDOWN);
if (ret == -ENOENT)
continue;
if (ret) {
@@ -1684,7 +1704,6 @@ static void fs_bdev_sync(struct block_device *bdev)
}
if (sb->s_root && (sb->s_flags & SB_ACTIVE))
sync_filesystem(sb);
- super_unlock_shared(sb);
}
}
--
2.53.0
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-10-23 9:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-23 9:41 [PATCH 0/5] super: rework super_lock() Christian Brauner
2026-10-23 9:41 ` [PATCH 1/5] super: add super_wait() Christian Brauner
2026-10-23 9:41 ` [PATCH 2/5] super: let super_lock() take flags Christian Brauner
2026-10-23 9:41 ` [PATCH 3/5] super: let super_lock() wait for a superblock being shut down Christian Brauner
2026-10-23 9:41 ` [PATCH 4/5] super: make sync(2)'s wait for s_umount killable Christian Brauner
2026-10-23 9:41 ` [PATCH 5/5] super: add a guard for super_lock() Christian Brauner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®