| Message ID | 20211125091443.762092-2-nborisov@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support adding a device during paused balance | expand |
On Thu, Nov 25, 2021 at 11:14:41AM +0200, Nikolay Borisov wrote: > Current set of exclusive operation states is not sufficient to handle all > practical use cases. In particular there is a need to be able to add a > device to a filesystem that have paused balance. Currently there is no > way to distinguish between a running and a paused balance. Fix this by > introducing BTRFS_EXCLOP_BALANCE_PAUSED which is going to be set in 2 > occasions: > > 1. When a filesystem is mounted with skip_balance and there is an > unfinished balance it will now be into BALANCE_PAUSED instead of > simply BALANCE state. > > 2. When a running balance is paused. > > Signed-off-by: Nikolay Borisov <nborisov@suse.com> > --- > fs/btrfs/ctree.h | 4 ++++ > fs/btrfs/ioctl.c | 23 +++++++++++++++++++++++ > fs/btrfs/volumes.c | 10 ++++++++-- > 3 files changed, 35 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h > index 5e29f3fc527d..79a873c6d186 100644 > --- a/fs/btrfs/ctree.h > +++ b/fs/btrfs/ctree.h > @@ -613,6 +613,7 @@ enum { > */ > enum btrfs_exclusive_operation { > BTRFS_EXCLOP_NONE, > + BTRFS_EXCLOP_BALANCE_PAUSED, > BTRFS_EXCLOP_BALANCE, > BTRFS_EXCLOP_DEV_ADD, > BTRFS_EXCLOP_DEV_REMOVE, > @@ -3316,6 +3317,9 @@ bool btrfs_exclop_start_try_lock(struct btrfs_fs_info *fs_info, > enum btrfs_exclusive_operation type); > void btrfs_exclop_start_unlock(struct btrfs_fs_info *fs_info); > void btrfs_exclop_finish(struct btrfs_fs_info *fs_info); > +void btrfs_exclop_balance(struct btrfs_fs_info *fs_info, > + enum btrfs_exclusive_operation op); > + > > /* file.c */ > int __init btrfs_auto_defrag_init(void); > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c > index 5263f991ffff..ffe33e853bfa 100644 > --- a/fs/btrfs/ioctl.c > +++ b/fs/btrfs/ioctl.c > @@ -414,6 +414,28 @@ void btrfs_exclop_finish(struct btrfs_fs_info *fs_info) > sysfs_notify(&fs_info->fs_devices->fsid_kobj, NULL, "exclusive_operation"); > } > > +void btrfs_exclop_balance(struct btrfs_fs_info *fs_info, > + enum btrfs_exclusive_operation op) > +{ > + switch (op) { > + case BTRFS_EXCLOP_BALANCE_PAUSED: > + spin_lock(&fs_info->super_lock); > + ASSERT(fs_info->exclusive_operation == BTRFS_EXCLOP_BALANCE || > + fs_info->exclusive_operation == BTRFS_EXCLOP_DEV_ADD); > + fs_info->exclusive_operation = BTRFS_EXCLOP_BALANCE_PAUSED; > + spin_unlock(&fs_info->super_lock); > + break; > + case BTRFS_EXCLOP_BALANCE: > + spin_lock(&fs_info->super_lock); > + ASSERT(fs_info->exclusive_operation == BTRFS_EXCLOP_BALANCE_PAUSED); > + fs_info->exclusive_operation = BTRFS_EXCLOP_BALANCE; > + spin_unlock(&fs_info->super_lock); > + break; > + default: > + WARN(1, "BTRFS: invalid balance operation requested\n"); As the fs_info is available, this shoud use btrfs_warn because it also prints the uuid/device of the filesytem, otherwise the message is useless. Also it could contain the number of the operation. I'll fix that.
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h index 5e29f3fc527d..79a873c6d186 100644 --- a/fs/btrfs/ctree.h +++ b/fs/btrfs/ctree.h @@ -613,6 +613,7 @@ enum { */ enum btrfs_exclusive_operation { BTRFS_EXCLOP_NONE, + BTRFS_EXCLOP_BALANCE_PAUSED, BTRFS_EXCLOP_BALANCE, BTRFS_EXCLOP_DEV_ADD, BTRFS_EXCLOP_DEV_REMOVE, @@ -3316,6 +3317,9 @@ bool btrfs_exclop_start_try_lock(struct btrfs_fs_info *fs_info, enum btrfs_exclusive_operation type); void btrfs_exclop_start_unlock(struct btrfs_fs_info *fs_info); void btrfs_exclop_finish(struct btrfs_fs_info *fs_info); +void btrfs_exclop_balance(struct btrfs_fs_info *fs_info, + enum btrfs_exclusive_operation op); + /* file.c */ int __init btrfs_auto_defrag_init(void); diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 5263f991ffff..ffe33e853bfa 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -414,6 +414,28 @@ void btrfs_exclop_finish(struct btrfs_fs_info *fs_info) sysfs_notify(&fs_info->fs_devices->fsid_kobj, NULL, "exclusive_operation"); } +void btrfs_exclop_balance(struct btrfs_fs_info *fs_info, + enum btrfs_exclusive_operation op) +{ + switch (op) { + case BTRFS_EXCLOP_BALANCE_PAUSED: + spin_lock(&fs_info->super_lock); + ASSERT(fs_info->exclusive_operation == BTRFS_EXCLOP_BALANCE || + fs_info->exclusive_operation == BTRFS_EXCLOP_DEV_ADD); + fs_info->exclusive_operation = BTRFS_EXCLOP_BALANCE_PAUSED; + spin_unlock(&fs_info->super_lock); + break; + case BTRFS_EXCLOP_BALANCE: + spin_lock(&fs_info->super_lock); + ASSERT(fs_info->exclusive_operation == BTRFS_EXCLOP_BALANCE_PAUSED); + fs_info->exclusive_operation = BTRFS_EXCLOP_BALANCE; + spin_unlock(&fs_info->super_lock); + break; + default: + WARN(1, "BTRFS: invalid balance operation requested\n"); + } +} + static int btrfs_ioctl_getversion(struct file *file, int __user *arg) { struct inode *inode = file_inode(file); @@ -4062,6 +4084,7 @@ static long btrfs_ioctl_balance(struct file *file, void __user *arg) spin_lock(&fs_info->balance_lock); bctl->flags |= BTRFS_BALANCE_RESUME; spin_unlock(&fs_info->balance_lock); + btrfs_exclop_balance(fs_info, BTRFS_EXCLOP_BALANCE); goto do_balance; } diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 45c91a2f172c..8cbc82d49ef0 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -4388,8 +4388,10 @@ int btrfs_balance(struct btrfs_fs_info *fs_info, ret = __btrfs_balance(fs_info); mutex_lock(&fs_info->balance_mutex); - if (ret == -ECANCELED && atomic_read(&fs_info->balance_pause_req)) + if (ret == -ECANCELED && atomic_read(&fs_info->balance_pause_req)) { btrfs_info(fs_info, "balance: paused"); + btrfs_exclop_balance(fs_info, BTRFS_EXCLOP_BALANCE_PAUSED); + } /* * Balance can be canceled by: * @@ -4465,6 +4467,10 @@ int btrfs_resume_balance_async(struct btrfs_fs_info *fs_info) return 0; } + spin_lock(&fs_info->super_lock); + ASSERT(fs_info->exclusive_operation == BTRFS_EXCLOP_BALANCE_PAUSED); + fs_info->exclusive_operation = BTRFS_EXCLOP_BALANCE; + spin_unlock(&fs_info->super_lock); /* * A ro->rw remount sequence should continue with the paused balance * regardless of who pauses it, system or the user as of now, so set @@ -4533,7 +4539,7 @@ int btrfs_recover_balance(struct btrfs_fs_info *fs_info) * is in a paused state and must have fs_info::balance_ctl properly * set up. */ - if (!btrfs_exclop_start(fs_info, BTRFS_EXCLOP_BALANCE)) + if (!btrfs_exclop_start(fs_info, BTRFS_EXCLOP_BALANCE_PAUSED)) btrfs_warn(fs_info, "balance: cannot set exclusive op status, resume manually");
Current set of exclusive operation states is not sufficient to handle all practical use cases. In particular there is a need to be able to add a device to a filesystem that have paused balance. Currently there is no way to distinguish between a running and a paused balance. Fix this by introducing BTRFS_EXCLOP_BALANCE_PAUSED which is going to be set in 2 occasions: 1. When a filesystem is mounted with skip_balance and there is an unfinished balance it will now be into BALANCE_PAUSED instead of simply BALANCE state. 2. When a running balance is paused. Signed-off-by: Nikolay Borisov <nborisov@suse.com> --- fs/btrfs/ctree.h | 4 ++++ fs/btrfs/ioctl.c | 23 +++++++++++++++++++++++ fs/btrfs/volumes.c | 10 ++++++++-- 3 files changed, 35 insertions(+), 2 deletions(-)