On Tue, Aug 22, 2023 at 11:05:51AM +1000, Stephen Rothwell wrote:
> Hi all,
>
> Today's linux-next merge of the vfs-brauner tree got a conflict in:
>
> fs/super.c
>
> between commits:
>
> 880b9577855e ("fs: distinguish between user initiated freeze and kernel initiated freeze")
> 59ba4fdd2d1f ("fs: wait for partially frozen filesystems")
>
> from the djw-vfs tree and commits:
>
> 0ed33598ddf3 ("super: use locking helpers")
> 5e8749141521 ("super: wait for nascent superblocks")
>
> from the vfs-brauner tree.
>
> I fixed it up (I think - see below) and can carry the fix as
> necessary. This is now fixed as far as linux-next is concerned, but any
> non trivial conflicts should be mentioned to your upstream maintainer
> when your tree is submitted for merging. You may also want to consider
> cooperating with the maintainer of the conflicting tree to minimise any
> particularly complex conflicts.
That looks correct, thank you.
Christian: I've been planning to merge the {freeze,thaw}_super @who
changes for 6.6; do you think more 'cooperating with the maintainer' is
needed, or shall I simply push my branch to Linus with a note that
s/down_write/super_lock_excl/ s/up_write/super_unlock_excl is needed to
resolve the merge the conflict?
--D
> --
> Cheers,
> Stephen Rothwell
>
> diff --cc fs/super.c
> index da68584815e4,a00e9f706f0f..000000000000
> --- a/fs/super.c
> +++ b/fs/super.c
> @@@ -1027,12 -1196,13 +1196,13 @@@ void emergency_remount(void
>
> static void do_thaw_all_callback(struct super_block *sb)
> {
> - down_write(&sb->s_umount);
> - if (sb->s_root && sb->s_flags & SB_BORN) {
> + bool born = super_lock_excl(sb);
> +
> + if (born && sb->s_root) {
> emergency_thaw_bdev(sb);
> - thaw_super_locked(sb);
> + thaw_super_locked(sb, FREEZE_HOLDER_USERSPACE);
> } else {
> - up_write(&sb->s_umount);
> + super_unlock_excl(sb);
> }
> }
>
> @@@ -1644,24 -1836,6 +1836,24 @@@ static void sb_freeze_unlock(struct sup
> percpu_up_write(sb->s_writers.rw_sem + level);
> }
>
> +static int wait_for_partially_frozen(struct super_block *sb)
> +{
> + int ret = 0;
> +
> + do {
> + unsigned short old = sb->s_writers.frozen;
> +
> - up_write(&sb->s_umount);
> ++ super_unlock_excl(sb);
> + ret = wait_var_event_killable(&sb->s_writers.frozen,
> + sb->s_writers.frozen != old);
> - down_write(&sb->s_umount);
> ++ __super_lock_excl(sb);
> + } while (ret == 0 &&
> + sb->s_writers.frozen != SB_UNFROZEN &&
> + sb->s_writers.frozen != SB_FREEZE_COMPLETE);
> +
> + return ret;
> +}
> +
> /**
> * freeze_super - lock the filesystem and force it into a consistent state
> * @sb: the super to lock
> @@@ -1711,34 -1874,10 +1903,34 @@@ int freeze_super(struct super_block *sb
> int ret;
>
> atomic_inc(&sb->s_active);
> - down_write(&sb->s_umount);
> + __super_lock_excl(sb);
> +
> +retry:
> + if (sb->s_writers.frozen == SB_FREEZE_COMPLETE) {
> + if (sb->s_writers.freeze_holders & who) {
> + deactivate_locked_super(sb);
> + return -EBUSY;
> + }
> +
> + WARN_ON(sb->s_writers.freeze_holders == 0);
> +
> + /*
> + * Someone else already holds this type of freeze; share the
> + * freeze and assign the active ref to the freeze.
> + */
> + sb->s_writers.freeze_holders |= who;
> - up_write(&sb->s_umount);
> ++ super_unlock_excl(sb);
> + return 0;
> + }
> +
> if (sb->s_writers.frozen != SB_UNFROZEN) {
> - deactivate_locked_super(sb);
> - return -EBUSY;
> + ret = wait_for_partially_frozen(sb);
> + if (ret) {
> + deactivate_locked_super(sb);
> + return ret;
> + }
> +
> + goto retry;
> }
>
> if (!(sb->s_flags & SB_BORN)) {
> @@@ -1748,10 -1887,8 +1940,10 @@@
>
> if (sb_rdonly(sb)) {
> /* Nothing to do really... */
> + sb->s_writers.freeze_holders |= who;
> sb->s_writers.frozen = SB_FREEZE_COMPLETE;
> + wake_up_var(&sb->s_writers.frozen);
> - up_write(&sb->s_umount);
> + super_unlock_excl(sb);
> return 0;
> }
>
> @@@ -1795,11 -1930,9 +1987,11 @@@
> * For debugging purposes so that fs can warn if it sees write activity
> * when frozen is set to SB_FREEZE_COMPLETE, and for thaw_super().
> */
> + sb->s_writers.freeze_holders |= who;
> sb->s_writers.frozen = SB_FREEZE_COMPLETE;
> + wake_up_var(&sb->s_writers.frozen);
> lockdep_sb_freeze_release(sb);
> - up_write(&sb->s_umount);
> + super_unlock_excl(sb);
> return 0;
> }
> EXPORT_SYMBOL(freeze_super);
> @@@ -1814,24 -1941,8 +2006,24 @@@ static int thaw_super_locked(struct sup
> {
> int error;
>
> - if (sb->s_writers.frozen != SB_FREEZE_COMPLETE) {
> + if (sb->s_writers.frozen == SB_FREEZE_COMPLETE) {
> + if (!(sb->s_writers.freeze_holders & who)) {
> - up_write(&sb->s_umount);
> ++ super_unlock_excl(sb);
> + return -EINVAL;
> + }
> +
> + /*
> + * Freeze is shared with someone else. Release our hold and
> + * drop the active ref that freeze_super assigned to the
> + * freezer.
> + */
> + if (sb->s_writers.freeze_holders & ~who) {
> + sb->s_writers.freeze_holders &= ~who;
> + deactivate_locked_super(sb);
> + return 0;
> + }
> + } else {
> - up_write(&sb->s_umount);
> + super_unlock_excl(sb);
> return -EINVAL;
> }
>
> @@@ -1867,19 -1974,13 +2059,19 @@@ out
> /**
> * thaw_super -- unlock filesystem
> * @sb: the super to thaw
> + * @who: context that wants to freeze
> *
> - * Unlocks the filesystem and marks it writeable again after freeze_super().
> + * Unlocks the filesystem and marks it writeable again after freeze_super()
> + * if there are no remaining freezes on the filesystem.
> + *
> + * @who should be:
> + * * %FREEZE_HOLDER_USERSPACE if userspace wants to thaw the fs;
> + * * %FREEZE_HOLDER_KERNEL if the kernel wants to thaw the fs.
> */
> -int thaw_super(struct super_block *sb)
> +int thaw_super(struct super_block *sb, enum freeze_holder who)
> {
> - down_write(&sb->s_umount);
> + __super_lock_excl(sb);
> - return thaw_super_locked(sb);
> + return thaw_super_locked(sb, who);
> }
> EXPORT_SYMBOL(thaw_super);
>
