All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/3] Support adding a device during paused balance
@ 2021-11-25  9:14 Nikolay Borisov
  2021-11-25  9:14 ` [PATCH v3 1/3] btrfs: introduce BTRFS_EXCLOP_BALANCE_PAUSED exclusive state Nikolay Borisov
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Nikolay Borisov @ 2021-11-25  9:14 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Nikolay Borisov


So here's v3 which takes care of the feedback received in the last round, namely:

 - Introduced btrfs_exclop_balance which is used to switch the balance state,
 alongside ASSERTS
 - Removed unused variable in balance_kthread

Nikolay Borisov (3):
  btrfs: introduce BTRFS_EXCLOP_BALANCE_PAUSED exclusive state
  btrfs: make device add compatible with paused balance in
    btrfs_exclop_start_try_lock
  btrfs: allow device add if balance is paused

 fs/btrfs/ctree.h   |  4 ++++
 fs/btrfs/ioctl.c   | 49 ++++++++++++++++++++++++++++++++++++++++++----
 fs/btrfs/volumes.c | 10 ++++++++--
 3 files changed, 57 insertions(+), 6 deletions(-)

--
2.25.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 1/3] btrfs: introduce BTRFS_EXCLOP_BALANCE_PAUSED exclusive state
  2021-11-25  9:14 [PATCH v3 0/3] Support adding a device during paused balance Nikolay Borisov
@ 2021-11-25  9:14 ` Nikolay Borisov
  2021-11-25 15:35   ` David Sterba
  2021-11-25  9:14 ` [PATCH v3 2/3] btrfs: make device add compatible with paused balance in btrfs_exclop_start_try_lock Nikolay Borisov
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 6+ messages in thread
From: Nikolay Borisov @ 2021-11-25  9:14 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Nikolay Borisov

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");
+	}
+}
+
 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");
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 2/3] btrfs: make device add compatible with paused balance in btrfs_exclop_start_try_lock
  2021-11-25  9:14 [PATCH v3 0/3] Support adding a device during paused balance Nikolay Borisov
  2021-11-25  9:14 ` [PATCH v3 1/3] btrfs: introduce BTRFS_EXCLOP_BALANCE_PAUSED exclusive state Nikolay Borisov
@ 2021-11-25  9:14 ` Nikolay Borisov
  2021-11-25  9:14 ` [PATCH v3 3/3] btrfs: allow device add if balance is paused Nikolay Borisov
  2021-11-25 15:43 ` [PATCH v3 0/3] Support adding a device during paused balance David Sterba
  3 siblings, 0 replies; 6+ messages in thread
From: Nikolay Borisov @ 2021-11-25  9:14 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Nikolay Borisov

