* [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* [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
* 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 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 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: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).