linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] Balance progress monitoring.
  2010-11-12  0:36 [PATCH v2 0/2] Balance management, kernel side Hugo Mills
@ 2010-11-09 22:43 ` Hugo Mills
  2010-11-09 22:52 ` [PATCH v2 2/2] Cancel filesystem balance Hugo Mills
  2011-03-20  8:52 ` [PATCH v2 0/2] Balance management, kernel side Andreas Philipp
  2 siblings, 0 replies; 14+ messages in thread
From: Hugo Mills @ 2010-11-09 22:43 UTC (permalink / raw)
  To: linux-btrfs, Goffredo Baroncelli, Chris Mason, liubo

This patch introduces a basic form of progress monitoring for balance
operations, by counting the number of block groups remaining. The
information is exposed to userspace by an ioctl.

Signed-off-by: Hugo Mills <hugo@carfax.org.uk>
---
 fs/btrfs/ctree.h   |    9 +++++++
 fs/btrfs/disk-io.c |    2 +
 fs/btrfs/ioctl.c   |   34 +++++++++++++++++++++++++++++
 fs/btrfs/ioctl.h   |    7 ++++++
 fs/btrfs/volumes.c |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 5 files changed, 111 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 8db9234..67fb603 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -841,6 +841,11 @@ struct btrfs_block_group_cache {
 	struct list_head cluster_list;
 };
 
+struct btrfs_balance_info {
+	u64 expected;
+	u64 completed;
+};
+
 struct reloc_control;
 struct btrfs_device;
 struct btrfs_fs_devices;
@@ -1050,6 +1055,10 @@ struct btrfs_fs_info {
 	unsigned metadata_ratio;
 
 	void *bdev_holder;
+
+	/* Keep track of any rebalance operations on this FS */
+	spinlock_t balance_info_lock;
+	struct btrfs_balance_info *balance_info;
 };
 
 /*
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index b40dfe4..87d9315 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1590,6 +1590,7 @@ struct btrfs_root *open_ctree(struct super_block *sb,
 	spin_lock_init(&fs_info->ref_cache_lock);
 	spin_lock_init(&fs_info->fs_roots_radix_lock);
 	spin_lock_init(&fs_info->delayed_iput_lock);
+	spin_lock_init(&fs_info->balance_info_lock);
 
 	init_completion(&fs_info->kobj_unregister);
 	fs_info->tree_root = tree_root;
@@ -1615,6 +1616,7 @@ struct btrfs_root *open_ctree(struct super_block *sb,
 	fs_info->sb = sb;
 	fs_info->max_inline = 8192 * 1024;
 	fs_info->metadata_ratio = 0;
+	fs_info->balance_info = NULL;
 
 	fs_info->thread_pool_size = min_t(unsigned long,
 					  num_online_cpus() + 2, 8);
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 463d91b..c247985 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2220,6 +2220,38 @@ static noinline long btrfs_ioctl_wait_sync(struct file *file, void __user *argp)
 	return btrfs_wait_for_commit(root, transid);
 }
 
+/*
+ * Return the current status of any balance operation
+ */
+long btrfs_ioctl_balance_progress(
+	struct btrfs_fs_info *fs_info,
+	struct btrfs_ioctl_balance_progress __user *user_dest)
+{
+	int ret = 0;
+	struct btrfs_ioctl_balance_progress dest;
+
+	spin_lock(&fs_info->balance_info_lock);
+	if (!fs_info->balance_info) {
+		ret = -EINVAL;
+		goto error;
+	}
+
+	dest.expected = fs_info->balance_info->expected;
+	dest.completed = fs_info->balance_info->completed;
+
+	spin_unlock(&fs_info->balance_info_lock);
+
+	if (copy_to_user(user_dest, &dest,
+			 sizeof(struct btrfs_ioctl_balance_progress)))
+		return -EFAULT;
+
+	return 0;
+
+error:
+	spin_unlock(&fs_info->balance_info_lock);
+	return ret;
+}
+
 long btrfs_ioctl(struct file *file, unsigned int
 		cmd, unsigned long arg)
 {
@@ -2255,6 +2287,8 @@ long btrfs_ioctl(struct file *file, unsigned int
 		return btrfs_ioctl_rm_dev(root, argp);
 	case BTRFS_IOC_BALANCE:
 		return btrfs_balance(root->fs_info->dev_root);
+	case BTRFS_IOC_BALANCE_PROGRESS:
+		return btrfs_ioctl_balance_progress(root->fs_info, argp);
 	case BTRFS_IOC_CLONE:
 		return btrfs_ioctl_clone(file, arg, 0, 0, 0);
 	case BTRFS_IOC_CLONE_RANGE:
diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h
index 17c99eb..b2103b2 100644
--- a/fs/btrfs/ioctl.h
+++ b/fs/btrfs/ioctl.h
@@ -145,6 +145,11 @@ struct btrfs_ioctl_space_args {
 	struct btrfs_ioctl_space_info spaces[0];
 };
 
+struct btrfs_ioctl_balance_progress {
+	__u64 expected;
+	__u64 completed;
+};
+
 #define BTRFS_IOC_SNAP_CREATE _IOW(BTRFS_IOCTL_MAGIC, 1, \
 				   struct btrfs_ioctl_vol_args)
 #define BTRFS_IOC_DEFRAG _IOW(BTRFS_IOCTL_MAGIC, 2, \
@@ -189,4 +194,6 @@ struct btrfs_ioctl_space_args {
 #define BTRFS_IOC_WAIT_SYNC  _IOW(BTRFS_IOCTL_MAGIC, 22, __u64)
 #define BTRFS_IOC_SNAP_CREATE_ASYNC _IOW(BTRFS_IOCTL_MAGIC, 23, \
 				   struct btrfs_ioctl_async_vol_args)
+#define BTRFS_IOC_BALANCE_PROGRESS _IOR(BTRFS_IOCTL_MAGIC, 25, \
+				  struct btrfs_ioctl_balance_progress)
 #endif
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 91851b5..f00edc1 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1904,6 +1904,7 @@ int btrfs_balance(struct btrfs_root *dev_root)
 	struct btrfs_root *chunk_root = dev_root->fs_info->chunk_root;
 	struct btrfs_trans_handle *trans;
 	struct btrfs_key found_key;
+	struct btrfs_balance_info *bal_info;
 
 	if (dev_root->fs_info->sb->s_flags & MS_RDONLY)
 		return -EROFS;
@@ -1911,6 +1912,20 @@ int btrfs_balance(struct btrfs_root *dev_root)
 	mutex_lock(&dev_root->fs_info->volume_mutex);
 	dev_root = dev_root->fs_info->dev_root;
 
+	bal_info = kmalloc(
+		sizeof(struct btrfs_balance_info),
+		GFP_NOFS);
+	if (!bal_info) {
+		ret = -ENOSPC;
+		goto error_no_status;
+	}
+	spin_lock(&dev_root->fs_info->balance_info_lock);
+	dev_root->fs_info->balance_info = bal_info;
+	bal_info->expected = -1; /* One less than actually counted,
+				    because chunk 0 is special */
+	bal_info->completed = 0;
+	spin_unlock(&dev_root->fs_info->balance_info_lock);
+
 	/* step one make some room on all the devices */
 	list_for_each_entry(device, devices, dev_list) {
 		old_size = device->total_bytes;
@@ -1934,10 +1949,42 @@ int btrfs_balance(struct btrfs_root *dev_root)
 		btrfs_end_transaction(trans, dev_root);
 	}
 
-	/* step two, relocate all the chunks */
+	/* step two, count the chunks */
 	path = btrfs_alloc_path();
-	BUG_ON(!path);
+	if (!path) {
+		ret = -ENOSPC;
+		goto error;
+	}
+
+	key.objectid = BTRFS_FIRST_CHUNK_TREE_OBJECTID;
+	key.offset = (u64)-1;
+	key.type = BTRFS_CHUNK_ITEM_KEY;
+
+	ret = btrfs_search_slot(NULL, chunk_root, &key, path, 0, 0);
+	if (ret <= 0) {
+		printk(KERN_ERR "btrfs: Failed to find the last chunk.\n");
+		BUG();
+	}
+
+	while (1) {
+		ret = btrfs_previous_item(chunk_root, path, 0,
+					  BTRFS_CHUNK_ITEM_KEY);
+		if (ret)
+			break;
+
+		spin_lock(&dev_root->fs_info->balance_info_lock);
+		bal_info->expected++;
+		spin_unlock(&dev_root->fs_info->balance_info_lock);
+	}
+
+	btrfs_free_path(path);
+	path = btrfs_alloc_path();
+	if (!path) {
+		ret = -ENOSPC;
+		goto error;
+	}
 
+	/* step three, relocate all the chunks */
 	key.objectid = BTRFS_FIRST_CHUNK_TREE_OBJECTID;
 	key.offset = (u64)-1;
 	key.type = BTRFS_CHUNK_ITEM_KEY;
@@ -1975,10 +2022,20 @@ int btrfs_balance(struct btrfs_root *dev_root)
 					   found_key.offset);
 		BUG_ON(ret && ret != -ENOSPC);
 		key.offset = found_key.offset - 1;
+		spin_lock(&dev_root->fs_info->balance_info_lock);
+		bal_info->completed++;
+		spin_unlock(&dev_root->fs_info->balance_info_lock);
+		printk(KERN_INFO "btrfs: balance: %llu/%llu block groups completed\n",
+		       bal_info->completed, bal_info->expected);
 	}
 	ret = 0;
 error:
 	btrfs_free_path(path);
+	spin_lock(&dev_root->fs_info->balance_info_lock);
+	kfree(dev_root->fs_info->balance_info);
+	dev_root->fs_info->balance_info = NULL;
+	spin_unlock(&dev_root->fs_info->balance_info_lock);
+error_no_status:
 	mutex_unlock(&dev_root->fs_info->volume_mutex);
 	return ret;
 }
-- 
1.7.1



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

* [PATCH v2 2/2] Cancel filesystem balance.
  2010-11-12  0:36 [PATCH v2 0/2] Balance management, kernel side Hugo Mills
  2010-11-09 22:43 ` [PATCH v2 1/2] Balance progress monitoring Hugo Mills
@ 2010-11-09 22:52 ` Hugo Mills
  2010-11-12  1:33   ` Li Zefan
  2011-03-20  8:52 ` [PATCH v2 0/2] Balance management, kernel side Andreas Philipp
  2 siblings, 1 reply; 14+ messages in thread
From: Hugo Mills @ 2010-11-09 22:52 UTC (permalink / raw)
  To: linux-btrfs, Goffredo Baroncelli, Chris Mason, liubo

This patch adds an ioctl for cancelling a btrfs balance operation
mid-flight. The ioctl simply sets a flag, and the operation terminates
after the current block group move has completed.

Signed-off-by: Hugo Mills <hugo@carfax.org.uk>
---
 fs/btrfs/ctree.h   |    1 +
 fs/btrfs/ioctl.c   |   28 ++++++++++++++++++++++++++++
 fs/btrfs/ioctl.h   |    3 ++-
 fs/btrfs/volumes.c |    7 ++++++-
 4 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 67fb603..5fa7163 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -844,6 +844,7 @@ struct btrfs_block_group_cache {
 struct btrfs_balance_info {
 	u64 expected;
 	u64 completed;
+	int cancel_pending;
 };
 
 struct reloc_control;
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index c247985..7e38856 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2252,6 +2252,32 @@ error:
 	return ret;
 }
 
+/*
+ * Cancel a running balance operation
+ */
+long btrfs_ioctl_balance_cancel(struct btrfs_fs_info *fs_info)
+{
+	int err = 0;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	spin_lock(&fs_info->balance_info_lock);
+	if(!fs_info->balance_info) {
+		err = -EINVAL;
+		goto error;
+	}
+	if(fs_info->balance_info->cancel_pending) {
+		err = -ECANCELED;
+		goto error;
+	}
+	fs_info->balance_info->cancel_pending = 1;
+
+error:
+	spin_unlock(&fs_info->balance_info_lock);
+	return err;
+}
+
 long btrfs_ioctl(struct file *file, unsigned int
 		cmd, unsigned long arg)
 {
@@ -2289,6 +2315,8 @@ long btrfs_ioctl(struct file *file, unsigned int
 		return btrfs_balance(root->fs_info->dev_root);
 	case BTRFS_IOC_BALANCE_PROGRESS:
 		return btrfs_ioctl_balance_progress(root->fs_info, argp);
+	case BTRFS_IOC_BALANCE_CANCEL:
+		return btrfs_ioctl_balance_cancel(root->fs_info);
 	case BTRFS_IOC_CLONE:
 		return btrfs_ioctl_clone(file, arg, 0, 0, 0);
 	case BTRFS_IOC_CLONE_RANGE:
diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h
index b2103b2..76ae121 100644
--- a/fs/btrfs/ioctl.h
+++ b/fs/btrfs/ioctl.h
@@ -195,5 +195,6 @@ struct btrfs_ioctl_balance_progress {
 #define BTRFS_IOC_SNAP_CREATE_ASYNC _IOW(BTRFS_IOCTL_MAGIC, 23, \
 				   struct btrfs_ioctl_async_vol_args)
 #define BTRFS_IOC_BALANCE_PROGRESS _IOR(BTRFS_IOCTL_MAGIC, 25, \
-				  struct btrfs_ioctl_balance_progress)
+				struct btrfs_ioctl_balance_progress)
+#define BTRFS_IOC_BALANCE_CANCEL _IO(BTRFS_IOCTL_MAGIC, 26)
 #endif
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index f00edc1..64b2f04 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1924,6 +1924,7 @@ int btrfs_balance(struct btrfs_root *dev_root)
 	bal_info->expected = -1; /* One less than actually counted,
 				    because chunk 0 is special */
 	bal_info->completed = 0;
+	bal_info->cancel_pending = 0;
 	spin_unlock(&dev_root->fs_info->balance_info_lock);
 
 	/* step one make some room on all the devices */
@@ -1989,7 +1990,7 @@ int btrfs_balance(struct btrfs_root *dev_root)
 	key.offset = (u64)-1;
 	key.type = BTRFS_CHUNK_ITEM_KEY;
 
-	while (1) {
+	while (!bal_info->cancel_pending) {
 		ret = btrfs_search_slot(NULL, chunk_root, &key, path, 0, 0);
 		if (ret < 0)
 			goto error;
@@ -2029,6 +2030,10 @@ int btrfs_balance(struct btrfs_root *dev_root)
 		       bal_info->completed, bal_info->expected);
 	}
 	ret = 0;
+	if(bal_info->cancel_pending) {
+		printk(KERN_INFO "btrfs: balance cancelled\n");
+		ret = -EINTR;
+	}
 error:
 	btrfs_free_path(path);
 	spin_lock(&dev_root->fs_info->balance_info_lock);
-- 
1.7.1


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

* [PATCH v2 0/2] Balance management, kernel side
@ 2010-11-12  0:36 Hugo Mills
  2010-11-09 22:43 ` [PATCH v2 1/2] Balance progress monitoring Hugo Mills
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Hugo Mills @ 2010-11-12  0:36 UTC (permalink / raw)
  To: linux-btrfs, Goffredo Baroncelli, Chris Mason, liubo

   These two patches give a degree of control over balance operations.
The first makes it possible to get an idea of how much work remains to
do, by tracking the number of block groups (chunks) that need to be
moved/rewritten. The second patch allows a running balance operation
to be cancelled when the current block group has been moved.

   Since the last version, I've added some more locking (assigning to
a u64 isn't atomic on non-64-bit architectures). I've not added the
sysfs bits, as I haven't had a chance to try out Goffredo's sysfs code
yet. I've also not implemented liubo's suggestion of tracking the
current block group ID (I'll take that discussion up with him
separately -- basically it's not a good fit with the "polling" method
required by this ioctl).

Hugo Mills (2):
  Balance progress monitoring.
  Cancel filesystem balance.

 fs/btrfs/ctree.h   |   10 ++++++++
 fs/btrfs/disk-io.c |    2 +
 fs/btrfs/ioctl.c   |   62 ++++++++++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/ioctl.h   |    8 ++++++
 fs/btrfs/volumes.c |   66 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 5 files changed, 146 insertions(+), 2 deletions(-)


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

* Re: [PATCH v2 2/2] Cancel filesystem balance.
  2010-11-09 22:52 ` [PATCH v2 2/2] Cancel filesystem balance Hugo Mills
@ 2010-11-12  1:33   ` Li Zefan
  2010-11-12  4:28     ` Chris Samuel
  0 siblings, 1 reply; 14+ messages in thread
From: Li Zefan @ 2010-11-12  1:33 UTC (permalink / raw)
  To: Hugo Mills; +Cc: linux-btrfs, Goffredo Baroncelli, Chris Mason, liubo

Hugo Mills wrote:
> This patch adds an ioctl for cancelling a btrfs balance operation
> mid-flight. The ioctl simply sets a flag, and the operation terminates
> after the current block group move has completed.
> 

Is there any blocker that prevents us from canceling balance
by just Ctrl+C ?

> Signed-off-by: Hugo Mills <hugo@carfax.org.uk>
> ---
>  fs/btrfs/ctree.h   |    1 +
>  fs/btrfs/ioctl.c   |   28 ++++++++++++++++++++++++++++
>  fs/btrfs/ioctl.h   |    3 ++-
>  fs/btrfs/volumes.c |    7 ++++++-
>  4 files changed, 37 insertions(+), 2 deletions(-)

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

* Re: [PATCH v2 2/2] Cancel filesystem balance.
  2010-11-12  1:33   ` Li Zefan
@ 2010-11-12  4:28     ` Chris Samuel
  2010-11-12  8:08       ` Helmut Hullen
                         ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Chris Samuel @ 2010-11-12  4:28 UTC (permalink / raw)
  To: Li Zefan; +Cc: Hugo Mills, linux-btrfs, Goffredo Baroncelli, Chris Mason, liubo

On 12/11/10 12:33, Li Zefan wrote:

> Is there any blocker that prevents us from canceling balance
> by just Ctrl+C ?

Given that there's been at least 1 report of it taking 12 hours
to balance a non-trivial amount of data I suspect putting this
operation into the background by default and having the cancel
option might be a better plan.

Thoughts ?

-- 
 Chris Samuel  :  http://www.csamuel.org/  :  Melbourne, VIC

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

* Re: [PATCH v2 2/2] Cancel filesystem balance.
  2010-11-12  4:28     ` Chris Samuel
@ 2010-11-12  8:08       ` Helmut Hullen
  2010-11-12  9:07       ` Sander
  2010-11-12 11:36       ` Hugo Mills
  2 siblings, 0 replies; 14+ messages in thread
From: Helmut Hullen @ 2010-11-12  8:08 UTC (permalink / raw)
  To: linux-btrfs

Hallo, Chris,

Du meintest am 12.11.10:

>> Is there any blocker that prevents us from canceling balance
>> by just Ctrl+C ?

> Given that there's been at least 1 report of it taking 12 hours
> to balance a non-trivial amount of data I suspect putting this
> operation into the background by default and having the cancel
> option might be a better plan.

> Thoughts ?

Here: 17 hours for adding a 1.5-TByte partition with 1 TByte data.  
That's no foreground job. Even monitoring blocks a console for a long  
time.

I've seen log entries in "/var/log/messages" - they don't (didn't) show  
how much work was remaining, but they showed at least that btrfs was  
still working hard.

Nov  9 06:52:35 Arktur kernel: btrfs: relocating block group 1546888675328 flags 1
Nov  9 06:53:20 Arktur kernel: btrfs: relocating block group 1545814933504 flags 1
...
Nov 10 00:14:01 Arktur kernel: btrfs: relocating block group 12582912 flags 1
Nov 10 00:14:06 Arktur kernel: btrfs: relocating block group 4194304 flags 4

Viele Gruesse!
Helmut

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

* Re: [PATCH v2 2/2] Cancel filesystem balance.
  2010-11-12  4:28     ` Chris Samuel
  2010-11-12  8:08       ` Helmut Hullen
@ 2010-11-12  9:07       ` Sander
  2010-11-12  9:26         ` Andreas Philipp
  2010-11-12 11:25         ` Helmut Hullen
  2010-11-12 11:36       ` Hugo Mills
  2 siblings, 2 replies; 14+ messages in thread
From: Sander @ 2010-11-12  9:07 UTC (permalink / raw)
  To: Chris Samuel
  Cc: Li Zefan, Hugo Mills, linux-btrfs, Goffredo Baroncelli,
	Chris Mason, liubo

Chris Samuel wrote (ao):
> On 12/11/10 12:33, Li Zefan wrote:
> 
> > Is there any blocker that prevents us from canceling balance
> > by just Ctrl+C ?
> 
> Given that there's been at least 1 report of it taking 12 hours
> to balance a non-trivial amount of data I suspect putting this
> operation into the background by default and having the cancel
> option might be a better plan.
> 
> Thoughts ?

My humble opinion: I very much like the way mdadm works, with the
progress bar in /proc/mdstat if an array is rebuilding for example.

	Sander

-- 
Humilis IT Services and Solutions
http://www.humilis.net

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

* Re: [PATCH v2 2/2] Cancel filesystem balance.
  2010-11-12  9:07       ` Sander
@ 2010-11-12  9:26         ` Andreas Philipp
  2010-11-12 11:25         ` Helmut Hullen
  1 sibling, 0 replies; 14+ messages in thread
From: Andreas Philipp @ 2010-11-12  9:26 UTC (permalink / raw)
  To: sander
  Cc: Chris Samuel, Li Zefan, Hugo Mills, linux-btrfs,
	Goffredo Baroncelli, Chris Mason, liubo


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
 
On 12.11.2010 10:07, Sander wrote:
> Chris Samuel wrote (ao):
>> On 12/11/10 12:33, Li Zefan wrote:
>>
>>> Is there any blocker that prevents us from canceling balance by
>>> just Ctrl+C ?
>>
>> Given that there's been at least 1 report of it taking 12 hours
>> to balance a non-trivial amount of data I suspect putting this
>> operation into the background by default and having the cancel
>> option might be a better plan.
>>
>> Thoughts ?
>
> My humble opinion: I very much like the way mdadm works, with the
> progress bar in /proc/mdstat if an array is rebuilding for
> example.
>
> Sander
Just my personal opinion: I would like the balance to happen in
background by default (with an option for foreground operation as
well) and another command/option to cancel the operation. Of course, a
progress bar (together with some estimate of the remaining time?) like
the one mdadm offers would be great.

Andreas Philipp
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
 
iQIcBAEBAgAGBQJM3QhMAAoJEJIcBJ3+XkgioZoQAJ5bAD1xShKfda9oPK75x707
0AYu7C4sUSbu/3BINCv7XEv8MNBU++2FezRc/weSXjgmkhX4pXpMBhFZL6edlpot
VQ0y87rJ79HPe1NibiAmp4HOW95J2+R987mOJpIU10xU1TvpWAhpQextUKi27eg+
Z+XO/vm0oCv0+6YSqKnjpUPNd12r5zo7msanPnny5t57oFDXd6LCRjflt5FsP6mI
HKdM3R3EiBdlCyJsBpjDCZmJpUKvOFqoqn2OT/g7BHkE6XjuY0HqUysFLbNtWGIr
zhnx95W+cqgFY1YAvLwbrirtzX8MvO4R83c+klwQJPM9eL3+GkxyMrbnN3uQ4ie+
1wsgVCyyVT6QFLXnVeqo4ZvSNZh2/9S6waL1T0cFj1YAKBnDI3mCPo+S9CcIUuH4
KnOv3bqA3okAy0WUh3FuWNOc9fMX3tEPtE+b9JqDo1BmG0ZdnsGEdbJQxRe1xRFG
CwWsT1efV1tXEqrfsigxAlpMj9PY/uagwEYmhjQG1QU9/yGhXeZYfvE/cdYD5+Nj
chB2CpHISyQSLKAqwvLRF/tmkIjMRWrW2O3j3RGmkyEyrGl7eFKM0//1n1kmkGMc
SEAF3/fn61Wt/4YtA9r1py1Xe1deNhBGVuQQe2M1YMhITV0wSjIWPjmeL0eYu8n7
EsJdFOXoQIxIlkI7/lB+
=FQQ8
-----END PGP SIGNATURE-----


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

* Re: [PATCH v2 2/2] Cancel filesystem balance.
  2010-11-12  9:07       ` Sander
  2010-11-12  9:26         ` Andreas Philipp
@ 2010-11-12 11:25         ` Helmut Hullen
  2010-11-12 12:04           ` Sander
  1 sibling, 1 reply; 14+ messages in thread
From: Helmut Hullen @ 2010-11-12 11:25 UTC (permalink / raw)
  To: linux-btrfs

Hallo, Sander,

Du meintest am 12.11.10:

>> Given that there's been at least 1 report of it taking 12 hours
>> to balance a non-trivial amount of data I suspect putting this
>> operation into the background by default and having the cancel
>> option might be a better plan.
>>
>> Thoughts ?

> My humble opinion: I very much like the way mdadm works, with the
> progress bar in /proc/mdstat if an array is rebuilding for example.

Hmmm - it blocks the console for a long time.

I prefer running those long time jobs via "at"; all messages go into a  
mail to root (or whoever has started the job).

"balance" seems to produce a logfile entry every 40 seconds, that may be  
about 1500 lines for a 1 TByte job.
A progress bar produces similar messages, but they aren't as good  
readable in an e-mail ...

I know those long and nearly unreadable mails p.e. from "squidGuard" ...

Viele Gruesse!
Helmut

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

* Re: [PATCH v2 2/2] Cancel filesystem balance.
  2010-11-12  4:28     ` Chris Samuel
  2010-11-12  8:08       ` Helmut Hullen
  2010-11-12  9:07       ` Sander
@ 2010-11-12 11:36       ` Hugo Mills
  2010-11-12 17:59         ` Hugo Mills
  2 siblings, 1 reply; 14+ messages in thread
From: Hugo Mills @ 2010-11-12 11:36 UTC (permalink / raw)
  To: Chris Samuel
  Cc: Li Zefan, linux-btrfs, Goffredo Baroncelli, Chris Mason, liubo

[-- Attachment #1: Type: text/plain, Size: 938 bytes --]

On Fri, Nov 12, 2010 at 03:28:08PM +1100, Chris Samuel wrote:
> On 12/11/10 12:33, Li Zefan wrote:
> 
> > Is there any blocker that prevents us from canceling balance
> > by just Ctrl+C ?
> 
> Given that there's been at least 1 report of it taking 12 hours
> to balance a non-trivial amount of data I suspect putting this
> operation into the background by default and having the cancel
> option might be a better plan.

   Only 12 hours? Last time I tried it, it took 19. :)

   It would certainly be easy enough to fork a copy of the userspace
tool to run the ioctl in the background. Probably a little more work
to make the balance a kernel thread. I'd prefer the former, for
ease of implementation.

   Hugo.

-- 
=== Hugo Mills: hugo@... carfax.org.uk | darksatanic.net | lug.org.uk ===
  PGP key: 515C238D from wwwkeys.eu.pgp.net or http://www.carfax.org.uk
   --- Great oxymorons of the world, no.  3: Military Intelligence ---   

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

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

* Re: [PATCH v2 2/2] Cancel filesystem balance.
  2010-11-12 11:25         ` Helmut Hullen
@ 2010-11-12 12:04           ` Sander
  0 siblings, 0 replies; 14+ messages in thread
From: Sander @ 2010-11-12 12:04 UTC (permalink / raw)
  To: Helmut Hullen; +Cc: linux-btrfs

Helmut Hullen wrote (ao):
> Du meintest am 12.11.10:
> > My humble opinion: I very much like the way mdadm works, with the
> > progress bar in /proc/mdstat if an array is rebuilding for example.
> 
> Hmmm - it blocks the console for a long time.

Actually, mdadm doesn't block the console as it returns the prompt
immediately :-)

	Sander

-- 
Humilis IT Services and Solutions
http://www.humilis.net

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

* Re: [PATCH v2 2/2] Cancel filesystem balance.
  2010-11-12 11:36       ` Hugo Mills
@ 2010-11-12 17:59         ` Hugo Mills
  0 siblings, 0 replies; 14+ messages in thread
From: Hugo Mills @ 2010-11-12 17:59 UTC (permalink / raw)
  To: Chris Samuel, Li Zefan, linux-btrfs, Goffredo Baroncelli,
	Chris Mason, liubo

[-- Attachment #1: Type: text/plain, Size: 4528 bytes --]

On Fri, Nov 12, 2010 at 11:36:55AM +0000, Hugo Mills wrote:
> On Fri, Nov 12, 2010 at 03:28:08PM +1100, Chris Samuel wrote:
> > On 12/11/10 12:33, Li Zefan wrote:
> > 
> > > Is there any blocker that prevents us from canceling balance
> > > by just Ctrl+C ?
> > 
> > Given that there's been at least 1 report of it taking 12 hours
> > to balance a non-trivial amount of data I suspect putting this
> > operation into the background by default and having the cancel
> > option might be a better plan.
> 
>    Only 12 hours? Last time I tried it, it took 19. :)
> 
>    It would certainly be easy enough to fork a copy of the userspace
> tool to run the ioctl in the background. Probably a little more work
> to make the balance a kernel thread. I'd prefer the former, for
> ease of implementation.

   How's this?


This patch makes a balance operation fork and detach from the current
terminal, to run the userspace side of the balance in the background.

Introduce a --wait switch so that a synchronous balance can be done if
the user requires.

Signed-off-by: Hugo Mills <hugo@carfax.org.uk>
---
 btrfs.c        |    8 ++++----
 btrfs_cmds.c   |   56 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
 man/btrfs.8.in |    2 +-
 3 files changed, 58 insertions(+), 8 deletions(-)

diff --git a/btrfs.c b/btrfs.c
index 93f7886..7b42658 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -91,12 +91,12 @@ static struct Command commands[] = {
 	  "filesystem df", "<path>\n"
 		"Show space usage information for a mount point\n."
 	},
-	{ do_balance, 1,
-	  "filesystem balance", "<path>\n"
+	{ do_balance, -1,
+	  "filesystem balance", "[-w|--wait] <path>\n"
 		"Balance the chunks across the device."
 	},
-	{ do_balance, 1,
-	  "balance start", "<path>\n"
+	{ do_balance, -1,
+	  "balance start", "[-w|--wait] <path>\n"
 		"Synonym for \"btrfs filesystem balance\"."
 	},
 	{ do_balance_progress, -1,
diff --git a/btrfs_cmds.c b/btrfs_cmds.c
index d246a8b..13be603 100644
--- a/btrfs_cmds.c
+++ b/btrfs_cmds.c
@@ -754,12 +754,41 @@ int do_add_volume(int nargs, char **args)
 
 }
 
+const struct option balance_options[] = {
+	{ "wait", 0, NULL, 'w' },
+	{ NULL, 0, NULL, 0 }
+};
+
 int do_balance(int argc, char **argv)
 {
-
 	int	fdmnt, ret=0;
+	int background = 1;
 	struct btrfs_ioctl_vol_args args;
-	char	*path = argv[1];
+	char *path;
+	int ttyfd;
+
+	optind = 1;
+	while(1) {
+		int c = getopt_long(argc, argv, "w", balance_options, NULL);
+		if (c < 0)
+			break;
+		switch(c) {
+		case 'w':
+			background = 0;
+			break;
+		default:
+			fprintf(stderr, "Invalid arguments for balance\n");
+			free(argv);
+			return 1;
+		}
+	}
+
+	if(optind >= argc) {
+		fprintf(stderr, "No filesystem path given for balance\n");
+		return 1;
+	}
+
+	path = argv[optind];
 
 	fdmnt = open_file_or_dir(path);
 	if (fdmnt < 0) {
@@ -767,8 +796,29 @@ int do_balance(int argc, char **argv)
 		return 12;
 	}
 
+	if (background) {
+		int pid = fork();
+		if (pid == 0) {
+			/* We're in the child, and can run in the background */
+			ttyfd = open("/dev/tty", O_RDWR);
+			if (ttyfd > 0)
+				ioctl(ttyfd, TIOCNOTTY, 0);
+			/* Fall through to the BTRFS_IOC_BALANCE ioctl */
+		} else if (pid > 0) {
+			/* We're in the parent, and the fork succeeded */
+			printf("Background balance started\n");
+			return 0;
+		} else {
+			/* We're in the parent, and the fork failed */
+			fprintf(stderr, "ERROR: can't start background process -- %s\n",
+					strerror(errno));
+		}
+	}
+
 	memset(&args, 0, sizeof(args));
-	ret = ioctl(fdmnt, BTRFS_IOC_BALANCE, &args);
+	printf("ioctl\n");
+	sleep(60);
+	/* ret = ioctl(fdmnt, BTRFS_IOC_BALANCE, &args); */
 	close(fdmnt);
 	if(ret<0){
 		fprintf(stderr, "ERROR: balancing '%s'\n", path);
diff --git a/man/btrfs.8.in b/man/btrfs.8.in
index 3f7642e..1410aaa 100644
--- a/man/btrfs.8.in
+++ b/man/btrfs.8.in
@@ -27,7 +27,7 @@ btrfs \- control a btrfs filesystem
 .PP
 \fBbtrfs\fP \fBdevice show\fP\fI <dev>|<label> [<dev>|<label>...]\fP
 .PP
-\fBbtrfs\fP \fBdevice balance\fP\fI <path> \fP
+\fBbtrfs\fP \fBdevice balance\fP [\fB-w\fP|\fB--wait\fP] \fI<path>\fP
 .PP
 \fBbtrfs\fP \fBdevice add\fP\fI <dev> [<dev>..] <path> \fP
 .PP

-- 
=== Hugo Mills: hugo@... carfax.org.uk | darksatanic.net | lug.org.uk ===
  PGP key: 515C238D from wwwkeys.eu.pgp.net or http://www.carfax.org.uk
    --- Turning,  pages turning in the widening bath, / The spine ---    
        cannot bear the humidity. / Books fall apart; the binding        
            cannot hold. / Page 129 is loosed upon the world.            

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

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

* Re: [PATCH v2 0/2] Balance management, kernel side
  2010-11-12  0:36 [PATCH v2 0/2] Balance management, kernel side Hugo Mills
  2010-11-09 22:43 ` [PATCH v2 1/2] Balance progress monitoring Hugo Mills
  2010-11-09 22:52 ` [PATCH v2 2/2] Cancel filesystem balance Hugo Mills
@ 2011-03-20  8:52 ` Andreas Philipp
  2011-03-20 11:37   ` Hugo Mills
  2 siblings, 1 reply; 14+ messages in thread
From: Andreas Philipp @ 2011-03-20  8:52 UTC (permalink / raw)
  To: Hugo Mills; +Cc: linux-btrfs, Goffredo Baroncelli, Chris Mason, liubo


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
 
Since balancing takes a long time I liked the idea of having some
progress counter and the ability to run this long lasting task in
background with another option to cancel it if necessary. So I wanted
to give it a try. Unfortunately, all the patches did not apply on top
of kernel version 2.6.38. Is there a newer version of this patch or
died this idea in the meantime? Of course, I will test any patches.

Thanks,
Andreas Philipp


On 12.11.2010 01:36, Hugo Mills wrote:
> These two patches give a degree of control over balance
> operations. The first makes it possible to get an idea of how much
> work remains to do, by tracking the number of block groups (chunks)
> that need to be moved/rewritten. The second patch allows a running
> balance operation to be cancelled when the current block group has
> been moved.
>
> Since the last version, I've added some more locking (assigning to
> a u64 isn't atomic on non-64-bit architectures). I've not added
> the sysfs bits, as I haven't had a chance to try out Goffredo's
> sysfs code yet. I've also not implemented liubo's suggestion of
> tracking the current block group ID (I'll take that discussion up
> with him separately -- basically it's not a good fit with the
> "polling" method required by this ioctl).
>
> Hugo Mills (2): Balance progress monitoring. Cancel filesystem
> balance.
>
> fs/btrfs/ctree.h | 10 ++++++++ fs/btrfs/disk-io.c | 2 +
> fs/btrfs/ioctl.c | 62
> ++++++++++++++++++++++++++++++++++++++++++++++++ fs/btrfs/ioctl.h
> | 8 ++++++ fs/btrfs/volumes.c | 66
> ++++++++++++++++++++++++++++++++++++++++++++++++++- 5 files
> changed, 146 insertions(+), 2 deletions(-)
>
> -- To unsubscribe from this list: send the line "unsubscribe
> linux-btrfs" in the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
 
iQIcBAEBAgAGBQJNhcBJAAoJEJIcBJ3+Xkgi5usP/iUORDKVdCw6gyzZRRWIYHpj
bTn9zgvyatAtRlwdzxA17XUzx6+r3RPMZPYGSn4tMfatjghfvtPDn9RX+YFTzSAL
OM8fYWfFY36dKYLJk4N2FZ3mDC5tHsU7SCzviqyAb5qlFWVmRXuq0YFQ1TjLQ528
r77BfkbiAVXRc+t9I4BrUHueuK1IPF+XijMzvwfH6iUaX9bZ9woOs8xCqP2MCG7U
3uiTf6Hcfw3mN77hy3zlf180Dh27h47YADPMoPym3J0o/9bjbo1KcBeJ+9TYn7mv
aN5pZWSadszlAPwcfidCNGz8O5+fsIAxfBvF0BHHISIBHU8SwlBrZNx/GzyGENd9
EQduDuvi9eLm2+T9ioKcXz7KqebKs6vt4NR5wXGv7j6vLlaB+LgbH2j0oHj1ZA94
lTwd9bfJBogZCxYUlCsEMKyv/JLY/e183H0DO9pbABrqyZbK5koF0SIZxp90i1Ep
YviSBWVyzr0yERP+qenLMNG5NMXMiCup9fGBd8Upil1hTlnxDqCbpvne2MbjsLsv
CGY2w8PAnfhmGpT9L14o6ExMriHu7OhegMvBATnBv3BI9pd0ev7Titwm9pUBW/X4
0toKMUI0630gTg1klds8ibo0x5BF+0MtE29X/WFhpepxMtrR1e+IOsAy409eTb67
qpo81U/CeaaYJi+gV367
=R7Bg
-----END PGP SIGNATURE-----


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

* Re: [PATCH v2 0/2] Balance management, kernel side
  2011-03-20  8:52 ` [PATCH v2 0/2] Balance management, kernel side Andreas Philipp
@ 2011-03-20 11:37   ` Hugo Mills
  0 siblings, 0 replies; 14+ messages in thread
From: Hugo Mills @ 2011-03-20 11:37 UTC (permalink / raw)
  To: Andreas Philipp; +Cc: linux-btrfs, Goffredo Baroncelli, Chris Mason, liubo

[-- Attachment #1: Type: text/plain, Size: 879 bytes --]

On Sun, Mar 20, 2011 at 09:52:26AM +0100, Andreas Philipp wrote:
> 
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>  
> Since balancing takes a long time I liked the idea of having some
> progress counter and the ability to run this long lasting task in
> background with another option to cancel it if necessary. So I wanted
> to give it a try. Unfortunately, all the patches did not apply on top
> of kernel version 2.6.38. Is there a newer version of this patch or
> died this idea in the meantime? Of course, I will test any patches.

   Odd you should mention that, I'm just redoing my patch stack
against the latest btrfs-unstable...

   Hugo.

-- 
=== Hugo Mills: hugo@... carfax.org.uk | darksatanic.net | lug.org.uk ===
  PGP key: 515C238D from wwwkeys.eu.pgp.net or http://www.carfax.org.uk
     --- IMPROVE YOUR ORGANISMS!!  -- Subject line of spam email ---     

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

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

end of thread, other threads:[~2011-03-20 11:37 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-12  0:36 [PATCH v2 0/2] Balance management, kernel side Hugo Mills
2010-11-09 22:43 ` [PATCH v2 1/2] Balance progress monitoring Hugo Mills
2010-11-09 22:52 ` [PATCH v2 2/2] Cancel filesystem balance Hugo Mills
2010-11-12  1:33   ` Li Zefan
2010-11-12  4:28     ` Chris Samuel
2010-11-12  8:08       ` Helmut Hullen
2010-11-12  9:07       ` Sander
2010-11-12  9:26         ` Andreas Philipp
2010-11-12 11:25         ` Helmut Hullen
2010-11-12 12:04           ` Sander
2010-11-12 11:36       ` Hugo Mills
2010-11-12 17:59         ` Hugo Mills
2011-03-20  8:52 ` [PATCH v2 0/2] Balance management, kernel side Andreas Philipp
2011-03-20 11:37   ` Hugo Mills

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).