This is needed to enable device add to work in cases when a file system
has been mounted with 'skip_balance' mount option.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/ioctl.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index ffe33e853bfa..ab05319854f8 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -387,6 +387,7 @@ bool btrfs_exclop_start(struct btrfs_fs_info *fs_info,
  *
  * Compatibility:
  * - the same type is already running
+ * - when trying to add a device and balance has been paused
  * - not BTRFS_EXCLOP_NONE - this is intentionally incompatible and the caller
  *   must check the condition first that would allow none -> @type
  */
@@ -394,7 +395,9 @@ bool btrfs_exclop_start_try_lock(struct btrfs_fs_info *fs_info,
 				 enum btrfs_exclusive_operation type)
 {
 	spin_lock(&fs_info->super_lock);
-	if (fs_info->exclusive_operation == type)
+	if (fs_info->exclusive_operation == type ||
+	    (fs_info->exclusive_operation == BTRFS_EXCLOP_BALANCE_PAUSED &&
+	     type == BTRFS_EXCLOP_DEV_ADD))
 		return true;
 
 	spin_unlock(&fs_info->super_lock);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 3/3] btrfs: allow device add if balance is paused
  2021-11-25  9:14 [PATCH v3 0/3] Support adding a device during paused balance Nikolay Borisov
  2021-11-25  9:14 ` [PATCH v3 1/3] btrfs: introduce BTRFS_EXCLOP_BALANCE_PAUSED exclusive state Nikolay Borisov
  2021-11-25  9:14 ` [PATCH v3 2/3] btrfs: make device add compatible with paused balance in btrfs_exclop_start_try_lock Nikolay Borisov
@ 2021-11-25  9:14 ` Nikolay Borisov
  2021-11-25 15:43 ` [PATCH v3 0/3] Support adding a device during paused balance David Sterba
  3 siblings, 0 replies; 6+ messages in thread
From: Nikolay Borisov @ 2021-11-25  9:14 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Nikolay Borisov

Currently paused balance precludes adding a device since they are both
considered exclusive ops and we can have at most 1 running at a time.
This is problematic in case a filesystem encounters an ENOSPC situation
while balance is running, in this case the only thing the user can do
is mount the fs with "skip_balance" which pauses balance and delete some
data to free up space for balance. However, it should be possible to add
a new device when balance is paused.

Fix this by allowing device add to proceed when balance is paused.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/ioctl.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index ab05319854f8..6d617d2d5fa2 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3172,13 +3172,25 @@ static int btrfs_ioctl_defrag(struct file *file, void __user *argp)
 static long btrfs_ioctl_add_dev(struct btrfs_fs_info *fs_info, void __user *arg)
 {
 	struct btrfs_ioctl_vol_args *vol_args;
+	bool restore_op = false;
 	int ret;
 
 	if (!capable(CAP_SYS_ADMIN))
 		return -EPERM;
 
-	if (!btrfs_exclop_start(fs_info, BTRFS_EXCLOP_DEV_ADD))
-		return BTRFS_ERROR_DEV_EXCL_RUN_IN_PROGRESS;
+	if (!btrfs_exclop_start(fs_info, BTRFS_EXCLOP_DEV_ADD)) {
+		if (!btrfs_exclop_start_try_lock(fs_info, BTRFS_EXCLOP_DEV_ADD))
+			return BTRFS_ERROR_DEV_EXCL_RUN_IN_PROGRESS;
+
+		/*
+		 * We can do the device add because we have a paused balanced,
+		 * change the exclusive op type and remember we should bring
+		 * back the paused balance
+		 */
+		fs_info->exclusive_operation = BTRFS_EXCLOP_DEV_ADD;
+		btrfs_exclop_start_unlock(fs_info);
+		restore_op = true;
+	}
 
 	vol_args = memdup_user(arg, sizeof(*vol_args));
 	if (IS_ERR(vol_args)) {
@@ -3194,7 +3206,10 @@ static long btrfs_ioctl_add_dev(struct btrfs_fs_info *fs_info, void __user *arg)
 
 	kfree(vol_args);
 out:
-	btrfs_exclop_finish(fs_info);
+	if (restore_op)
+		btrfs_exclop_balance(fs_info, BTRFS_EXCLOP_BALANCE_PAUSED);
+	else
+		btrfs_exclop_finish(fs_info);
 	return ret;
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/3] btrfs: introduce BTRFS_EXCLOP_BALANCE_PAUSED exclusive state
  2021-11-25  9:14 ` [PATCH v3 1/3] btrfs: introduce BTRFS_EXCLOP_BALANCE_PAUSED exclusive state Nikolay Borisov
@ 2021-11-25 15:35   ` David Sterba
  0 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2021-11-25 15:35 UTC (permalink / raw)
  To: Nikolay Borisov; +Cc: linux-btrfs

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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 0/3] Support adding a device during paused balance
  2021-11-25  9:14 [PATCH v3 0/3] Support adding a device during paused balance Nikolay Borisov
                   ` (2 preceding siblings ...)
  2021-11-25  9:14 ` [PATCH v3 3/3] btrfs: allow device add if balance is paused Nikolay Borisov
@ 2021-11-25 15:43 ` David Sterba
  3 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2021-11-25 15:43 UTC (permalink / raw)
  To: Nikolay Borisov; +Cc: linux-btrfs

On Thu, Nov 25, 2021 at 11:14:40AM +0200, Nikolay Borisov wrote:
> 
> So here's v3 which takes care of the feedback received in the last round, namely:
> 
>  - Introduced btrfs_exclop_balance which is used to switch the balance state,
>  alongside ASSERTS
>  - Removed unused variable in balance_kthread
> 
> Nikolay Borisov (3):
>   btrfs: introduce BTRFS_EXCLOP_BALANCE_PAUSED exclusive state
>   btrfs: make device add compatible with paused balance in
>     btrfs_exclop_start_try_lock
>   btrfs: allow device add if balance is paused

With the minor message fixup added to misc-next, thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-11-25 15:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-25  9:14 [PATCH v3 0/3] Support adding a device during paused balance Nikolay Borisov
2021-11-25  9:14 ` [PATCH v3 1/3] btrfs: introduce BTRFS_EXCLOP_BALANCE_PAUSED exclusive state Nikolay Borisov
2021-11-25 15:35   ` David Sterba
2021-11-25  9:14 ` [PATCH v3 2/3] btrfs: make device add compatible with paused balance in btrfs_exclop_start_try_lock Nikolay Borisov
2021-11-25  9:14 ` [PATCH v3 3/3] btrfs: allow device add if balance is paused Nikolay Borisov
2021-11-25 15:43 ` [PATCH v3 0/3] Support adding a device during paused balance David Sterba

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.