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

[Jan Kara]

On Tue 27-10-26 11:13:09, Christian Brauner wrote:
> On Tue, Oct 27, 2026 at 09:11:42AM +0100, Christian Brauner wrote:
> > On Mon, Oct 26, 2026 at 01:02:46PM +0100, Jan Kara wrote:
> > > On Fri 23-10-26 11:41:04, Christian Brauner wrote:
> > > > 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.
> > > 
> > > This might get surprising in the future, I guess SUPER_ITER_KILLABLE flag
> > > might be a more explicit way to control this but I can live with what we
> > > have now.
> > 
> > I think you're right I'll fix that up in-tree.
> 
> I've folded:

Thanks!

								Honza

> 
> diff --git a/fs/super.c b/fs/super.c
> index aff2169ca94d..cd6f57ababe0 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -1015,6 +1015,7 @@ enum super_iter_flags_t {
>         SUPER_ITER_REVERSE      = (1U << 1),
>         SUPER_ITER_WAIT_SHUTDOWN = (1U << 2),
>         SUPER_ITER_SKIP_UNBORN  = (1U << 3),
> +       SUPER_ITER_KILLABLE     = (1U << 4),
>  };
> 
>  static inline struct super_block *first_super(enum super_iter_flags_t flags)
> @@ -1040,7 +1041,9 @@ static int __iterate_supers(void (*f)(struct super_block *, void *), void *arg,
>         int ret = 0;
> 
>         if (flags & SUPER_ITER_WAIT_SHUTDOWN)
> -               lock_flags |= SUPER_LOCK_KILLABLE | SUPER_LOCK_WAIT_SHUTDOWN;
> +               lock_flags |= SUPER_LOCK_WAIT_SHUTDOWN;
> +       if (flags & SUPER_ITER_KILLABLE)
> +               lock_flags |= SUPER_LOCK_KILLABLE;
> 
>         spin_lock(&sb_lock);
> 
> @@ -1122,6 +1125,7 @@ void iterate_supers_born(void (*f)(struct super_block *, void *), void *arg)
>  int iterate_supers_sync(void (*f)(struct super_block *, void *), void *arg)
>  {
>         return __iterate_supers(f, arg, SUPER_ITER_WAIT_SHUTDOWN |
> +                                       SUPER_ITER_KILLABLE |
>                                         SUPER_ITER_SKIP_UNBORN);
>  }
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR