Re: [PATCH] Btrfs: merge inode_list in __merge_refs
On Thu, Nov 08, 2012 at 10:35:19PM +0100, Alexander Block wrote: > > Used wrong CC for stable list. Corrected now. This is not the correct way to submit patches for inclusion in the stable kernel tree. Please read Documentation/stable_kernel_rules.txt for how to do this properly. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How does btrfs handle sudden shutdowns?
Michael Kjörling kjorling.se> writes: > > Can btrfs deal reasonably gracefully with sudden shutdowns? (I'm > mainly thinking of power outages which lead to logical structure > damage but not physical media damage.) > Really rather well! We've had a sequence of power-cuts around here and I've scrubbed each time, finding only one corruption over all which was fixed by the scrub and no data lost. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 15/26] Btrfs: add a new source file with device replace code
On Thu, Nov 08, 2012 at 06:24:36PM +0100, Stefan Behrens wrote: > On Thu, 8 Nov 2012 22:50:47 +0800, Liu Bo wrote: > > On Tue, Nov 06, 2012 at 05:38:33PM +0100, Stefan Behrens wrote: [...] > >> + btrfs_dev_replace_unlock(dev_replace); > >> + > >> + btrfs_wait_ordered_extents(root, 0); > >> + > >> + /* force writing the updated state information to disk */ > >> + trans = btrfs_start_transaction(root, 0); > > > > why a start_transaction here? Any reasons? > > (same question also for some other places) > > > > Without this transaction, there is outstanding I/O which is not flushed. > Pending writes that go only to the old disk need to be flushed before > the mode is switched to write all live data to the source disk and to > the target disk as well. The copy operation that is part of the scrub > code works on the commit root for performance reasons. Every write > request that is performed after the commit root is established needs to > go to both disks. Those requests that already have the bdev assigned > (i.e., btrfs_map_bio() was already called) cannot be duplicated anymore > to write to the new disk as well. > > btrfs_dev_replace_finishing() looks similar and goes through a > transaction commit between the steps where the bdev in the mapping tree > is swapped and the step when the old bdev is freed. Otherwise the bdev > would be accessed after being freed. > I see, if you're only about to flush metadata, why not join a transaction? thanks, liubo -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Btrfs-progs: disable qgroupid 0 for quota_tree
On 08/11/12 20:55, Miao Xie wrote: > In kernel, qgroupid 0 is a special number when we run the quota group > limit command. > > So, we should not be able to create a quota group whose id is 0, > otherwise the kernel can't deal with it. Fix it. This is probably a stupid question - but if its not meant to be possible to create such a thing shouldn't this be fixed in the kernel (as well as here) to reject attempts from user space to create it? Otherwise it's possible for a non-aware program (or a user who is playing) to still create it. cheers, Chris -- Chris Samuel : http://www.csamuel.org/ : Melbourne, VIC -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Btrfs: merge inode_list in __merge_refs
On Thu, Nov 8, 2012 at 10:27 PM, Alexander Block
wrote:
> When __merge_refs merges two refs, it is also needed to merge the
> inode_list of both refs. Otherwise we have missed backrefs and memory
> leaks. This happens for example if two inodes share an extent and
> both lie in the same leaf and thus also have the same parent.
>
> Signed-off-by: Alexander Block
> Reviewed-by: Jan Schmidt
> ---
> fs/btrfs/backref.c | 13 +++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> index 208d8aa..aea6d2d 100644
> --- a/fs/btrfs/backref.c
> +++ b/fs/btrfs/backref.c
> @@ -461,6 +461,7 @@ static int __merge_refs(struct list_head *head, int mode)
> pos2 = n2, n2 = pos2->next) {
> struct __prelim_ref *ref2;
> struct __prelim_ref *xchg;
> + struct extent_inode_elem *eie;
>
> ref2 = list_entry(pos2, struct __prelim_ref, list);
>
> @@ -472,12 +473,20 @@ static int __merge_refs(struct list_head *head, int
> mode)
> ref1 = ref2;
> ref2 = xchg;
> }
> - ref1->count += ref2->count;
> } else {
> if (ref1->parent != ref2->parent)
> continue;
> - ref1->count += ref2->count;
> }
> +
> + eie = ref1->inode_list;
> + while (eie && eie->next)
> + eie = eie->next;
> + if (eie)
> + eie->next = ref2->inode_list;
> + else
> + ref1->inode_list = ref2->inode_list;
> + ref1->count += ref2->count;
> +
> list_del(&ref2->list);
> kfree(ref2);
> }
> --
> 1.7.10.4
>
Used wrong CC for stable list. Corrected now.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Btrfs: merge inode_list in __merge_refs
When __merge_refs merges two refs, it is also needed to merge the
inode_list of both refs. Otherwise we have missed backrefs and memory
leaks. This happens for example if two inodes share an extent and
both lie in the same leaf and thus also have the same parent.
Signed-off-by: Alexander Block
Reviewed-by: Jan Schmidt
---
fs/btrfs/backref.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index 208d8aa..aea6d2d 100644
--- a/fs/btrfs/backref.c
+++ b/fs/btrfs/backref.c
@@ -461,6 +461,7 @@ static int __merge_refs(struct list_head *head, int mode)
pos2 = n2, n2 = pos2->next) {
struct __prelim_ref *ref2;
struct __prelim_ref *xchg;
+ struct extent_inode_elem *eie;
ref2 = list_entry(pos2, struct __prelim_ref, list);
@@ -472,12 +473,20 @@ static int __merge_refs(struct list_head *head, int mode)
ref1 = ref2;
ref2 = xchg;
}
- ref1->count += ref2->count;
} else {
if (ref1->parent != ref2->parent)
continue;
- ref1->count += ref2->count;
}
+
+ eie = ref1->inode_list;
+ while (eie && eie->next)
+ eie = eie->next;
+ if (eie)
+ eie->next = ref2->inode_list;
+ else
+ ref1->inode_list = ref2->inode_list;
+ ref1->count += ref2->count;
+
list_del(&ref2->list);
kfree(ref2);
}
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/26] Btrfs: Add device replace code
On 11/08/2012 06:31 PM, Stefan Behrens wrote:
> On Thu, 8 Nov 2012 13:50:19 +0100, Goffredo Baroncelli wrote:
[...]
>> I think that so "replace" would be the natural extension to the "add"
>> and "delete" subcommands.
>
> "btrfs device replace "
> was also my first idea. It used to be like this initially.
>
> "btrfs device replace cancel "
> was the point when I gave up putting it below the "device" commands. IMO
> that's just too long, too much to type.
>
> Now it has the same look and feel as the "scrub" commands ("scrub
> start", "scrub status" and "scrub cancel").
Yes, but scrub was a a new command. Instead I see "replace" as an
extension of "btrfs device add/del" (from an user interface POV).
If someone would extend the "btrfs device delete" command to support
status/pause/resume, how could do it ?
May be we need a new series of command which handle the "background
process" (like btrfs replace, btrfs device delete, btrfs subvolume
delete) to status/stop/suspend/resume these processes ?
I am doing a bit of brain-storming...
>
>
--
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Btrfs Slow Down (Metadata Starvation?)
One of my Btrfs partitions ran into a severe slowdown recently. Operations that would normally complete in 20-30 seconds were now requiring hours. There were no errors or warnings in dmesg (Alt-SysRq-W is below, but shows nothing out of the ordinary). And if I took the partition offline, it would pass btrfsck without error. So far, I've found not indications of corruption. The kernel is a version 3.6.6 merged with the for-linus branch for 3.7. I usually mount with compress-force-lzo, but no autodefrag or other options. The symptoms were consistent with some kind of corner case metadata starvation. While under pressure, my 'btrfs df' would show something like the following: # btrfs fi df /mnt/sabayon9/ Data: total=7.00GB, used=6.00GB System: total=4.00MB, used=4.00KB Metadata: total=768.00MB, used=737.65MB For some reason, btrfs was not allocating any additional metadata space. The partition is 25 GB, and not very full: /dev/sda2 btrfs 25165824 7047816 18082844 29% /mnt/sabayon9 When I rebooted into a 3.4 kernel (which is merged with the Btrfs code for 3.5), the slow down cleared after I mounted the partition, and triggered an allocation of metadata up to 1 GB. I would note that I tried my 3.5 vintage kernel (which is merged with the Btrfs code for 3.6), and was unable to clear the issue. This tends to strengthen my suspicion that this is some kind of corner case since this code has been out there for a while now. Currently, I'm showing something like this with 'btrfs df' # btrfs fi df /mnt/sabayon9/ Data: total=8.00GB, used=5.97GB System: total=4.00MB, used=4.00KB Metadata: total=1.00GB, used=722.65MB Now, everything is operating normally in my 3.6.6 kernel. I've saved an image of the partition in it's 'slow-down' condition in case it becomes desirable to test something in that condition. I'm including my dmesg output of an Alt-SysRq-W operation, but I don't see anything useful there. [18697.498504] SysRq : Show Blocked State [18697.498510] taskPC stack pid father [18697.498551] btrfs-submit-1 D 0210 0 4236 2 0x [18697.498556] 880123d53b70 0046 8801231908d0 880123d53fd8 [18697.498560] 4000 00012700 88012aaf4380 880124269680 [18697.498563] 0006 880125e18000 880125e18000 [18697.498567] Call Trace: [18697.498576] [] ? __blk_run_queue+0x1e/0x20 [18697.498580] [] ? queue_unplugged+0x83/0x99 [18697.498585] [] schedule+0x64/0x66 [18697.498588] [] io_schedule+0x8f/0xce [18697.498591] [] get_request+0x559/0x5b0 [18697.498596] [] ? abort_exclusive_wait+0x8e/0x8e [18697.498599] [] blk_queue_bio+0x1b7/0x315 [18697.498602] [] generic_make_request+0x9f/0xe1 [18697.498605] [] submit_bio+0xe4/0x103 [18697.498640] [] run_scheduled_bios+0x28c/0x428 [btrfs] [18697.498660] [] pending_bios_fn+0x15/0x17 [btrfs] [18697.498679] [] worker_loop+0x15f/0x497 [btrfs] [18697.498698] [] ? btrfs_queue_worker+0x272/0x272 [btrfs] [18697.498702] [] kthread+0x8b/0x93 [18697.498707] [] kernel_thread_helper+0x4/0x10 [18697.498710] [] ? kthread_freezable_should_stop+0x57/0x57 [18697.498714] [] ? gs_change+0xb/0xb [18697.498718] btrfs-transacti D 0002 0 4248 2 0x [18697.498721] 880123d83b70 0046 880123d83fd8 [18697.498725] 4000 00012700 88012aaf4380 880123d79680 [18697.498728] a0049342 880123d83bf0 880123d83b10 810c9ee4 [18697.498732] Call Trace: [18697.498748] [] ? check_leaf+0x2d4/0x2d4 [btrfs] [18697.498753] [] ? release_pages+0x1b2/0x1c1 [18697.498772] [] ? submit_one_bio+0x8a/0x94 [btrfs] [18697.498776] [] ? ktime_get_ts+0x56/0xbc [18697.498780] [] ? delayacct_end+0x79/0x84 [18697.498784] [] ? __lock_page+0x68/0x68 [18697.498787] [] schedule+0x64/0x66 [18697.498790] [] io_schedule+0x8f/0xce [18697.498793] [] sleep_on_page+0xe/0x12 [18697.498796] [] __wait_on_bit+0x48/0x7b [18697.498799] [] ? find_get_pages_tag+0xf4/0x130 [18697.498803] [] wait_on_page_bit+0x72/0x74 [18697.498806] [] ? autoremove_wake_function+0x38/0x38 [18697.498810] [] filemap_fdatawait_range+0x87/0x13e [18697.498829] [] ? free_extent_state+0x7d/0x85 [btrfs] [18697.498849] [] ? clear_extent_bit+0x272/0x2aa [btrfs] [18697.498866] [] btrfs_wait_marked_extents+0x7d/0xce [btrfs] [18697.498884] [] btrfs_write_and_wait_marked_extents+0x2e/0x3e [btrfs] [18697.498902] [] btrfs_write_and_wait_transaction+0x44/0x46 [btrfs] [18697.498919] [] btrfs_commit_transaction+0x65b/0x969 [btrfs] [18697.498923] [] ? abort_exclusive_wait+0x8e/0x8e [18697.498939] [] transaction_kthread+0xdf/0x1a0 [btrfs] [18697.498956] [] ? cleaner_kthread+0xe6/0xe6 [btrfs] [18697.498959] [] kthread+0x8b/0x93 [18697.498963] [] kernel_thread_helper+0x4/0x10 [18697.498967] [] ? kthread_freezable_should_stop+0x57/0x57 [18697.498970] [] ? gs_change+0xb/0xb [18697.498987] cp
Re: [PATCH 00/26] Btrfs: Add device replace code
On Thu, 8 Nov 2012 13:50:19 +0100, Goffredo Baroncelli wrote:
> great work. However I have a suggestion: what about putting all the
> command under 'device' sub commands: something like:
>
> - btrfs device replace
>
> - btrfs device status
>
> Where "btrfs device status" would show only the status of the
> "replacing" operation; but in the future it could show also the status
> of the "delete" command (which it is the only other "device
> sub-command" which needs time to complete).
>
> Of course I am not asking to complete the "btrfs device status" part
> for the "btrfs device delete" command. This could be implemented in a
> second time.
>
> I think that so "replace" would be the natral extension to the "add"
> and "delete" subcommands.
"btrfs device replace "
was also my first idea. It used to be like this initially.
"btrfs device replace cancel "
was the point when I gave up putting it below the "device" commands. IMO
that's just too long, too much to type.
Now it has the same look and feel as the "scrub" commands ("scrub
start", "scrub status" and "scrub cancel").
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 15/26] Btrfs: add a new source file with device replace code
On Thu, 8 Nov 2012 22:50:47 +0800, Liu Bo wrote:
> On Tue, Nov 06, 2012 at 05:38:33PM +0100, Stefan Behrens wrote:
>> +out:
>> +if (path) {
>> +btrfs_release_path(path);
>> +btrfs_free_path(path);
>
> btrfs_free_path(path) will do release for you :)
>
Right :) Thanks.
>> +int btrfs_dev_replace_start(struct btrfs_root *root,
>> +struct btrfs_ioctl_dev_replace_args *args)
>> +{
>> +struct btrfs_trans_handle *trans;
>> +struct btrfs_fs_info *fs_info = root->fs_info;
>> +struct btrfs_dev_replace *dev_replace = &fs_info->dev_replace;
>> +int ret;
>> +struct btrfs_device *tgt_device = NULL;
>> +struct btrfs_device *src_device = NULL;
>> +
>> +switch (args->start.cont_reading_from_srcdev_mode) {
>> +case BTRFS_IOCTL_DEV_REPLACE_CONT_READING_FROM_SRCDEV_MODE_ALWAYS:
>> +case BTRFS_IOCTL_DEV_REPLACE_CONT_READING_FROM_SRCDEV_MODE_AVOID:
>> +break;
>> +default:
>> +return -EINVAL;
>> +}
>> +
>> +if ((args->start.srcdevid == 0 && args->start.srcdev_name[0] == '\0') ||
>> +args->start.tgtdev_name[0] == '\0')
>> +return -EINVAL;
>> +
>> +mutex_lock(&fs_info->volume_mutex);
>> +ret = btrfs_init_dev_replace_tgtdev(root, args->start.tgtdev_name,
>> +&tgt_device);
>> +if (ret) {
>> +pr_err("btrfs: target device %s is invalid!\n",
>> + args->start.tgtdev_name);
>> +mutex_unlock(&fs_info->volume_mutex);
>> +return -EINVAL;
>> +}
>> +
>> +ret = btrfs_dev_replace_find_srcdev(root, args->start.srcdevid,
>> +args->start.srcdev_name,
>> +&src_device);
>> +mutex_unlock(&fs_info->volume_mutex);
>> +if (ret) {
>> +ret = -EINVAL;
>> +goto leave_no_lock;
>> +}
>> +
>> +if (tgt_device->total_bytes < src_device->total_bytes) {
>> +pr_err("btrfs: target device is smaller than source device!\n");
>> +ret = -EINVAL;
>> +goto leave_no_lock;
>> +}
>> +
>> +btrfs_dev_replace_lock(dev_replace);
>> +switch (dev_replace->replace_state) {
>> +case BTRFS_IOCTL_DEV_REPLACE_STATE_NEVER_STARTED:
>> +case BTRFS_IOCTL_DEV_REPLACE_STATE_FINISHED:
>> +case BTRFS_IOCTL_DEV_REPLACE_STATE_CANCELED:
>> +break;
>> +case BTRFS_IOCTL_DEV_REPLACE_STATE_STARTED:
>> +case BTRFS_IOCTL_DEV_REPLACE_STATE_SUSPENDED:
>> +args->result = BTRFS_IOCTL_DEV_REPLACE_RESULT_ALREADY_STARTED;
>> +goto leave;
>> +}
>> +
>> +dev_replace->cont_reading_from_srcdev_mode =
>> +args->start.cont_reading_from_srcdev_mode;
>> +WARN_ON(!src_device);
>> +dev_replace->srcdev = src_device;
>> +WARN_ON(!tgt_device);
>> +dev_replace->tgtdev = tgt_device;
>> +
>> +tgt_device->total_bytes = src_device->total_bytes;
>> +tgt_device->disk_total_bytes = src_device->disk_total_bytes;
>> +tgt_device->bytes_used = src_device->bytes_used;
>> +
>> +/*
>> + * from now on, the writes to the srcdev are all duplicated to
>> + * go to the tgtdev as well (refer to btrfs_map_block()).
>> + */
>> +dev_replace->replace_state = BTRFS_IOCTL_DEV_REPLACE_STATE_STARTED;
>> +dev_replace->time_started = btrfs_get_seconds_since_1970();
>> +dev_replace->cursor_left = 0;
>> +dev_replace->committed_cursor_left = 0;
>> +dev_replace->cursor_left_last_write_of_item = 0;
>> +dev_replace->cursor_right = 0;
>> +dev_replace->is_valid = 1;
>> +dev_replace->item_needs_writeback = 1;
>> +args->result = BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_ERROR;
>> +btrfs_dev_replace_unlock(dev_replace);
>> +
>> +btrfs_wait_ordered_extents(root, 0);
>> +
>> +/* force writing the updated state information to disk */
>> +trans = btrfs_start_transaction(root, 0);
>
> why a start_transaction here? Any reasons?
> (same question also for some other places)
>
Without this transaction, there is outstanding I/O which is not flushed.
Pending writes that go only to the old disk need to be flushed before
the mode is switched to write all live data to the source disk and to
the target disk as well. The copy operation that is part of the scrub
code works on the commit root for performance reasons. Every write
request that is performed after the commit root is established needs to
go to both disks. Those requests that already have the bdev assigned
(i.e., btrfs_map_bio() was already called) cannot be duplicated anymore
to write to the new disk as well.
btrfs_dev_replace_finishing() looks similar and goes through a
transaction commit between the steps where the bdev in the mapping tree
is swapped and the step when the old bdev is freed. Otherwise the bdev
would be accessed after being freed.
>> +if (IS_ERR(trans)) {
>> +ret = PTR_ERR(trans);
>> +
question about btrfsck/mount behavior on corrupted fs
Hi all, I've been playing with btrfs resize recently and run into strange looking behavior to me. One of my simple test scenario was following: - partition some block device (lets say sda sectors 2-2000 are sda1) - try to create btrfs on top of it (just mkfs.btrfs /dev/sda1) - fill the fs with data - unmount the device - let's simulate usual mistake here: downsize the partition with btrfs filesystem w/o proper btrfs resize command. In fact I moved the beginning of the sda2 partition into middle of former sda1. so now it looks like: 2-1000 sda1, 1001-2000 sda2. - remount the fs I was surprised both commands btrfsck and mount were successful w/o any error report even in syslog. Does it work as intended? Do you plan to support some sort of checks during mount/btrfsck? Possibly a new feature in btrfsck utility to warn the user something ugly happened to his fs? Is there any way to fix broken fs like this (supposing the cut off area didn't contain allocated blocks)? Kind regards Ondrej -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 15/26] Btrfs: add a new source file with device replace code
On Tue, Nov 06, 2012 at 05:38:33PM +0100, Stefan Behrens wrote:
> This adds a new file to the sources together with the header file
> and the changes to ioctl.h that are required by the new C source
> file.
>
> Signed-off-by: Stefan Behrens
> ---
> fs/btrfs/dev-replace.c | 843
> +
> fs/btrfs/dev-replace.h | 44 +++
> fs/btrfs/ioctl.h | 45 +++
> 3 files changed, 932 insertions(+)
>
> diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
> new file mode 100644
> index 000..1d56163
> --- /dev/null
> +++ b/fs/btrfs/dev-replace.c
> @@ -0,0 +1,843 @@
> +/*
> + * Copyright (C) STRATO AG 2012. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public
> + * License v2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public
> + * License along with this program; if not, write to the
> + * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
> + * Boston, MA 021110-1307, USA.
> + */
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include "compat.h"
> +#include "ctree.h"
> +#include "extent_map.h"
> +#include "disk-io.h"
> +#include "transaction.h"
> +#include "print-tree.h"
> +#include "volumes.h"
> +#include "async-thread.h"
> +#include "check-integrity.h"
> +#include "rcu-string.h"
> +#include "dev-replace.h"
> +
> +static u64 btrfs_get_seconds_since_1970(void);
> +static int btrfs_dev_replace_finishing(struct btrfs_fs_info *fs_info,
> +int scrub_ret);
> +static void btrfs_dev_replace_update_device_in_mapping_tree(
> + struct btrfs_fs_info *fs_info,
> + struct btrfs_device *srcdev,
> + struct btrfs_device *tgtdev);
> +static int btrfs_dev_replace_find_srcdev(struct btrfs_root *root, u64
> srcdevid,
> + char *srcdev_name,
> + struct btrfs_device **device);
> +static u64 __btrfs_dev_replace_cancel(struct btrfs_fs_info *fs_info);
> +static int btrfs_dev_replace_kthread(void *data);
> +static int btrfs_dev_replace_continue_on_mount(struct btrfs_fs_info
> *fs_info);
> +
> +
> +int btrfs_init_dev_replace(struct btrfs_fs_info *fs_info)
> +{
> + struct btrfs_key key;
> + struct btrfs_root *dev_root = fs_info->dev_root;
> + struct btrfs_dev_replace *dev_replace = &fs_info->dev_replace;
> + struct extent_buffer *eb;
> + int slot;
> + int ret = 0;
> + struct btrfs_path *path = NULL;
> + int item_size;
> + struct btrfs_dev_replace_item *ptr;
> + u64 src_devid;
> +
> + path = btrfs_alloc_path();
> + if (!path) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + key.objectid = 0;
> + key.type = BTRFS_DEV_REPLACE_KEY;
> + key.offset = 0;
> + ret = btrfs_search_slot(NULL, dev_root, &key, path, 0, 0);
> + if (ret) {
> +no_valid_dev_replace_entry_found:
> + ret = 0;
> + dev_replace->replace_state =
> + BTRFS_DEV_REPLACE_ITEM_STATE_NEVER_STARTED;
> + dev_replace->cont_reading_from_srcdev_mode =
> + BTRFS_DEV_REPLACE_ITEM_CONT_READING_FROM_SRCDEV_MODE_ALWAYS;
> + dev_replace->replace_state = 0;
> + dev_replace->time_started = 0;
> + dev_replace->time_stopped = 0;
> + atomic64_set(&dev_replace->num_write_errors, 0);
> + atomic64_set(&dev_replace->num_uncorrectable_read_errors, 0);
> + dev_replace->cursor_left = 0;
> + dev_replace->committed_cursor_left = 0;
> + dev_replace->cursor_left_last_write_of_item = 0;
> + dev_replace->cursor_right = 0;
> + dev_replace->srcdev = NULL;
> + dev_replace->tgtdev = NULL;
> + dev_replace->is_valid = 0;
> + dev_replace->item_needs_writeback = 0;
> + goto out;
> + }
> + slot = path->slots[0];
> + eb = path->nodes[0];
> + item_size = btrfs_item_size_nr(eb, slot);
> + ptr = btrfs_item_ptr(eb, slot, struct btrfs_dev_replace_item);
> +
> + if (item_size != sizeof(struct btrfs_dev_replace_item)) {
> + pr_warn("btrfs: dev_replace entry found has unexpected size,
> ignore entry\n");
> + goto no_valid_dev_replace_entry_found;
> + }
> +
> + src_devid = btrfs_dev_replace_src_devid(eb, ptr);
> + dev
Re: [PATCH 07/26] Btrfs: add two more find_device() methods
On Tue, Nov 06, 2012 at 05:38:25PM +0100, Stefan Behrens wrote:
> The new function btrfs_find_device_missing_or_by_path() will be
> used for the device replace procedure. This function itself calls
> the second new function btrfs_find_device_by_path().
> Unfortunately, it is not possible to currently make the rest of the
> code use these functions as well, since all functions that look
> similar at first view are all a little bit different in what they
> are doing. But in the future, new code could benefit from these
> two new functions, and currently, device replace uses them.
>
> Signed-off-by: Stefan Behrens
> ---
> fs/btrfs/volumes.c | 74
> ++
> fs/btrfs/volumes.h | 5
> 2 files changed, 79 insertions(+)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index eeed97d..bcd3097 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1512,6 +1512,80 @@ error_undo:
> goto error_brelse;
> }
>
> +int btrfs_find_device_by_path(struct btrfs_root *root, char *device_path,
> + struct btrfs_device **device)
> +{
> + int ret = 0;
> + struct btrfs_super_block *disk_super;
> + u64 devid;
> + u8 *dev_uuid;
> + struct block_device *bdev;
> + struct buffer_head *bh = NULL;
> +
> + *device = NULL;
> + mutex_lock(&uuid_mutex);
Since the caller have held volume_mutex, we can get rid of the
mutex_lock here, can't we?
> + bdev = blkdev_get_by_path(device_path, FMODE_READ,
> + root->fs_info->bdev_holder);
> + if (IS_ERR(bdev)) {
> + ret = PTR_ERR(bdev);
> + bdev = NULL;
> + goto out;
> + }
> +
> + set_blocksize(bdev, 4096);
> + invalidate_bdev(bdev);
> + bh = btrfs_read_dev_super(bdev);
> + if (!bh) {
> + ret = -EINVAL;
> + goto out;
> + }
I made a scan for this 'get bdev & bh' in the code, I think
maybe we can make a function,
func_get(device_path, flags, mode, &bdev, &bh, flush)
where we need to take care of setting bdev = NULL, bh = NULL, and
'flush' is for filemap_bdev(). Besides, we also need to make
some proper error handling.
thanks,
liubo
> + disk_super = (struct btrfs_super_block *)bh->b_data;
> + devid = btrfs_stack_device_id(&disk_super->dev_item);
> + dev_uuid = disk_super->dev_item.uuid;
> + *device = btrfs_find_device(root, devid, dev_uuid,
> + disk_super->fsid);
> + brelse(bh);
> + if (!*device)
> + ret = -ENOENT;
> +out:
> + mutex_unlock(&uuid_mutex);
> + if (bdev)
> + blkdev_put(bdev, FMODE_READ);
> + return ret;
> +}
> +
> +int btrfs_find_device_missing_or_by_path(struct btrfs_root *root,
> + char *device_path,
> + struct btrfs_device **device)
> +{
> + *device = NULL;
> + if (strcmp(device_path, "missing") == 0) {
> + struct list_head *devices;
> + struct btrfs_device *tmp;
> +
> + devices = &root->fs_info->fs_devices->devices;
> + /*
> + * It is safe to read the devices since the volume_mutex
> + * is held by the caller.
> + */
> + list_for_each_entry(tmp, devices, dev_list) {
> + if (tmp->in_fs_metadata && !tmp->bdev) {
> + *device = tmp;
> + break;
> + }
> + }
> +
> + if (!*device) {
> + pr_err("btrfs: no missing device found\n");
> + return -ENOENT;
> + }
> +
> + return 0;
> + } else {
> + return btrfs_find_device_by_path(root, device_path, device);
> + }
> +}
> +
> /*
> * does all the dirty work required for changing file system's UUID.
> */
> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
> index 1789cda..657bb12 100644
> --- a/fs/btrfs/volumes.h
> +++ b/fs/btrfs/volumes.h
> @@ -268,6 +268,11 @@ int btrfs_scan_one_device(const char *path, fmode_t
> flags, void *holder,
> struct btrfs_fs_devices **fs_devices_ret);
> int btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
> void btrfs_close_extra_devices(struct btrfs_fs_devices *fs_devices);
> +int btrfs_find_device_missing_or_by_path(struct btrfs_root *root,
> + char *device_path,
> + struct btrfs_device **device);
> +int btrfs_find_device_by_path(struct btrfs_root *root, char *device_path,
> + struct btrfs_device **device);
> int btrfs_add_device(struct btrfs_trans_handle *trans,
>struct btrfs_root *root,
>struct btrfs_device *device);
> --
> 1.8.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btr
Re: [PATCH 00/26] Btrfs: Add device replace code
Hi Stefan, great work. However I have a suggestion: what about putting all the command under 'device' sub commands: something like: - btrfs device replace - btrfs device status Where "btrfs device status" would show only the status of the "replacing" operation; but in the future it could show also the status of the "delete" command (which it is the only other "device sub-command" which needs time to complete). Of course I am not asking to complete the "btrfs device status" part for the "btrfs device delete" command. This could be implemented in a second time. I think that so "replace" would be the natral extension to the "add" and "delete" subcommands. BR G.Baroncelli On Wed, Nov 7, 2012 at 2:12 PM, Stefan Behrens wrote: > On Wed, 07 Nov 2012 11:14:36 +0900, Tsutomu Itoh wrote: >> (2012/11/07 1:38), Stefan Behrens wrote: >>> replace start [-Bfr] | >> >> I think that >> "btrfs replace start [-Bfr] | " >> of the same synopsis as other subcommands is better. > > You are right. 'btrfs device add' and 'btrfs device delete' have the > path at the end and the device in front, and there is no good reason to > not make it look equally. I will change it accordingly and post a V2 of > the Btrfs-progs patch within the next few days. > > Thanks! > > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: fs/btrfs/dev-replace.c:486:17: sparse: incompatible types in comparison expression (different address spaces)
> Hi Fengguang,
Hi Stefan!
> Assuming that your script performs a periodic git fetch and git reset,
> and then starts compile runs on different architectures, the only
> explanation that I have is that something went wrong with the git
> operation in your script. It looks like some C source files have been
> updated, but the header files are old. Some kind of inconsistency that
> either a git reset --hard should fix, or rm -rf . and git clone to start
> from the beginning.
>
> Could you check it please?
That's definitely good suggestion! I've added the check before each make
to make sure the tree is in a clean state:
git_check_reset()
{
[[ $(git --no-pager diff) ]] && {
notice "unexpected modified work tree"
dump_stack
git --no-pager diff
git reset --hard
}
}
> And could you please point me to some documentation, how this (very
> useful !) service from Intel is working?
Sure (and thanks!). Here is a nice article:
KS2012: Kernel build/boot testing
https://lwn.net/Articles/514278/
Thanks,
Fengguang
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: fs/btrfs/dev-replace.c:486:17: sparse: incompatible types in comparison expression (different address spaces)
On Thu, 8 Nov 2012 15:35:41 +0800, Fengguang Wu wrote:
> Hi Stefan,
>
> FYI, there are new sparse warnings show up in
>
> tree: git://git.kernel.org/pub/scm/linux/kernel/git/josef/btrfs-next.git
> master
> head: c1014be59ba93855c31fda9d9cf4319cc6f9eeb1
> commit: d8e784f51e2e1d1c57f091fdb49456c4e7fb62d2 Btrfs: add a new source file
> with device replace code
> date: 21 hours ago
>
> + fs/btrfs/dev-replace.c:486:17: sparse: incompatible types in comparison
> expression (different address spaces)
> + fs/btrfs/dev-replace.c:486:17: sparse: incompatible types in comparison
> expression (different address spaces)
> fs/btrfs/dev-replace.c:745:17: sparse: incompatible types in comparison
> expression (different address spaces)
> fs/btrfs/dev-replace.c:745:17: sparse: incompatible types in comparison
> expression (different address spaces)
> fs/btrfs/dev-replace.c:766:30: sparse: too many arguments for function
> btrfs_scrub_dev
> fs/btrfs/dev-replace.c:407:30: sparse: too many arguments for function
> btrfs_scrub_dev
> fs/btrfs/dev-replace.c: In function 'btrfs_init_dev_replace':
> fs/btrfs/dev-replace.c:141:8: error: 'BTRFS_DEV_REPLACE_DEVID' undeclared
> (first use in this function)
> fs/btrfs/dev-replace.c:141:8: note: each undeclared identifier is reported
> only once for each function it appears in
> fs/btrfs/dev-replace.c:168:23: error: 'struct btrfs_device' has no member
> named 'is_tgtdev_for_dev_replace'
> fs/btrfs/dev-replace.c:169:4: error: implicit declaration of function
> 'btrfs_init_dev_replace_tgtdev_for_resume'
> [-Werror=implicit-function-declaration]
> fs/btrfs/dev-replace.c: In function 'btrfs_dev_replace_start':
> fs/btrfs/dev-replace.c:331:2: error: implicit declaration of function
> 'btrfs_init_dev_replace_tgtdev' [-Werror=implicit-function-declaration]
> fs/btrfs/dev-replace.c:409:10: error: too many arguments to function
> 'btrfs_scrub_dev'
> In file included from fs/btrfs/dev-replace.c:30:0:
> fs/btrfs/ctree.h:3648:5: note: declared here
> fs/btrfs/dev-replace.c:422:3: error: implicit declaration of function
> 'btrfs_destroy_dev_replace_tgtdev' [-Werror=implicit-function-declaration]
> fs/btrfs/dev-replace.c: In function 'btrfs_dev_replace_finishing':
> fs/btrfs/dev-replace.c:500:12: error: 'struct btrfs_device' has no member
> named 'is_tgtdev_for_dev_replace'
> fs/btrfs/dev-replace.c:502:22: error: 'BTRFS_DEV_REPLACE_DEVID' undeclared
> (first use in this function)
> fs/btrfs/dev-replace.c:516:2: error: implicit declaration of function
> 'btrfs_rm_dev_replace_srcdev' [-Werror=implicit-function-declaration]
> fs/btrfs/dev-replace.c: In function 'btrfs_resume_dev_replace_async':
> fs/btrfs/dev-replace.c:726:2: error: 'struct btrfs_fs_info' has no member
> named 'mutually_exclusive_operation_running'
> fs/btrfs/dev-replace.c: In function 'btrfs_dev_replace_kthread':
> fs/btrfs/dev-replace.c:756:21: error: 'struct btrfs_fs_info' has no member
> named 'mutually_exclusive_operation_running'
> fs/btrfs/dev-replace.c: In function 'btrfs_dev_replace_continue_on_mount':
> fs/btrfs/dev-replace.c:769:10: error: too many arguments to function
> 'btrfs_scrub_dev'
> In file included from fs/btrfs/dev-replace.c:30:0:
> fs/btrfs/ctree.h:3648:5: note: declared here
> cc1: some warnings being treated as errors
>
> vim +486 fs/btrfs/dev-replace.c
>
> d8e784f5 Stefan Behrens 2012-11-05 470 }
> d8e784f5 Stefan Behrens 2012-11-05 471 ret =
> btrfs_commit_transaction(trans, root);
> d8e784f5 Stefan Behrens 2012-11-05 472 WARN_ON(ret);
> d8e784f5 Stefan Behrens 2012-11-05 473
> d8e784f5 Stefan Behrens 2012-11-05 474 /* keep away write_all_supers()
> during the finishing procedure */
> d8e784f5 Stefan Behrens 2012-11-05 475
> mutex_lock(&root->fs_info->fs_devices->device_list_mutex);
> d8e784f5 Stefan Behrens 2012-11-05 476
> btrfs_dev_replace_lock(dev_replace);
> d8e784f5 Stefan Behrens 2012-11-05 477 dev_replace->replace_state =
> d8e784f5 Stefan Behrens 2012-11-05 478 scrub_ret ?
> BTRFS_IOCTL_DEV_REPLACE_STATE_CANCELED
> d8e784f5 Stefan Behrens 2012-11-05 479 :
> BTRFS_IOCTL_DEV_REPLACE_STATE_FINISHED;
> d8e784f5 Stefan Behrens 2012-11-05 480 dev_replace->tgtdev = NULL;
> d8e784f5 Stefan Behrens 2012-11-05 481 dev_replace->srcdev = NULL;
> d8e784f5 Stefan Behrens 2012-11-05 482 dev_replace->time_stopped =
> btrfs_get_seconds_since_1970();
> d8e784f5 Stefan Behrens 2012-11-05 483
> dev_replace->item_needs_writeback = 1;
> d8e784f5 Stefan Behrens 2012-11-05 484
> d8e784f5 Stefan Behrens 2012-11-05 485 if (scrub_ret) {
> d8e784f5 Stefan Behrens 2012-11-05 @486 printk_in_rcu(KERN_ERR
> d8e784f5 Stefan Behrens 2012-11-05 487 "btrfs:
> btrfs_scrub_dev(%s, %llu, %s) failed %d\n",
> d8e784f5 Stefan Behrens 2012-11-05 488
> rcu_str_deref(src_device->name),
> d8e784f5 Stefan Be
[RFC PATCH] Btrfs-progs: disable qgroupid 0 for quota_tree
From: Wang Shilong
In kernel, qgroupid 0 is a special number when we run the quota group limit
command.
So, we should not be able to create a quota group whose id is 0, otherwise the
kernel
can't deal with it. Fix it.
Signed-off-by: Wang Shilong
Signed-off-by: Miao Xie
---
cmds-qgroup.c |4
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/cmds-qgroup.c b/cmds-qgroup.c
index 70019d0..dfff1b9 100644
--- a/cmds-qgroup.c
+++ b/cmds-qgroup.c
@@ -86,6 +86,10 @@ static int qgroup_create(int create, int argc, char **argv)
args.create = create;
args.qgroupid = parse_qgroupid(argv[1]);
+ if (!args.qgroupid) {
+ fprintf(stderr, "ERROR: qgroup 0 is not supported\n");
+ return 30;
+ }
fd = open_file_or_dir(path);
if (fd < 0) {
fprintf(stderr, "ERROR: can't access '%s'\n", path);
-- 1.7.7.6
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] Btrfs-progs: check the relation of two group by real level numbers
From: Wang Shilong
Comparing qgroupid is not good way to check the relationship of two groups,
the right way is to compare the real level numbers.
Signed-off-by: Wang Shilong
Signed-off-by: Miao Xie
---
cmds-qgroup.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/cmds-qgroup.c b/cmds-qgroup.c
index c4122bf..70019d0 100644
--- a/cmds-qgroup.c
+++ b/cmds-qgroup.c
@@ -50,7 +50,7 @@ static int qgroup_assign(int assign, int argc, char **argv)
/*
* FIXME src should accept subvol path
*/
- if (args.src >= args.dst) {
+ if ((args.src >> 48) >= (args.dst >> 48)) {
fprintf(stderr, "ERROR: bad relation requested '%s'\n", path);
return 12;
}
-- 1.7.7.6
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] Btrfs-progs: clean up reduplicate parse_qgroupid() and replace atoi with strtoull
From: Wang Shilong
1. parse_qgroupid() is implemented twice, clean up the reduplicate code.
2. atoi() can not detect errors, so use strtoull() instead of it.
Signed-off-by: Wang Shilong
Signed-off-by: Miao Xie
---
cmds-qgroup.c | 15 +--
qgroup.c | 22 ++
qgroup.h |2 ++
3 files changed, 21 insertions(+), 18 deletions(-)
diff --git a/cmds-qgroup.c b/cmds-qgroup.c
index 129a4f0..c4122bf 100644
--- a/cmds-qgroup.c
+++ b/cmds-qgroup.c
@@ -24,26 +24,13 @@
#include "ioctl.h"
#include "commands.h"
+#include "qgroup.h"
static const char * const qgroup_cmd_group_usage[] = {
"btrfs qgroup [options] ",
NULL
};
-static u64 parse_qgroupid(char *p)
-{
- char *s = strchr(p, '/');
- u64 level;
- u64 id;
-
- if (!s)
- return atoll(p);
- level = atoll(p);
- id = atoll(s + 1);
-
- return (level << 48) | id;
-}
-
static int qgroup_assign(int assign, int argc, char **argv)
{
int ret = 0;
diff --git a/qgroup.c b/qgroup.c
index 4083b57..dafde12 100644
--- a/qgroup.c
+++ b/qgroup.c
@@ -22,15 +22,29 @@
u64 parse_qgroupid(char *p)
{
char *s = strchr(p, '/');
+ char *ptr_src_end = p + strlen(p);
+ char *ptr_parse_end = NULL;
u64 level;
u64 id;
- if (!s)
- return atoll(p);
- level = atoll(p);
- id = atoll(s + 1);
+ if (!s) {
+ id = strtoull(p, &ptr_parse_end, 10);
+ if (ptr_parse_end != ptr_src_end)
+ goto err;
+ return id;
+ }
+ level = strtoull(p, &ptr_parse_end, 10);
+ if (ptr_parse_end != s)
+ goto err;
+
+ id = strtoull(s+1, &ptr_parse_end, 10);
+ if (ptr_parse_end != ptr_src_end)
+ goto err;
return (level << 48) | id;
+err:
+ fprintf(stderr, "ERROR:invalid qgroupid\n");
+ exit(-1);
}
int qgroup_inherit_size(struct btrfs_qgroup_inherit *p)
diff --git a/qgroup.h b/qgroup.h
index f7af8c5..ad14c88 100644
--- a/qgroup.h
+++ b/qgroup.h
@@ -20,7 +20,9 @@
#define _BTRFS_QGROUP_H
#include "ioctl.h"
+#include "kerncompat.h"
+u64 parse_qgroupid(char *p);
int qgroup_inherit_size(struct btrfs_qgroup_inherit *p);
int qgroup_inherit_realloc(struct btrfs_qgroup_inherit **inherit,
int incgroups, int inccopies);
-- 1.7.7.6
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] Btrfs-progs: fix arg parsing for btrfs qgroup limit commands
From: Wang Shilong
We can use this command in two ways.
1. btrfs qgroup limit size qgroupid path
2. btrfs qgroup limit size path
Before applying this patch, we differentiate them by check the parsing result
of the second argument. It is not so good because it may make some mistakes,
For example:
btrfs qgroup limit 1M 123456
^ It is a subvolume name.
In fact, we can differentiate them just by the number of arguments, so fix it
by this way.
Signed-off-by: Wang Shilong
Signed-off-by: Miao Xie
---
cmds-qgroup.c | 14 ++
1 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/cmds-qgroup.c b/cmds-qgroup.c
index 1525c11..129a4f0 100644
--- a/cmds-qgroup.c
+++ b/cmds-qgroup.c
@@ -383,7 +383,6 @@ static int cmd_qgroup_limit(int argc, char **argv)
}
memset(&args, 0, sizeof(args));
- args.qgroupid = parse_qgroupid(argv[optind + 1]);
if (size) {
if (compressed)
args.lim.flags |= BTRFS_QGROUP_LIMIT_RFER_CMPR |
@@ -397,9 +396,8 @@ static int cmd_qgroup_limit(int argc, char **argv)
}
}
- if (args.qgroupid == 0) {
- if (check_argc_exact(argc - optind, 2))
- usage(cmd_qgroup_limit_usage);
+ if (argc - optind == 2) {
+ args.qgroupid = 0;
path = argv[optind + 1];
ret = test_issubvolume(path);
if (ret < 0) {
@@ -415,11 +413,11 @@ static int cmd_qgroup_limit(int argc, char **argv)
* keep qgroupid at 0, this indicates that the subvolume the
* fd refers to is to be limited
*/
- } else {
- if (check_argc_exact(argc - optind, 3))
- usage(cmd_qgroup_limit_usage);
+ } else if (argc - optind == 3) {
+ args.qgroupid = parse_qgroupid(argv[optind + 1]);
path = argv[optind + 2];
- }
+ } else
+ usage(cmd_qgroup_limit_usage);
fd = open_file_or_dir(path);
if (fd < 0) {
-- 1.7.7.6
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
