[PATCH v3 1/2] btrfs: remove empty fs_devices to prevent memory runout
There is a global list @fs_uuids to keep @fs_devices object
for each created btrfs. But when a btrfs becomes empty
(all devices belong to it are gone), its @fs_devices remains
in @fs_uuids list until module exit.
If we keeps mkfs.btrfs on the same device again and again,
all empty @fs_devices produced are sure to eat up our memory.
So this case has better to be prevented.
I think that each time we setup btrfs on that device, we should
check whether we are stealing some device from another btrfs
seen before. To faciliate the search procedure, we could insert
all @btrfs_device in a rb_root, one @btrfs_device per each physical
device, with @bdev-bd_dev as key. Each time device stealing happens,
we should replace the corresponding @btrfs_device in the rb_root with
an up-to-date version.
If the stolen device is the last device in its @fs_devices,
then we have an empty btrfs to be deleted.
Actually there are 3 ways to steal devices and lead to empty btrfs
1. mkfs, with -f option
2. device add, with -f option
3. device replace, with -f option
We should act under these cases.
Moreover, there are special cases to consider:
o If there are seed devices, then it is asured that
the devices in cloned @fs_devices are not treated as valid devices.
o If a device disappears and reappears without any touch, its
@bdev-bd_dev may change, so we have to re-insert it into the rb_root.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
changelog
v1-v2: add handle for device disappears and reappears event
v2-v3: fix some style problems
*Note*
Actually this handles the case when a device disappears and
reappears without any touch.
We are going to recycle all dead btrfs_device in another patch.
Two events leads to the deads:
1) device disappears and never returns again
2) device disappears and returns with a new fs on it
A shrinker shall kill the deads.
---
fs/btrfs/super.c | 1 +
fs/btrfs/volumes.c | 283 ++---
fs/btrfs/volumes.h | 6 ++
3 files changed, 232 insertions(+), 58 deletions(-)
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 60f7cbe..001cba5 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2184,6 +2184,7 @@ static void __exit exit_btrfs_fs(void)
btrfs_end_io_wq_exit();
unregister_filesystem(btrfs_fs_type);
btrfs_exit_sysfs();
+ btrfs_cleanup_valid_dev_root();
btrfs_cleanup_fs_uuids();
btrfs_exit_compress();
btrfs_hash_exit();
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 0144790..d4fda8f 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -27,6 +27,7 @@
#include linux/kthread.h
#include linux/raid/pq.h
#include linux/semaphore.h
+#include linux/rbtree.h
#include asm/div64.h
#include ctree.h
#include extent_map.h
@@ -52,6 +53,127 @@ static void btrfs_dev_stat_print_on_load(struct
btrfs_device *device);
DEFINE_MUTEX(uuid_mutex);
static LIST_HEAD(fs_uuids);
+static struct rb_root valid_dev_root = RB_ROOT;
+
+static struct btrfs_device *insert_valid_device(struct btrfs_device *new_dev)
+{
+ struct rb_node **p;
+ struct rb_node *parent;
+ struct rb_node *new;
+ struct btrfs_device *old_dev;
+
+ WARN_ON(!mutex_is_locked(uuid_mutex));
+
+ parent = NULL;
+ new = new_dev-rb_node;
+
+ p = valid_dev_root.rb_node;
+ while (*p) {
+ parent = *p;
+ old_dev = rb_entry(parent, struct btrfs_device, rb_node);
+
+ if (new_dev-devnum old_dev-devnum) {
+ p = parent-rb_left;
+ } else if (new_dev-devnum old_dev-devnum) {
+ p = parent-rb_right;
+ } else {
+ rb_replace_node(parent, new, valid_dev_root);
+ RB_CLEAR_NODE(parent);
+
+ goto out;
+ }
+ }
+
+ old_dev = NULL;
+ rb_link_node(new, parent, p);
+ rb_insert_color(new, valid_dev_root);
+
+out:
+ return old_dev;
+}
+
+static void free_fs_devices(struct btrfs_fs_devices *fs_devices)
+{
+ struct btrfs_device *device;
+
+ WARN_ON(fs_devices-opened);
+
+ while (!list_empty(fs_devices-devices)) {
+ device = list_entry(fs_devices-devices.next,
+ struct btrfs_device, dev_list);
+ list_del(device-dev_list);
+ rcu_string_free(device-name);
+ kfree(device);
+ }
+ kfree(fs_devices);
+}
+
+static void remove_empty_fs_if_need(struct btrfs_fs_devices *old_fs)
+{
+ struct btrfs_fs_devices *seed_fs;
+
+ if (!list_empty(old_fs-devices))
+ return;
+
+ list_del(old_fs-list);
+
+ /* free the seed clones */
+ seed_fs = old_fs-seed;
+ free_fs_devices(old_fs);
+ while (seed_fs
Re: [PATCH v2 1/2 RESEND] btrfs: remove empty fs_devices to prevent memory runout
Oh, sorry, some format style problems...
let me resend a new one.
On Thu, 2015-01-15 at 16:53 +0800, Gui Hecheng wrote:
There is a global list @fs_uuids to keep @fs_devices object
for each created btrfs. But when a btrfs becomes empty
(all devices belong to it are gone), its @fs_devices remains
in @fs_uuids list until module exit.
If we keeps mkfs.btrfs on the same device again and again,
all empty @fs_devices produced are sure to eat up our memory.
So this case has better to be prevented.
I think that each time we setup btrfs on that device, we should
check whether we are stealing some device from another btrfs
seen before. To faciliate the search procedure, we could insert
all @btrfs_device in a rb_root, one @btrfs_device per each physical
device, with @bdev-bd_dev as key. Each time device stealing happens,
we should replace the corresponding @btrfs_device in the rb_root with
an up-to-date version.
If the stolen device is the last device in its @fs_devices,
then we have an empty btrfs to be deleted.
Actually there are 3 ways to steal devices and lead to empty btrfs
1. mkfs, with -f option
2. device add, with -f option
3. device replace, with -f option
We should act under these cases.
Moreover, there are special cases to consider:
o If there are seed devices, then it is asured that
the devices in cloned @fs_devices are not treated as valid devices.
o If a device disappears and reappears without any touch, its
@bdev-bd_dev may change, so we have to re-insert it into the rb_root.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
changelog
v1-v2: add handle for device disappears and reappears event
*Note*
Actually this handles the case when a device disappears and
reappears without any touch.
We are going to recycle all dead btrfs_device in another patch.
Two events leads to the deads:
1) device disappears and never returns again
2) device disappears and returns with a new fs on it
A shrinker shall kill the deads.
---
fs/btrfs/super.c | 1 +
fs/btrfs/volumes.c | 281
++---
fs/btrfs/volumes.h | 6 ++
3 files changed, 230 insertions(+), 58 deletions(-)
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 60f7cbe..001cba5 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2184,6 +2184,7 @@ static void __exit exit_btrfs_fs(void)
btrfs_end_io_wq_exit();
unregister_filesystem(btrfs_fs_type);
btrfs_exit_sysfs();
+ btrfs_cleanup_valid_dev_root();
btrfs_cleanup_fs_uuids();
btrfs_exit_compress();
btrfs_hash_exit();
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 0144790..228a7e0 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -27,6 +27,7 @@
#include linux/kthread.h
#include linux/raid/pq.h
#include linux/semaphore.h
+#include linux/rbtree.h
#include asm/div64.h
#include ctree.h
#include extent_map.h
@@ -52,6 +53,126 @@ static void btrfs_dev_stat_print_on_load(struct
btrfs_device *device);
DEFINE_MUTEX(uuid_mutex);
static LIST_HEAD(fs_uuids);
+static struct rb_root valid_dev_root = RB_ROOT;
+
+static struct btrfs_device *insert_valid_device(struct btrfs_device *new_dev)
+{
+ struct rb_node **p;
+ struct rb_node *parent;
+ struct rb_node *new;
+ struct btrfs_device *old_dev;
+
+ WARN_ON(!mutex_is_locked(uuid_mutex));
+
+ parent = NULL;
+ new = new_dev-rb_node;
+
+ p = valid_dev_root.rb_node;
+ while (*p) {
+ parent = *p;
+ old_dev = rb_entry(parent, struct btrfs_device, rb_node);
+
+ if (new_dev-devnum old_dev-devnum)
+ p = parent-rb_left;
+ else if (new_dev-devnum old_dev-devnum)
+ p = parent-rb_right;
+ else {
+ rb_replace_node(parent, new, valid_dev_root);
+ RB_CLEAR_NODE(parent);
+
+ goto out;
+ }
+ }
+
+ old_dev = NULL;
+ rb_link_node(new, parent, p);
+ rb_insert_color(new, valid_dev_root);
+
+out:
+ return old_dev;
+}
+
+static void free_fs_devices(struct btrfs_fs_devices *fs_devices)
+{
+ struct btrfs_device *device;
+ WARN_ON(fs_devices-opened);
+ while (!list_empty(fs_devices-devices)) {
+ device = list_entry(fs_devices-devices.next,
+ struct btrfs_device, dev_list);
+ list_del(device-dev_list);
+ rcu_string_free(device-name);
+ kfree(device);
+ }
+ kfree(fs_devices);
+}
+
+static void remove_empty_fs_if_need(struct btrfs_fs_devices *old_fs)
+{
+ struct btrfs_fs_devices *seed_fs;
+
+ if (!list_empty(old_fs-devices))
+ return;
+
+ list_del(old_fs-list);
+
+ /* free the seed
[PATCH v2 1/2 RESEND] btrfs: remove empty fs_devices to prevent memory runout
There is a global list @fs_uuids to keep @fs_devices object
for each created btrfs. But when a btrfs becomes empty
(all devices belong to it are gone), its @fs_devices remains
in @fs_uuids list until module exit.
If we keeps mkfs.btrfs on the same device again and again,
all empty @fs_devices produced are sure to eat up our memory.
So this case has better to be prevented.
I think that each time we setup btrfs on that device, we should
check whether we are stealing some device from another btrfs
seen before. To faciliate the search procedure, we could insert
all @btrfs_device in a rb_root, one @btrfs_device per each physical
device, with @bdev-bd_dev as key. Each time device stealing happens,
we should replace the corresponding @btrfs_device in the rb_root with
an up-to-date version.
If the stolen device is the last device in its @fs_devices,
then we have an empty btrfs to be deleted.
Actually there are 3 ways to steal devices and lead to empty btrfs
1. mkfs, with -f option
2. device add, with -f option
3. device replace, with -f option
We should act under these cases.
Moreover, there are special cases to consider:
o If there are seed devices, then it is asured that
the devices in cloned @fs_devices are not treated as valid devices.
o If a device disappears and reappears without any touch, its
@bdev-bd_dev may change, so we have to re-insert it into the rb_root.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
changelog
v1-v2: add handle for device disappears and reappears event
*Note*
Actually this handles the case when a device disappears and
reappears without any touch.
We are going to recycle all dead btrfs_device in another patch.
Two events leads to the deads:
1) device disappears and never returns again
2) device disappears and returns with a new fs on it
A shrinker shall kill the deads.
---
fs/btrfs/super.c | 1 +
fs/btrfs/volumes.c | 281 ++---
fs/btrfs/volumes.h | 6 ++
3 files changed, 230 insertions(+), 58 deletions(-)
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 60f7cbe..001cba5 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2184,6 +2184,7 @@ static void __exit exit_btrfs_fs(void)
btrfs_end_io_wq_exit();
unregister_filesystem(btrfs_fs_type);
btrfs_exit_sysfs();
+ btrfs_cleanup_valid_dev_root();
btrfs_cleanup_fs_uuids();
btrfs_exit_compress();
btrfs_hash_exit();
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 0144790..228a7e0 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -27,6 +27,7 @@
#include linux/kthread.h
#include linux/raid/pq.h
#include linux/semaphore.h
+#include linux/rbtree.h
#include asm/div64.h
#include ctree.h
#include extent_map.h
@@ -52,6 +53,126 @@ static void btrfs_dev_stat_print_on_load(struct
btrfs_device *device);
DEFINE_MUTEX(uuid_mutex);
static LIST_HEAD(fs_uuids);
+static struct rb_root valid_dev_root = RB_ROOT;
+
+static struct btrfs_device *insert_valid_device(struct btrfs_device *new_dev)
+{
+ struct rb_node **p;
+ struct rb_node *parent;
+ struct rb_node *new;
+ struct btrfs_device *old_dev;
+
+ WARN_ON(!mutex_is_locked(uuid_mutex));
+
+ parent = NULL;
+ new = new_dev-rb_node;
+
+ p = valid_dev_root.rb_node;
+ while (*p) {
+ parent = *p;
+ old_dev = rb_entry(parent, struct btrfs_device, rb_node);
+
+ if (new_dev-devnum old_dev-devnum)
+ p = parent-rb_left;
+ else if (new_dev-devnum old_dev-devnum)
+ p = parent-rb_right;
+ else {
+ rb_replace_node(parent, new, valid_dev_root);
+ RB_CLEAR_NODE(parent);
+
+ goto out;
+ }
+ }
+
+ old_dev = NULL;
+ rb_link_node(new, parent, p);
+ rb_insert_color(new, valid_dev_root);
+
+out:
+ return old_dev;
+}
+
+static void free_fs_devices(struct btrfs_fs_devices *fs_devices)
+{
+ struct btrfs_device *device;
+ WARN_ON(fs_devices-opened);
+ while (!list_empty(fs_devices-devices)) {
+ device = list_entry(fs_devices-devices.next,
+ struct btrfs_device, dev_list);
+ list_del(device-dev_list);
+ rcu_string_free(device-name);
+ kfree(device);
+ }
+ kfree(fs_devices);
+}
+
+static void remove_empty_fs_if_need(struct btrfs_fs_devices *old_fs)
+{
+ struct btrfs_fs_devices *seed_fs;
+
+ if (!list_empty(old_fs-devices))
+ return;
+
+ list_del(old_fs-list);
+
+ /* free the seed clones */
+ seed_fs = old_fs-seed;
+ free_fs_devices(old_fs);
+ while (seed_fs) {
+ old_fs = seed_fs
[PATCH 2/2 RESEND] btrfs: introduce shrinker for rb_tree that keeps valid btrfs_devices
The following patch:
btrfs: remove empty fs_devices to prevent memory runout
introduces @valid_dev_root aiming at recording @btrfs_device objects that
have corresponding block devices with btrfs.
But if a block device is broken or unplugged, no one tells the
@valid_dev_root to cleanup the dead objects.
To recycle the memory occuppied by those deads, we could rely on
the shrinker. The shrinker's scan function will traverse the
@valid_dev_root and trys to open the devices one by one, if it fails
or encounters a non-btrfs it will remove the dead @btrfs_device.
A special case to deal with is that a block device is unplugged and
replugged, then it appears with a new @bdev-bd_dev as devnum.
In this case, we should remove the older since we should have a new
one for that block device already.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
fs/btrfs/super.c | 10
fs/btrfs/volumes.c | 74 +-
fs/btrfs/volumes.h | 4 +++
3 files changed, 87 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 001cba5..022381e 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2017,6 +2017,12 @@ static struct miscdevice btrfs_misc = {
.fops = btrfs_ctl_fops
};
+static struct shrinker btrfs_valid_dev_shrinker = {
+ .scan_objects = btrfs_valid_dev_scan,
+ .count_objects = btrfs_valid_dev_count,
+ .seeks = DEFAULT_SEEKS,
+};
+
MODULE_ALIAS_MISCDEV(BTRFS_MINOR);
MODULE_ALIAS(devname:btrfs-control);
@@ -2130,6 +2136,8 @@ static int __init init_btrfs_fs(void)
btrfs_init_lockdep();
+ register_shrinker(btrfs_valid_dev_shrinker);
+
btrfs_print_info();
err = btrfs_run_sanity_tests();
@@ -2143,6 +2151,7 @@ static int __init init_btrfs_fs(void)
return 0;
unregister_ioctl:
+ unregister_shrinker(btrfs_valid_dev_shrinker);
btrfs_interface_exit();
free_end_io_wq:
btrfs_end_io_wq_exit();
@@ -2183,6 +2192,7 @@ static void __exit exit_btrfs_fs(void)
btrfs_interface_exit();
btrfs_end_io_wq_exit();
unregister_filesystem(btrfs_fs_type);
+ unregister_shrinker(btrfs_valid_dev_shrinker);
btrfs_exit_sysfs();
btrfs_cleanup_valid_dev_root();
btrfs_cleanup_fs_uuids();
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 228a7e0..5462557 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -54,6 +54,7 @@ static void btrfs_dev_stat_print_on_load(struct btrfs_device
*device);
DEFINE_MUTEX(uuid_mutex);
static LIST_HEAD(fs_uuids);
static struct rb_root valid_dev_root = RB_ROOT;
+static atomic_long_t unopened_dev_count = ATOMIC_LONG_INIT(0);
static struct btrfs_device *insert_valid_device(struct btrfs_device *new_dev)
{
@@ -130,6 +131,8 @@ static void free_invalid_device(struct btrfs_device
*invalid_dev)
{
struct btrfs_fs_devices *old_fs;
+ atomic_long_dec(unopened_dev_count);
+
old_fs = invalid_dev-fs_devices;
mutex_lock(old_fs-device_list_mutex);
list_del(invalid_dev-dev_list);
@@ -605,6 +608,7 @@ static noinline int device_list_add(const char *path,
list_add_rcu(device-dev_list, fs_devices-devices);
fs_devices-num_devices++;
mutex_unlock(fs_devices-device_list_mutex);
+ atomic_long_inc(unopened_dev_count);
ret = 1;
device-fs_devices = fs_devices;
@@ -778,6 +782,7 @@ again:
blkdev_put(device-bdev, device-mode);
device-bdev = NULL;
fs_devices-open_devices--;
+ atomic_long_inc(unopened_dev_count);
}
if (device-writeable) {
list_del_init(device-dev_alloc_list);
@@ -840,8 +845,10 @@ static int __btrfs_close_devices(struct btrfs_fs_devices
*fs_devices)
struct btrfs_device *new_device;
struct rcu_string *name;
- if (device-bdev)
+ if (device-bdev) {
fs_devices-open_devices--;
+ atomic_long_inc(unopened_dev_count);
+ }
if (device-writeable
device-devid != BTRFS_DEV_REPLACE_DEVID) {
@@ -971,6 +978,7 @@ static int __btrfs_open_devices(struct btrfs_fs_devices
*fs_devices,
fs_devices-rotating = 1;
fs_devices-open_devices++;
+ atomic_long_dec(unopened_dev_count);
if (device-writeable
device-devid != BTRFS_DEV_REPLACE_DEVID) {
fs_devices-rw_devices++;
@@ -6848,3 +6856,67 @@ void btrfs_update_commit_device_bytes_used(struct
btrfs_root *root,
}
unlock_chunks(root);
}
+
+static unsigned long shrink_valid_dev_root(void)
+{
+ struct rb_node *n;
+ struct btrfs_device *device;
+ struct
Re: [PATCH] btrfs: cleanup a straight free-after-malloc branch for free-space-cache
On Wed, 2015-01-14 at 16:22 +0100, David Sterba wrote:
On Wed, Jan 14, 2015 at 04:18:54PM +0800, Gui Hecheng wrote:
Move the branch that is unrelated to the result of io_ctl_init() before
the function call, so we can save a kmalloc() kfree() pair in that
branch.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
fs/btrfs/free-space-cache.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
index d6c03f7..88f6122 100644
--- a/fs/btrfs/free-space-cache.c
+++ b/fs/btrfs/free-space-cache.c
@@ -1132,10 +1132,6 @@ static int __btrfs_write_out_cache(struct btrfs_root
*root, struct inode *inode,
if (!i_size_read(inode))
return -1;
- ret = io_ctl_init(io_ctl, inode, root, 1);
- if (ret)
- return -1;
I'm not sure this preserves the original semantics. This can fail if
there's no memory, fine, but also ENOSPC if the crcs do not fit into
the first page as the comment in io_ctl_init() says. There's an
additional condition that the inode is not FREE_INO, ie. it is the
FREE_SPACE inode.
So in some cases io_ctl_init may fail but would not after your patch.
-
if (block_group (block_group-flags BTRFS_BLOCK_GROUP_DATA)) {
down_write(block_group-data_rwsem);
spin_lock(block_group-lock);
@@ -1145,11 +1141,15 @@ static int __btrfs_write_out_cache(struct
btrfs_root *root, struct inode *inode,
up_write(block_group-data_rwsem);
BTRFS_I(inode)-generation = 0;
ret = 0;
- goto out;
+ goto out_skip;
}
spin_unlock(block_group-lock);
}
+ ret = io_ctl_init(io_ctl, inode, root, 1);
+ if (ret)
+ return -1;
This would leave block_group-data_rwsem locked, ie. another exit path
would have to be added that would reflect the current state (no io_ctl
initialized and the extent range not locked). We cannot use out_enospc
here.
Yes, you're right, the -data_rwsem shall not be left locked.
I'm not sure if the kmalloc/kfree savings are significant here.
I'm not sure whether it brings much, please *ignore* this patch and I
will do more checks.
Thanks,
Gui
+
/* Lock all pages first so we can lock the extent safely. */
io_ctl_prepare_pages(io_ctl, inode, 0);
@@ -1212,13 +1212,14 @@ static int __btrfs_write_out_cache(struct
btrfs_root *root, struct inode *inode,
/* Flush the dirty pages in the cache file. */
ret = flush_dirty_cache(inode);
if (ret)
- goto out;
+ goto out_free;
/* Update the cache item to tell everyone this cache file is valid. */
ret = update_cache_item(trans, root, inode, path, offset,
entries, bitmaps);
-out:
+out_free:
io_ctl_free(io_ctl);
+out_skip:
if (ret) {
invalidate_inode_pages2(inode-i_mapping);
BTRFS_I(inode)-generation = 0;
@@ -1232,7 +1233,7 @@ out_nospc:
if (block_group (block_group-flags BTRFS_BLOCK_GROUP_DATA))
up_write(block_group-data_rwsem);
- goto out;
+ goto out_free;
}
--
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
[PATCH] btrfs: fix raid56 scrub failed in xfstests btrfs/072
The xfstests btrfs/072 reports uncorrectable read errors in dmesg, because scrub forgets to use commit_root for parity scrub routine and scrub attempts to scrub those extents items whose contents are not fully on disk. To fix it, we just add the @search_commit_root flag back. Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com Signed-off-by: Qu Wenruo quwen...@cn.fujitsu.com --- fs/btrfs/scrub.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c index f2bb13a..aa8ff75 100644 --- a/fs/btrfs/scrub.c +++ b/fs/btrfs/scrub.c @@ -3065,6 +3065,8 @@ static noinline_for_stack int scrub_stripe(struct scrub_ctx *sctx, path-search_commit_root = 1; path-skip_locking = 1; + ppath-search_commit_root = 1; + ppath-skip_locking = 1; /* * trigger the readahead for extent tree csum tree and wait for * completion. During readahead, the scrub is officially paused -- 1.8.1.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] btrfs: introduce shrinker for rb_tree that keeps valid btrfs_devices
[ping]
On Wed, 2014-12-10 at 15:39 +0800, Gui Hecheng wrote:
The following patch:
btrfs: remove empty fs_devices to prevent memory runout
introduces @valid_dev_root aiming at recording @btrfs_device objects that
have corresponding block devices with btrfs.
But if a block device is broken or unplugged, no one tells the
@valid_dev_root to cleanup the dead objects.
To recycle the memory occuppied by those deads, we could rely on
the shrinker. The shrinker's scan function will traverse the
@valid_dev_root and trys to open the devices one by one, if it fails
or encounters a non-btrfs it will remove the dead @btrfs_device.
A special case to deal with is that a block device is unplugged and
replugged, then it appears with a new @bdev-bd_dev as devnum.
In this case, we should remove the older since we should have a new
one for that block device already.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
fs/btrfs/super.c | 10
fs/btrfs/volumes.c | 74
+-
fs/btrfs/volumes.h | 4 +++
3 files changed, 87 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index ee09a56..29069af 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1987,6 +1987,12 @@ static struct miscdevice btrfs_misc = {
.fops = btrfs_ctl_fops
};
+static struct shrinker btrfs_valid_dev_shrinker = {
+ .scan_objects = btrfs_valid_dev_scan,
+ .count_objects = btrfs_valid_dev_count,
+ .seeks = DEFAULT_SEEKS,
+};
+
MODULE_ALIAS_MISCDEV(BTRFS_MINOR);
MODULE_ALIAS(devname:btrfs-control);
@@ -2100,6 +2106,8 @@ static int __init init_btrfs_fs(void)
btrfs_init_lockdep();
+ register_shrinker(btrfs_valid_dev_shrinker);
+
btrfs_print_info();
err = btrfs_run_sanity_tests();
@@ -2113,6 +2121,7 @@ static int __init init_btrfs_fs(void)
return 0;
unregister_ioctl:
+ unregister_shrinker(btrfs_valid_dev_shrinker);
btrfs_interface_exit();
free_end_io_wq:
btrfs_end_io_wq_exit();
@@ -2153,6 +2162,7 @@ static void __exit exit_btrfs_fs(void)
btrfs_interface_exit();
btrfs_end_io_wq_exit();
unregister_filesystem(btrfs_fs_type);
+ unregister_shrinker(btrfs_valid_dev_shrinker);
btrfs_exit_sysfs();
btrfs_cleanup_valid_dev_root();
btrfs_cleanup_fs_uuids();
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 7093cce..62f37b1 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -54,6 +54,7 @@ static void btrfs_dev_stat_print_on_load(struct
btrfs_device *device);
DEFINE_MUTEX(uuid_mutex);
static LIST_HEAD(fs_uuids);
static struct rb_root valid_dev_root = RB_ROOT;
+static atomic_long_t unopened_dev_count = ATOMIC_LONG_INIT(0);
static struct btrfs_device *insert_valid_device(struct btrfs_device *new_dev)
{
@@ -130,6 +131,8 @@ static void free_invalid_device(struct btrfs_device
*invalid_dev)
{
struct btrfs_fs_devices *old_fs;
+ atomic_long_dec(unopened_dev_count);
+
old_fs = invalid_dev-fs_devices;
mutex_lock(old_fs-device_list_mutex);
list_del(invalid_dev-dev_list);
@@ -615,6 +618,7 @@ static noinline int device_list_add(const char *path,
list_add_rcu(device-dev_list, fs_devices-devices);
fs_devices-num_devices++;
mutex_unlock(fs_devices-device_list_mutex);
+ atomic_long_inc(unopened_dev_count);
ret = 1;
device-fs_devices = fs_devices;
@@ -788,6 +792,7 @@ again:
blkdev_put(device-bdev, device-mode);
device-bdev = NULL;
fs_devices-open_devices--;
+ atomic_long_inc(unopened_dev_count);
}
if (device-writeable) {
list_del_init(device-dev_alloc_list);
@@ -850,8 +855,10 @@ static int __btrfs_close_devices(struct btrfs_fs_devices
*fs_devices)
struct btrfs_device *new_device;
struct rcu_string *name;
- if (device-bdev)
+ if (device-bdev) {
fs_devices-open_devices--;
+ atomic_long_inc(unopened_dev_count);
+ }
if (device-writeable
device-devid != BTRFS_DEV_REPLACE_DEVID) {
@@ -981,6 +988,7 @@ static int __btrfs_open_devices(struct btrfs_fs_devices
*fs_devices,
fs_devices-rotating = 1;
fs_devices-open_devices++;
+ atomic_long_dec(unopened_dev_count);
if (device-writeable
device-devid != BTRFS_DEV_REPLACE_DEVID) {
fs_devices-rw_devices++;
@@ -6828,3 +6836,67 @@ void btrfs_update_commit_device_bytes_used(struct
btrfs_root *root,
}
unlock_chunks(root);
}
+
+static unsigned long shrink_valid_dev_root
Re: [PATCH v3] btrfs-progs: Documentation: add T/P/E description for resize cmd
On Fri, 2015-01-02 at 17:21 +0100, David Sterba wrote: On Fri, Jan 02, 2015 at 05:12:04PM +0100, David Sterba wrote: On Thu, Jan 01, 2015 at 08:27:55PM -0700, Chris Murphy wrote: Small problem with the rendering of this commit d4ef1a06f8be623ae94e4d498c306e8dd1605bef, when I use 'man btrfs filesystem' the above portion looks like this: 'K', 'M', 'G', 'T', 'P', or 'E\', I'm not sure why there's a trailing slash after the E. Me neither, but it looks like a bug in the asciidoc processing. Seems that only the first ' has to be quoted, and consumes the next unquoted ' as a pair, so with the last \' the next one is missing and is printed verbatim: Fixed by: -units designators: \'K\', \'M\', \'G\', \'T\', \'P\', or \'E\', which represent +units designators: \'K', \'M', \'G', \'T', \'P', or \'E', which represent Oh, sorry, I missed this problem, thanks for fixing it. -Gui -- 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: cleanup init for list in free-space-cache
o removed an unecessary INIT_LIST_HEAD after LIST_HEAD o merge a declare INIT_LIST_HEAD pair into one LIST_HEAD Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com --- fs/btrfs/free-space-cache.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c index d6c03f7..8d346d3 100644 --- a/fs/btrfs/free-space-cache.c +++ b/fs/btrfs/free-space-cache.c @@ -651,15 +651,13 @@ static int __load_free_space_cache(struct btrfs_root *root, struct inode *inode, struct io_ctl io_ctl; struct btrfs_key key; struct btrfs_free_space *e, *n; - struct list_head bitmaps; + LIST_HEAD(bitmaps); u64 num_entries; u64 num_bitmaps; u64 generation; u8 type; int ret = 0; - INIT_LIST_HEAD(bitmaps); - /* Nothing in the space cache, goodbye */ if (!i_size_read(inode)) return 0; @@ -2903,7 +2901,6 @@ int btrfs_find_space_cluster(struct btrfs_root *root, trace_btrfs_find_cluster(block_group, offset, bytes, empty_size, min_bytes); - INIT_LIST_HEAD(bitmaps); ret = setup_cluster_no_bitmap(block_group, cluster, bitmaps, offset, bytes + empty_size, cont1_bytes, min_bytes); -- 1.8.1.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 v2] btrfs-progs: Documentation: add T/P/E description for resize cmd
On Mon, 2014-12-29 at 17:07 +0100, David Sterba wrote: On Mon, Dec 22, 2014 at 03:22:53PM +0800, Gui Hecheng wrote: Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com Reviewed-by: Satoru Takeuchi takeuchi_sat...@jp.fujitsu.com --- changelog v1-v2: s/\'E\'(EiB)/or \'E\'(EiB)/ as suggested by Satoru, thanks. --- Documentation/btrfs-filesystem.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/btrfs-filesystem.txt b/Documentation/btrfs-filesystem.txt index a8f2972..138ba6c 100644 --- a/Documentation/btrfs-filesystem.txt +++ b/Documentation/btrfs-filesystem.txt @@ -102,8 +102,8 @@ If the prefix + or - is present the size is increased or decreased by the quantity size. If no units are specified, the unit of the size parameter defaults to bytes. Optionally, the size parameter may be suffixed by one of the following -units designators: \'K\', \'M', or \'G', kilobytes, megabytes, or gigabytes, -respectively. +units designators: \'K\'(KiB), \'M\'(MiB), \'G\'(GiB), \'T\'(TiB), \'P\'(PiB) +or \'E\'(EiB). I find this a bit confusing as this would suggest that eg. $ btrfs fi resize -1GiB is valid. Oh, you're right, thanks for pointing it out, then I'll just follow the old way and send a new one. -Gui -- 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
[PATCH v3] btrfs-progs: Documentation: add T/P/E description for resize cmd
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com Reviewed-by: Satoru Takeuchi takeuchi_sat...@jp.fujitsu.com --- changelog v1-v2: s/\'E\'(EiB)/or \'E\'(EiB)/ as suggested by Satoru, thanks. v2-v3: replace confusing format 'K'(KiB) etc. Thanks, David. --- Documentation/btrfs-filesystem.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Documentation/btrfs-filesystem.txt b/Documentation/btrfs-filesystem.txt index a8f2972..96c4420 100644 --- a/Documentation/btrfs-filesystem.txt +++ b/Documentation/btrfs-filesystem.txt @@ -102,8 +102,9 @@ If the prefix + or - is present the size is increased or decreased by the quantity size. If no units are specified, the unit of the size parameter defaults to bytes. Optionally, the size parameter may be suffixed by one of the following -units designators: \'K\', \'M', or \'G', kilobytes, megabytes, or gigabytes, -respectively. +units designators: \'K\', \'M\', \'G\', \'T\', \'P\', or \'E\', which represent +KiB, MiB, GiB, TiB, PiB, or EiB, respectively. + + If \'max' is passed, the filesystem will occupy all available space on the device devid. -- 1.8.1.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
[PATCH 1/2] btrfs-progs: move check_arg_type() to util.c
The check_arg_type() function does quite generic thing,
move it to utils.c.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
cmds-filesystem.c | 32
utils.c | 32
utils.h | 1 +
3 files changed, 33 insertions(+), 32 deletions(-)
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 253f105..4d7a797 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -537,38 +537,6 @@ static int print_one_fs(struct btrfs_ioctl_fs_info_args
*fs_info,
return 0;
}
-/* This function checks if the given input parameter is
- * an uuid or a path
- * return -1: some error in the given input
- * return 0: unknow input
- * return 1: given input is uuid
- * return 2: given input is path
- */
-static int check_arg_type(char *input)
-{
- uuid_t out;
- char path[PATH_MAX];
-
- if (!input)
- return -EINVAL;
-
- if (realpath(input, path)) {
- if (is_block_device(path) == 1)
- return BTRFS_ARG_BLKDEV;
-
- if (is_mount_point(path) == 1)
- return BTRFS_ARG_MNTPOINT;
-
- return BTRFS_ARG_UNKNOWN;
- }
-
- if (strlen(input) == (BTRFS_UUID_UNPARSED_SIZE - 1)
- !uuid_parse(input, out))
- return BTRFS_ARG_UUID;
-
- return BTRFS_ARG_UNKNOWN;
-}
-
static int btrfs_scan_kernel(void *search)
{
int ret = 0, fd;
diff --git a/utils.c b/utils.c
index 2a92416..80f85e9 100644
--- a/utils.c
+++ b/utils.c
@@ -852,6 +852,38 @@ int is_mount_point(const char *path)
return ret;
}
+/* This function checks if the given input parameter is
+ * an uuid or a path
+ * return -1: some error in the given input
+ * return 0: unknow input
+ * return 1: given input is uuid
+ * return 2: given input is path
+ */
+int check_arg_type(const char *input)
+{
+ uuid_t out;
+ char path[PATH_MAX];
+
+ if (!input)
+ return -EINVAL;
+
+ if (realpath(input, path)) {
+ if (is_block_device(path) == 1)
+ return BTRFS_ARG_BLKDEV;
+
+ if (is_mount_point(path) == 1)
+ return BTRFS_ARG_MNTPOINT;
+
+ return BTRFS_ARG_UNKNOWN;
+ }
+
+ if (strlen(input) == (BTRFS_UUID_UNPARSED_SIZE - 1)
+ !uuid_parse(input, out))
+ return BTRFS_ARG_UUID;
+
+ return BTRFS_ARG_UNKNOWN;
+}
+
/*
* Find the mount point for a mounted device.
* On success, returns 0 with mountpoint in *mp.
diff --git a/utils.h b/utils.h
index 289e86b..8d67720 100644
--- a/utils.h
+++ b/utils.h
@@ -115,6 +115,7 @@ int set_label(const char *btrfs_dev, const char *label);
char *__strncpy__null(char *dest, const char *src, size_t n);
int is_block_device(const char *file);
int is_mount_point(const char *file);
+int check_arg_type(const char *input);
int open_path_or_dev_mnt(const char *path, DIR **dirstream);
u64 btrfs_device_size(int fd, struct stat *st);
/* Helper to always get proper size of the destination string */
--
1.8.1.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 2/2] btrfs-progs: refine btrfs-debug-tree error prompt when a mount point given
On Thu, 2014-12-25 at 15:49 +0900, Satoru Takeuchi wrote:
On 2014/12/25 10:16, Gui Hecheng wrote:
Now, if exec:
# btrfs-debug-tree mount_point
it echos:
: Superblock bytenr is larger than device size
But it is quite misleading, because it is a valid btrfs.
In this case, we should tell the developer to provide a block device.
After apply:
: 'mount_point' is not a block device
: 'usage: btrfs-debug-tree [options] device
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
btrfs-debug-tree.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/btrfs-debug-tree.c b/btrfs-debug-tree.c
index e46500d..7f079a9 100644
--- a/btrfs-debug-tree.c
+++ b/btrfs-debug-tree.c
@@ -179,6 +179,12 @@ int main(int ac, char **av)
if (check_argc_exact(ac, 1))
print_usage();
+ ret = check_arg_type(av[optind]);
+ if (ret != BTRFS_ARG_BLKDEV) {
+ fprintf(stderr, '%s' is not a block device\n, av[optind]);
fprintf(stderr, ERROR: '%s' is ...) is better since
other error messages in btrfs-progs have this convention.
Yes, it is good to make it clear about the ERROR, but since there is
no ERROR word throughout the source file, I think I would make all
error prompts to start with the ERROR word globally in the progs in
another patch.
Thanks,
Gui
Reviewed-by: Satoru Takeuchi takeuchi_sat...@jp.fujitsu.com
Thanks,
Satoru
+ print_usage();
+ }
+
info = open_ctree_fs_info(av[optind], 0, 0, OPEN_CTREE_PARTIAL);
if (!info) {
fprintf(stderr, unable to open %s\n, av[optind]);
--
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-progs: Fix btrfs fi show by uuid and label
On Tue, 2014-12-23 at 11:34 -0800, Justin Maggard wrote:
Commit 8be2fff (btrfs-progs: apply realpath for btrfs fi
show when mount point is given) changed the behavior of
btrfs fi show to return an error if the call to realpath()
failed. This broke the ability to specify a filesystem by
uuid or label.
Oh, that's my fault, thanks for correcting this.
So let's not consider a failed call to realpath() as an
error. If the user really specified a bad device, just
return nothing like we did before.
---
cmds-filesystem.c |9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 8f037dd..a654e6f 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -901,13 +901,8 @@ static int cmd_show(int argc, char **argv)
* realpath do /mnt/btrfs/ = /mnt/btrfs
* which shall be recognized by btrfs_scan_kernel()
*/
- if (!realpath(search, path)) {
- fprintf(stderr, ERROR: Could not show %s: %s\n,
- search, strerror(errno));
- return 1;
- }
-
- search = path;
+ if (realpath(search, path))
+ search = path;
This looks nice and stay consistent with the behavior before my faulty
commit.
Reviewed-by: Gui Hecheng guihc.f...@cn.fujitsu.com
Thanks,
Gui
/*
* Needs special handling if input arg is block dev And if
--
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 1/2] Remove unnecessary placeholder in btrfs_err_code
On Mon, 2014-12-22 at 19:39 +0900, Satoru Takeuchi wrote:
Sorry, I forgot to add Signed-off-by line.
---
From: Satoru Takeuchi takeuchi_sat...@jp.fujitsu.com
notused is not necessary. Set 1 to the first entry is enough.
Hi Satoru,
Actually, the struct is copied from the kernel header
include/uapi/linux/btrfs.h.
I think they should stay the same, so either change the kernel header
in another path, or just keep them in old style.
Thanks,
Gui
Signed-off-by: Satoru Takeuchi takeuchi_sat...@jp.fujitsu.com
---
ioctl.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/ioctl.h b/ioctl.h
index 2c2c7c1..c377b96 100644
--- a/ioctl.h
+++ b/ioctl.h
@@ -474,8 +474,7 @@ struct btrfs_ioctl_qgroup_create_args {
/* Error codes as returned by the kernel */
enum btrfs_err_code {
- notused,
- BTRFS_ERROR_DEV_RAID1_MIN_NOT_MET,
+ BTRFS_ERROR_DEV_RAID1_MIN_NOT_MET = 1,
BTRFS_ERROR_DEV_RAID10_MIN_NOT_MET,
BTRFS_ERROR_DEV_RAID5_MIN_NOT_MET,
BTRFS_ERROR_DEV_RAID6_MIN_NOT_MET,
-- 1.8.3.1 -- 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
--
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-progs: Documentation: add T/P/E description for resize cmd
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com --- Documentation/btrfs-filesystem.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/btrfs-filesystem.txt b/Documentation/btrfs-filesystem.txt index a8f2972..f2bd50d 100644 --- a/Documentation/btrfs-filesystem.txt +++ b/Documentation/btrfs-filesystem.txt @@ -102,8 +102,8 @@ If the prefix + or - is present the size is increased or decreased by the quantity size. If no units are specified, the unit of the size parameter defaults to bytes. Optionally, the size parameter may be suffixed by one of the following -units designators: \'K\', \'M', or \'G', kilobytes, megabytes, or gigabytes, -respectively. +units designators: \'K\'(KiB), \'M\'(MiB), \'G\'(GiB), \'T\'(TiB), \'P\'(PiB), +\'E\'(EiB). + If \'max' is passed, the filesystem will occupy all available space on the device devid. -- 1.8.1.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
[PATCH v2] btrfs-progs: Documentation: add T/P/E description for resize cmd
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com Reviewed-by: Satoru Takeuchi takeuchi_sat...@jp.fujitsu.com --- changelog v1-v2: s/\'E\'(EiB)/or \'E\'(EiB)/ as suggested by Satoru, thanks. --- Documentation/btrfs-filesystem.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/btrfs-filesystem.txt b/Documentation/btrfs-filesystem.txt index a8f2972..138ba6c 100644 --- a/Documentation/btrfs-filesystem.txt +++ b/Documentation/btrfs-filesystem.txt @@ -102,8 +102,8 @@ If the prefix + or - is present the size is increased or decreased by the quantity size. If no units are specified, the unit of the size parameter defaults to bytes. Optionally, the size parameter may be suffixed by one of the following -units designators: \'K\', \'M', or \'G', kilobytes, megabytes, or gigabytes, -respectively. +units designators: \'K\'(KiB), \'M\'(MiB), \'G\'(GiB), \'T\'(TiB), \'P\'(PiB) +or \'E\'(EiB). + If \'max' is passed, the filesystem will occupy all available space on the device devid. -- 1.8.1.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 v2] Fix wrong memory free on check_is_root
On Thu, 2014-12-18 at 16:39 +0900, Satoru Takeuchi wrote:
From: Satoru Takeuchi takeuchi_sat...@jp.fujitsu.com
When / is Btrfs, btrfs property subcommand / regards it
as non-root by mistake.
check_is_root() regards @object as a file system root if
the following two conditions are satisfied.
a) Both @object and its parent directory are Btrfs object
(file system root, subvolume, inode, and device
used for Btrfs).
b) fsid of the above mentioned two objects are different.
It doesn't work if @object is / because, in this case,
fsid of / and its parent (it's also /), are the same.
* Test environment
Two Btrfs file system (not subvolume) / and /home/sat/mnt.
* How to reproduce
Submit btrfs prop get against the above mentioned file systems.
* Test Result
** Actual result (without my patch)
==
# btrfs prop get /home/sat/mnt/
ro=false
label= # label is displayed because it's a file system root
# btrfs prop get /
ro=false # label is not displayed even if it's a file system
root
==
** Expected result (with my patch)
==
# ./btrfs-new prop get btrfs-auto-test/
# ./btrfs-new prop get /home/sat/mnt/
ro=false
label=
# ./btrfs-new prop get /
ro=false
label=foo# label is displayed
===
Signed-off-by: Satoru Takeuchi takeuchi_sat...@jp.fujitsu.com
Reported-by: Naohiro Aota na...@elisp.net
Looks good to me.
Reviewed-by: Gui Hecheng guihc.f...@cn.fujitsu.com
Thanks,
Gui
---
changelog
v2: Return a correct error code when realpath() fails.
Thank you Gui Hecheng for pointing my mistake out.
https://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg40204.html
---
cmds-property.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/cmds-property.c b/cmds-property.c
index a764293..6501338 100644
--- a/cmds-property.c
+++ b/cmds-property.c
@@ -124,7 +124,18 @@ static int check_is_root(const char *object)
int ret;
u8 fsid[BTRFS_FSID_SIZE];
u8 fsid2[BTRFS_FSID_SIZE];
- char *tmp;
+ char *tmp = NULL;
+ char *rp;
+
+ rp = realpath(object, NULL);
+ if (!rp) {
+ ret = -errno;
+ goto out;
+ }
+ if (!strcmp(rp, /)) {
+ ret = 0;
+ goto out;
+ }
tmp = malloc(strlen(object) + 5);
if (!tmp) {
@@ -165,6 +176,7 @@ static int check_is_root(const char *object)
out:
free(tmp);
+ free(rp);
return ret;
}
--
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-progs: remove uneccessary subvol name check for subvol deletion
When we want to delete a subvol, we first check to see whether it is
a subvolume or not. After the check, we are sure that it is a valid
subvol, don't have to check its name.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
cmds-subvolume.c | 15 ---
1 file changed, 15 deletions(-)
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index 53eec46..a35e2ad 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -281,21 +281,6 @@ again:
vname = basename(dupvname);
free(cpath);
- if (!test_issubvolname(vname)) {
- fprintf(stderr, ERROR: incorrect subvolume name '%s'\n,
- vname);
- ret = 1;
- goto out;
- }
-
- len = strlen(vname);
- if (len == 0 || len = BTRFS_VOL_NAME_MAX) {
- fprintf(stderr, ERROR: snapshot name too long '%s'\n,
- vname);
- ret = 1;
- goto out;
- }
-
fd = open_file_or_dir(dname, dirstream);
if (fd 0) {
fprintf(stderr, ERROR: can't access '%s'\n, dname);
--
1.8.1.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 1/2] Fix wrong memory free on check_is_root
On Thu, 2014-12-18 at 15:09 +0900, Satoru Takeuchi wrote:
From: Satoru Takeuchi takeuchi_sat...@jp.fujitsu.com
Date: Thu, 18 Dec 2014 14:35:22 +0900
@tmp is freed even if its allocation fails.
Signed-off-by: Satoru Takeuchi takeuchi_sat...@jp.fujitsu.com
---
cmds-property.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/cmds-property.c b/cmds-property.c
index a764293..a4bc127 100644
--- a/cmds-property.c
+++ b/cmds-property.c
@@ -140,31 +140,32 @@ static int check_is_root(const char *object)
Hi Satoru,
tmp = malloc()
if (!tmp) {
...
goto out;
}
Actually, if the malloc() fails, it returns a NULL, and the free()
handles the NULL internally.
I think there is no special need to deal with this case by the prog
itself.
Thanks,
Gui
if (ret 0) {
fprintf(stderr, ERROR: get_fsid for %s failed. %s\n, object,
strerror(-ret));
- goto out;
+ goto free_tmp_out;
}
ret = get_fsid(tmp, fsid2, 1);
if (ret == -ENOTTY) {
ret = 0;
- goto out;
+ goto free_tmp_out;
} else if (ret == -ENOTDIR) {
ret = 1;
- goto out;
+ goto free_tmp_out;
} else if (ret 0) {
fprintf(stderr, ERROR: get_fsid for %s failed. %s\n, tmp,
strerror(-ret));
- goto out;
+ goto free_tmp_out;
}
if (memcmp(fsid, fsid2, BTRFS_FSID_SIZE)) {
ret = 0;
- goto out;
+ goto free_tmp_out;
}
ret = 1;
-out:
+free_tmp_out:
free(tmp);
+out:
return ret;
}
--
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 2/2] btrfs-progs: fix the file system root is regarded as non-root
On Thu, 2014-12-18 at 15:42 +0900, Satoru Takeuchi wrote:
From: Satoru Takeuchi takeuchi_sat...@jp.fujitsu.com
When / is Btrfs, btrfs property subcommand / regards it
as non-root by mistake.
check_is_root() regards @object as a file system root if
the following two conditions are satisfied.
a) Both @object and its parent directory are Btrfs object
(file system root, subvolume, inode, and device
used for Btrfs).
b) fsid of the above mentioned two objects are different.
It doesn't work if @object is / because, in this case,
fsid of / and its parent (it's also /), are the same.
* Test environment
Two Btrfs file system (not subvolume) / and /home/sat/mnt.
* How to reproduce
Submit btrfs prop get against the above mentioned file systems.
* Test Result
** Actual result (without my patch)
==
# btrfs prop get /home/sat/mnt/
ro=false
label= # label is displayed because it's a file system root
# btrfs prop get /
ro=false # label is not displayed even if it's a file system
root
==
** Expected result (with my patch)
==
# ./btrfs-new prop get btrfs-auto-test/
# ./btrfs-new prop get /home/sat/mnt/
ro=false
label=
# ./btrfs-new prop get /
ro=false
label=foo# label is displayed
===
Signed-off-by: Satoru Takeuchi takeuchi_sat...@jp.fujitsu.com
Reported-by: Naohiro Aota na...@elisp.net
---
cmds-property.c | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/cmds-property.c b/cmds-property.c
index a4bc127..640af99 100644
--- a/cmds-property.c
+++ b/cmds-property.c
@@ -125,11 +125,22 @@ static int check_is_root(const char *object)
u8 fsid[BTRFS_FSID_SIZE];
u8 fsid2[BTRFS_FSID_SIZE];
char *tmp;
+ char *rp;
+
+ rp = realpath(object, NULL);
+ if (!rp) {
+ ret = -ENOMEM;
Hi Satoru，
IMO, ret = -errno may be better, because there are many possible return
values of realpath().
Thanks,
Gui
+ goto out;
+ }
+ if (!strcmp(rp, /)) {
+ ret = 0;
+ goto free_rp_out;
+ }
tmp = malloc(strlen(object) + 5);
if (!tmp) {
ret = -ENOMEM;
- goto out;
+ goto free_rp_out;
}
strcpy(tmp, object);
if (tmp[strlen(tmp) - 1] != '/')
@@ -165,6 +176,8 @@ static int check_is_root(const char *object)
free_tmp_out:
free(tmp);
+free_rp_out:
+ free(rp);
out:
return ret;
}
--
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: introduce shrinker for rb_tree that keeps valid btrfs_devices
The following patch:
btrfs: remove empty fs_devices to prevent memory runout
introduces @valid_dev_root aiming at recording @btrfs_device objects that
have corresponding block devices with btrfs.
But if a block device is broken or unplugged, no one tells the
@valid_dev_root to cleanup the dead objects.
To recycle the memory occuppied by those deads, we could rely on
the shrinker. The shrinker's scan function will traverse the
@valid_dev_root and trys to open the devices one by one, if it fails
or encounters a non-btrfs it will remove the dead @btrfs_device.
A special case to deal with is that a block device is unplugged and
replugged, then it appears with a new @bdev-bd_dev as devnum.
In this case, we should remove the older since we should have a new
one for that block device already.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
fs/btrfs/super.c | 10
fs/btrfs/volumes.c | 74 +-
fs/btrfs/volumes.h | 4 +++
3 files changed, 87 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index ee09a56..29069af 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1987,6 +1987,12 @@ static struct miscdevice btrfs_misc = {
.fops = btrfs_ctl_fops
};
+static struct shrinker btrfs_valid_dev_shrinker = {
+ .scan_objects = btrfs_valid_dev_scan,
+ .count_objects = btrfs_valid_dev_count,
+ .seeks = DEFAULT_SEEKS,
+};
+
MODULE_ALIAS_MISCDEV(BTRFS_MINOR);
MODULE_ALIAS(devname:btrfs-control);
@@ -2100,6 +2106,8 @@ static int __init init_btrfs_fs(void)
btrfs_init_lockdep();
+ register_shrinker(btrfs_valid_dev_shrinker);
+
btrfs_print_info();
err = btrfs_run_sanity_tests();
@@ -2113,6 +2121,7 @@ static int __init init_btrfs_fs(void)
return 0;
unregister_ioctl:
+ unregister_shrinker(btrfs_valid_dev_shrinker);
btrfs_interface_exit();
free_end_io_wq:
btrfs_end_io_wq_exit();
@@ -2153,6 +2162,7 @@ static void __exit exit_btrfs_fs(void)
btrfs_interface_exit();
btrfs_end_io_wq_exit();
unregister_filesystem(btrfs_fs_type);
+ unregister_shrinker(btrfs_valid_dev_shrinker);
btrfs_exit_sysfs();
btrfs_cleanup_valid_dev_root();
btrfs_cleanup_fs_uuids();
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 7093cce..62f37b1 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -54,6 +54,7 @@ static void btrfs_dev_stat_print_on_load(struct btrfs_device
*device);
DEFINE_MUTEX(uuid_mutex);
static LIST_HEAD(fs_uuids);
static struct rb_root valid_dev_root = RB_ROOT;
+static atomic_long_t unopened_dev_count = ATOMIC_LONG_INIT(0);
static struct btrfs_device *insert_valid_device(struct btrfs_device *new_dev)
{
@@ -130,6 +131,8 @@ static void free_invalid_device(struct btrfs_device
*invalid_dev)
{
struct btrfs_fs_devices *old_fs;
+ atomic_long_dec(unopened_dev_count);
+
old_fs = invalid_dev-fs_devices;
mutex_lock(old_fs-device_list_mutex);
list_del(invalid_dev-dev_list);
@@ -615,6 +618,7 @@ static noinline int device_list_add(const char *path,
list_add_rcu(device-dev_list, fs_devices-devices);
fs_devices-num_devices++;
mutex_unlock(fs_devices-device_list_mutex);
+ atomic_long_inc(unopened_dev_count);
ret = 1;
device-fs_devices = fs_devices;
@@ -788,6 +792,7 @@ again:
blkdev_put(device-bdev, device-mode);
device-bdev = NULL;
fs_devices-open_devices--;
+ atomic_long_inc(unopened_dev_count);
}
if (device-writeable) {
list_del_init(device-dev_alloc_list);
@@ -850,8 +855,10 @@ static int __btrfs_close_devices(struct btrfs_fs_devices
*fs_devices)
struct btrfs_device *new_device;
struct rcu_string *name;
- if (device-bdev)
+ if (device-bdev) {
fs_devices-open_devices--;
+ atomic_long_inc(unopened_dev_count);
+ }
if (device-writeable
device-devid != BTRFS_DEV_REPLACE_DEVID) {
@@ -981,6 +988,7 @@ static int __btrfs_open_devices(struct btrfs_fs_devices
*fs_devices,
fs_devices-rotating = 1;
fs_devices-open_devices++;
+ atomic_long_dec(unopened_dev_count);
if (device-writeable
device-devid != BTRFS_DEV_REPLACE_DEVID) {
fs_devices-rw_devices++;
@@ -6828,3 +6836,67 @@ void btrfs_update_commit_device_bytes_used(struct
btrfs_root *root,
}
unlock_chunks(root);
}
+
+static unsigned long shrink_valid_dev_root(void)
+{
+ struct rb_node *n;
+ struct btrfs_device *device;
+ struct
Re: btrfs fi show not accepting mount path as arg?
On Thu, 2014-12-04 at 19:20 +0530, Shriramana Sharma wrote: Using SuSE Tumbleweed. Observe: [samjnaa:~] sudo btrfs fi show root's password: Label: 'BRIHATII' uuid: 57836428-576e-466b-8a28-7961712867ab Total devices 1 FS bytes used 460.19GiB devid1 size 931.51GiB used 464.04GiB path /dev/sdb1 Btrfs v3.17+20141103 [samjnaa:~] sudo btrfs fi show /dev/sdb1 Label: 'BRIHATII' uuid: 57836428-576e-466b-8a28-7961712867ab Total devices 1 FS bytes used 460.16GiB devid1 size 931.51GiB used 464.04GiB path /dev/sdb1 Btrfs v3.17+20141103 [samjnaa:~] mount | grep btrfs /dev/sdb1 on /run/media/samjnaa/BRIHATII type btrfs (rw,nosuid,nodev,relatime,space_cache,uhelper=udisks2) [samjnaa:~] sudo btrfs fi show /run/media/samjnaa/BRIHATII/ Btrfs v3.17+20141103 [samjnaa:~] But the manpage of btrfs filesystem says that path is an acceptable argument to show, by which I understand to mean the mounted path of the filesystem (or a subvolume thereof?). Some problem here? Hi Shriramana Sharma, I think this problem has already got a fix, see below: https://patchwork.kernel.org/patch/5391131/ Thanks, Gui -- 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 v2] btrfs: remove empty fs_devices to prevent memory runout
There is a global list @fs_uuids to keep @fs_devices object
for each created btrfs. But when a btrfs becomes empty
(all devices belong to it are gone), its @fs_devices remains
in @fs_uuids list until module exit.
If we keeps mkfs.btrfs on the same device again and again,
all empty @fs_devices produced are sure to eat up our memory.
So this case has better to be prevented.
I think that each time we setup btrfs on that device, we should
check whether we are stealing some device from another btrfs
seen before. To faciliate the search procedure, we could insert
all @btrfs_device in a rb_root, one @btrfs_device per each physical
device, with @bdev-bd_dev as key. Each time device stealing happens,
we should replace the corresponding @btrfs_device in the rb_root with
an up-to-date version.
If the stolen device is the last device in its @fs_devices,
then we have an empty btrfs to be deleted.
Actually there are 3 ways to steal devices and lead to empty btrfs
1. mkfs, with -f option
2. device add, with -f option
3. device replace, with -f option
We should act under these cases.
Moreover, there are special cases to consider:
o If there are seed devices, then it is asured that
the devices in cloned @fs_devices are not treated as valid devices.
o If a device disappears and reappears without any touch, its
@bdev-bd_dev may change, so we have to re-insert it into the rb_root.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
changelog
v1-v2: add handle for device disappears and reappears event
*Note*
Actually this handles the case when a device disappears and
reappears without any touch.
We are going to recycle all dead btrfs_device in another patch.
Two events leads to the deads:
1) device disappears and never returns again
2) device disappears and returns with a new fs on it
A shrinker shall kill the deads.
---
fs/btrfs/super.c | 1 +
fs/btrfs/volumes.c | 281 ++---
fs/btrfs/volumes.h | 6 ++
3 files changed, 230 insertions(+), 58 deletions(-)
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 54bd91e..ee09a56 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2154,6 +2154,7 @@ static void __exit exit_btrfs_fs(void)
btrfs_end_io_wq_exit();
unregister_filesystem(btrfs_fs_type);
btrfs_exit_sysfs();
+ btrfs_cleanup_valid_dev_root();
btrfs_cleanup_fs_uuids();
btrfs_exit_compress();
btrfs_hash_exit();
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 0192051..7093cce 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -27,6 +27,7 @@
#include linux/kthread.h
#include linux/raid/pq.h
#include linux/semaphore.h
+#include linux/rbtree.h
#include asm/div64.h
#include ctree.h
#include extent_map.h
@@ -52,6 +53,126 @@ static void btrfs_dev_stat_print_on_load(struct
btrfs_device *device);
DEFINE_MUTEX(uuid_mutex);
static LIST_HEAD(fs_uuids);
+static struct rb_root valid_dev_root = RB_ROOT;
+
+static struct btrfs_device *insert_valid_device(struct btrfs_device *new_dev)
+{
+ struct rb_node **p;
+ struct rb_node *parent;
+ struct rb_node *new;
+ struct btrfs_device *old_dev;
+
+ WARN_ON(!mutex_is_locked(uuid_mutex));
+
+ parent = NULL;
+ new = new_dev-rb_node;
+
+ p = valid_dev_root.rb_node;
+ while (*p) {
+ parent = *p;
+ old_dev = rb_entry(parent, struct btrfs_device, rb_node);
+
+ if (new_dev-devnum old_dev-devnum)
+ p = parent-rb_left;
+ else if (new_dev-devnum old_dev-devnum)
+ p = parent-rb_right;
+ else {
+ rb_replace_node(parent, new, valid_dev_root);
+ RB_CLEAR_NODE(parent);
+
+ goto out;
+ }
+ }
+
+ old_dev = NULL;
+ rb_link_node(new, parent, p);
+ rb_insert_color(new, valid_dev_root);
+
+out:
+ return old_dev;
+}
+
+static void free_fs_devices(struct btrfs_fs_devices *fs_devices)
+{
+ struct btrfs_device *device;
+ WARN_ON(fs_devices-opened);
+ while (!list_empty(fs_devices-devices)) {
+ device = list_entry(fs_devices-devices.next,
+ struct btrfs_device, dev_list);
+ list_del(device-dev_list);
+ rcu_string_free(device-name);
+ kfree(device);
+ }
+ kfree(fs_devices);
+}
+
+static void remove_empty_fs_if_need(struct btrfs_fs_devices *old_fs)
+{
+ struct btrfs_fs_devices *seed_fs;
+
+ if (!list_empty(old_fs-devices))
+ return;
+
+ list_del(old_fs-list);
+
+ /* free the seed clones */
+ seed_fs = old_fs-seed;
+ free_fs_devices(old_fs);
+ while (seed_fs) {
+ old_fs = seed_fs
[PATCH] btrfs-progs: fix return value problem for btrfs sub show
If you exec:
# btrfs sub show dir == non-subvolume dir
The cmd print error messages as expected, but returns 0.
By convetion, it should return non-zero and we should explicitly
set it before it goto out.
With other pieces adopted:
1) removed a unnecessary return value set -EINVAL
2) fixed another code branch which may return 0 upon error.
3) with 2) applied, the ret = 0 follows can be removed
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
cmds-subvolume.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index fa58a24..53eec46 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -906,6 +906,7 @@ static int cmd_subvol_show(int argc, char **argv)
}
if (!ret) {
fprintf(stderr, ERROR: '%s' is not a subvolume\n, fullpath);
+ ret = 1;
goto out;
}
@@ -919,7 +920,6 @@ static int cmd_subvol_show(int argc, char **argv)
fprintf(stderr,
ERROR: %s doesn't belong to btrfs mount point\n,
fullpath);
- ret = -EINVAL;
goto out;
}
ret = 1;
@@ -958,13 +958,13 @@ static int cmd_subvol_show(int argc, char **argv)
memset(get_ri, 0, sizeof(get_ri));
get_ri.root_id = sv_id;
- if (btrfs_get_subvol(mntfd, get_ri)) {
+ ret = btrfs_get_subvol(mntfd, get_ri);
+ if (ret) {
fprintf(stderr, ERROR: can't find '%s'\n,
svpath);
goto out;
}
- ret = 0;
/* print the info */
printf(%s\n, fullpath);
printf(\tName: \t\t\t%s\n, get_ri.name);
--
1.8.1.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
[PATCH] btrfs-progs: apply realpath for btrfs fi show when mount point is given
For now,
# btrfs fi show /mnt/btrfs
gives info correctly, while
# btrfs fi show /mnt/btrfs/
gives nothing.
This implies that the @realpath() function should be applied to
unify the behavior.
Made a more clear comment right above the call as well.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
cmds-filesystem.c | 60 +++
1 file changed, 34 insertions(+), 26 deletions(-)
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index cd6b3c6..4e6d2f6 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -901,39 +901,47 @@ static int cmd_show(int argc, char **argv)
if (strlen(search) == 0)
usage(cmd_show_usage);
type = check_arg_type(search);
+
+ /*
+* For search is a device:
+* realpath do /dev/mapper/XX = /dev/dm-X
+* which is required by BTRFS_SCAN_DEV
+* For search is a mountpoint:
+* realpath do /mnt/btrfs/ = /mnt/btrfs
+* which shall be recognized by btrfs_scan_kernel()
+*/
+ if (!realpath(search, path)) {
+ fprintf(stderr, ERROR: Could not show %s: %s\n,
+ search, strerror(errno));
+ return 1;
+ }
+
+ search = path;
+
/*
* needs spl handling if input arg is block dev
* And if input arg is mount-point just print it
* right away
*/
- if (type == BTRFS_ARG_BLKDEV) {
- if (where == BTRFS_SCAN_LBLKID) {
- /* we need to do this because
-* legacy BTRFS_SCAN_DEV
-* provides /dev/dm-x paths
-*/
- if (realpath(search, path))
- search = path;
+ if (type == BTRFS_ARG_BLKDEV where != BTRFS_SCAN_LBLKID) {
+ ret = get_btrfs_mount(search,
+ mp, sizeof(mp));
+ if (!ret) {
+ /* given block dev is mounted*/
+ search = mp;
+ type = BTRFS_ARG_MNTPOINT;
} else {
- ret = get_btrfs_mount(search,
- mp, sizeof(mp));
- if (!ret) {
- /* given block dev is mounted*/
- search = mp;
- type = BTRFS_ARG_MNTPOINT;
- } else {
- ret = dev_to_fsid(search, fsid);
- if (ret) {
- fprintf(stderr,
- ERROR: No btrfs on
%s\n,
- search);
- return 1;
- }
- uuid_unparse(fsid, uuid_buf);
- search = uuid_buf;
- type = BTRFS_ARG_UUID;
- goto devs_only;
+ ret = dev_to_fsid(search, fsid);
+ if (ret) {
+ fprintf(stderr,
+ ERROR: No btrfs on %s\n,
+ search);
+ return 1;
}
+ uuid_unparse(fsid, uuid_buf);
+ search = uuid_buf;
+ type = BTRFS_ARG_UUID;
+ goto devs_only;
}
}
}
--
1.8.1.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 v4] btrfs-progs: fix page align issue for lzo compress in restore
On Tue, 2014-10-14 at 11:32 +0200, David Sterba wrote: On Tue, Oct 14, 2014 at 10:06:16AM +0200, Marc Dietrich wrote: This hasn't landed in an btrfs-progs branch I found. Any update? I had it tagged for review and found something that needs fixing. The PAGE_CACHE_SIZE is hardcoded to 4k, this will break on filesystems with larger sectors (eg. the powerpc machines). I'll scheudule the patch post 3.17, with a fix. Hi David， I note that this patch is not yet in the latest integration, how's the fix going? -Gui -- 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/2] btrfs-progs: remove dead condition for btrfs_map_block
The @search_cache_extent() only returns the next cache_extent or NULL,
it will never return the previous cache_extent.
So just remove the dead condition for previous cache_extent handle.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
volumes.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/volumes.c b/volumes.c
index 5b007fc..a1fd162 100644
--- a/volumes.c
+++ b/volumes.c
@@ -1320,7 +1320,7 @@ again:
kfree(multi);
return -ENOENT;
}
- if (ce-start logical || ce-start + ce-size logical) {
+ if (ce-start logical) {
kfree(multi);
return -ENOENT;
}
--
1.8.1.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
[PATCH 2/2] btrfs-progs: convert: fix unable to rollback case with removed empty block groups
Run fstests: btrfs/012 will fail with message:
unable to do rollback
It is because the rollback function checks sequentially each piece of space
to map to a certain block group. If some piece doesn't, rollback refuses to
continue.
After kernel commit:
commit 47ab2a6c689913db23ccae38349714edf8365e0a
Btrfs: remove empty block groups automatically
Empty block groups are removed, so there are possible gaps:
|--block group 1--| |--block group 2--|
^
|
gap
So the piece of space of the gap belongs to a removed empty block group,
and rollback should detect this case, and feel free to continue.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
btrfs-convert.c | 13 +++--
volumes.c | 2 ++
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/btrfs-convert.c b/btrfs-convert.c
index a544fc6..504c7b3 100644
--- a/btrfs-convert.c
+++ b/btrfs-convert.c
@@ -2368,8 +2368,17 @@ static int may_rollback(struct btrfs_root *root)
while (1) {
ret = btrfs_map_block(info-mapping_tree, WRITE, bytenr,
length, multi, 0, NULL);
- if (ret)
+ if (ret) {
+ if (ret == -ENOENT) {
+ /* removed block group at the tail */
+ if (length == (u64)-1)
+ break;
+
+ /* removed block group in the middle */
+ goto next;
+ }
goto fail;
+ }
num_stripes = multi-num_stripes;
physical = multi-stripes[0].physical;
@@ -2377,7 +2386,7 @@ static int may_rollback(struct btrfs_root *root)
if (num_stripes != 1 || physical != bytenr)
goto fail;
-
+next:
bytenr += length;
if (bytenr = total_bytes)
break;
diff --git a/volumes.c b/volumes.c
index a1fd162..a988cdb 100644
--- a/volumes.c
+++ b/volumes.c
@@ -1318,10 +1318,12 @@ again:
ce = search_cache_extent(map_tree-cache_tree, logical);
if (!ce) {
kfree(multi);
+ *length = (u64)-1;
return -ENOENT;
}
if (ce-start logical) {
kfree(multi);
+ *length = ce-start - logical;
return -ENOENT;
}
--
1.8.1.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: File test operator for subvols; possible bug in 'btrfs show directory'
On Tue, 2014-11-25 at 02:11 +, boris wrote: Hi all, I was looking for a quick method of testing whether a working directory is a subvolume. Couldn't see an obvious one, so tried 'btrfs show somesubvol≥'. It printed a fail message as expected but returned 0 exit status. Bug? Hi boris, I take a quick look at the code branch and I think it is a Bug indeed. THe code just forgot to set the proper return value before leave out. To test whether a dir is a subvolume, what about # btrfs sub list | grep dirname as 'btrfs sub list will list all subvolumes under mntpoint. The dirname should be relative path under mntpoint is as I could remember. Thanks, Gui Can I put in a feature request for a shell file test operator for subvols, please (or something of the kind)? http://tldp.org/LDP/abs/html/fto.html . The letter v (for volume, both upper and lower case forms; one for subvol, other for snapshots) is unused afaict. Have I missed the obvious? Or scored a false positive? TIA. -- 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: [PATCH] btrfs: remove empty fs_devices to prevent memory runout
On Fri, 2014-11-21 at 19:03 +0800, Anand Jain wrote:
Hi Gui,
Thanks for attempting this.
There was this patch previously attempted for the same problem,
which I had to nack..
[PATCH 1/2] btrfs: device list could grow infinite
I haven't seen your fix in full yet, But looks like you are using
dev_t to suffice the problem of identifying unique device. wonder
how does this would this react when device goes through the disappear
and reappear events ?
Thanks, Anand
Hi Anand,
Thanks for your comments.
Definitly we are dealing with the same problem, but in different ways.
And as you see there are cases relating to replace seed to handle.
With my patch, the device stealing problem which causes possible
memory leak has gone.
Oh, yes, the disappear reappear problem is really a piece missing.
Disappear reappear causes dev_t to change for the same physical
device, and there is certain to be dead btrfs_device objects that can
not be found by anybody in my rb_tree.
In my mind, check the rb_tree for deads every time it changes is
neccessary to handle the problem.
But there should be more efficient ways to go, any suggestions?
Thanks,
Gui
On 11/20/14 10:15 AM, Gui Hecheng wrote:
There is a global list @fs_uuids to keep @fs_devices object
for each created btrfs. But when a btrfs becomes empty
(all devices belong to it are gone), its @fs_devices remains
in @fs_uuids list until module exit.
If we keeps mkfs.btrfs on the same device again and again,
all empty @fs_devices produced are sure to eat up our memory.
So this case has better to be prevented.
I think that each time we setup btrfs on that device, we should
check whether we are stealing some device from another btrfs
seen before. To faciliate the search procedure, we could insert
all @btrfs_device in a rb_root, one @btrfs_device per each physical
device, with @bdev-bd_dev as key. Each time device stealing happens,
we should replace the corresponding @btrfs_device in the rb_root with
an up-to-date version.
If the stolen device is the last device in its @fs_devices,
then we have an empty btrfs to be deleted.
Actually there are 3 ways to steal devices and lead to empty btrfs
1. mkfs, with -f option
2. device add, with -f option
3. device replace, with -f option
We should act under these cases.
Moreover, if there are seed devices, then it is asured that
the devices in cloned @fs_devices are not treated as valid devices.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
fs/btrfs/super.c | 1 +
fs/btrfs/volumes.c | 181
-
fs/btrfs/volumes.h | 6 ++
3 files changed, 172 insertions(+), 16 deletions(-)
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 54bd91e..ee09a56 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2154,6 +2154,7 @@ static void __exit exit_btrfs_fs(void)
btrfs_end_io_wq_exit();
unregister_filesystem(btrfs_fs_type);
btrfs_exit_sysfs();
+ btrfs_cleanup_valid_dev_root();
btrfs_cleanup_fs_uuids();
btrfs_exit_compress();
btrfs_hash_exit();
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 0192051..ba86b1b 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -27,6 +27,7 @@
#include linux/kthread.h
#include linux/raid/pq.h
#include linux/semaphore.h
+#include linux/rbtree.h
#include asm/div64.h
#include ctree.h
#include extent_map.h
@@ -52,6 +53,120 @@ static void btrfs_dev_stat_print_on_load(struct
btrfs_device *device);
DEFINE_MUTEX(uuid_mutex);
static LIST_HEAD(fs_uuids);
+static struct rb_root valid_dev_root = RB_ROOT;
+
+static struct btrfs_device *insert_valid_device(struct btrfs_device
*new_dev)
+{
+ struct rb_node **p;
+ struct rb_node *parent;
+ struct rb_node *new;
+ struct btrfs_device *old_dev;
+
+ WARN_ON(!mutex_is_locked(uuid_mutex));
+
+ parent = NULL;
+ new = new_dev-rb_node;
+
+ p = valid_dev_root.rb_node;
+ while (*p) {
+ parent = *p;
+ old_dev = rb_entry(parent, struct btrfs_device, rb_node);
+
+ if (new_dev-devnum old_dev-devnum)
+ p = parent-rb_left;
+ else if (new_dev-devnum old_dev-devnum)
+ p = parent-rb_right;
+ else {
+ rb_replace_node(parent, new, valid_dev_root);
+ RB_CLEAR_NODE(parent);
+
+ goto out;
+ }
+ }
+
+ old_dev = NULL;
+ rb_link_node(new, parent, p);
+ rb_insert_color(new, valid_dev_root);
+
+out:
+ return old_dev;
+}
+
+static void free_fs_devices(struct btrfs_fs_devices *fs_devices)
+{
+ struct btrfs_device *device;
+ WARN_ON(fs_devices-opened);
+ while (!list_empty(fs_devices-devices)) {
+ device
[PATCH] btrfs: remove empty fs_devices to prevent memory runout
There is a global list @fs_uuids to keep @fs_devices object
for each created btrfs. But when a btrfs becomes empty
(all devices belong to it are gone), its @fs_devices remains
in @fs_uuids list until module exit.
If we keeps mkfs.btrfs on the same device again and again,
all empty @fs_devices produced are sure to eat up our memory.
So this case has better to be prevented.
I think that each time we setup btrfs on that device, we should
check whether we are stealing some device from another btrfs
seen before. To faciliate the search procedure, we could insert
all @btrfs_device in a rb_root, one @btrfs_device per each physical
device, with @bdev-bd_dev as key. Each time device stealing happens,
we should replace the corresponding @btrfs_device in the rb_root with
an up-to-date version.
If the stolen device is the last device in its @fs_devices,
then we have an empty btrfs to be deleted.
Actually there are 3 ways to steal devices and lead to empty btrfs
1. mkfs, with -f option
2. device add, with -f option
3. device replace, with -f option
We should act under these cases.
Moreover, if there are seed devices, then it is asured that
the devices in cloned @fs_devices are not treated as valid devices.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
fs/btrfs/super.c | 1 +
fs/btrfs/volumes.c | 181 -
fs/btrfs/volumes.h | 6 ++
3 files changed, 172 insertions(+), 16 deletions(-)
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 54bd91e..ee09a56 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2154,6 +2154,7 @@ static void __exit exit_btrfs_fs(void)
btrfs_end_io_wq_exit();
unregister_filesystem(btrfs_fs_type);
btrfs_exit_sysfs();
+ btrfs_cleanup_valid_dev_root();
btrfs_cleanup_fs_uuids();
btrfs_exit_compress();
btrfs_hash_exit();
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 0192051..ba86b1b 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -27,6 +27,7 @@
#include linux/kthread.h
#include linux/raid/pq.h
#include linux/semaphore.h
+#include linux/rbtree.h
#include asm/div64.h
#include ctree.h
#include extent_map.h
@@ -52,6 +53,120 @@ static void btrfs_dev_stat_print_on_load(struct
btrfs_device *device);
DEFINE_MUTEX(uuid_mutex);
static LIST_HEAD(fs_uuids);
+static struct rb_root valid_dev_root = RB_ROOT;
+
+static struct btrfs_device *insert_valid_device(struct btrfs_device *new_dev)
+{
+ struct rb_node **p;
+ struct rb_node *parent;
+ struct rb_node *new;
+ struct btrfs_device *old_dev;
+
+ WARN_ON(!mutex_is_locked(uuid_mutex));
+
+ parent = NULL;
+ new = new_dev-rb_node;
+
+ p = valid_dev_root.rb_node;
+ while (*p) {
+ parent = *p;
+ old_dev = rb_entry(parent, struct btrfs_device, rb_node);
+
+ if (new_dev-devnum old_dev-devnum)
+ p = parent-rb_left;
+ else if (new_dev-devnum old_dev-devnum)
+ p = parent-rb_right;
+ else {
+ rb_replace_node(parent, new, valid_dev_root);
+ RB_CLEAR_NODE(parent);
+
+ goto out;
+ }
+ }
+
+ old_dev = NULL;
+ rb_link_node(new, parent, p);
+ rb_insert_color(new, valid_dev_root);
+
+out:
+ return old_dev;
+}
+
+static void free_fs_devices(struct btrfs_fs_devices *fs_devices)
+{
+ struct btrfs_device *device;
+ WARN_ON(fs_devices-opened);
+ while (!list_empty(fs_devices-devices)) {
+ device = list_entry(fs_devices-devices.next,
+ struct btrfs_device, dev_list);
+ list_del(device-dev_list);
+ rcu_string_free(device-name);
+ kfree(device);
+ }
+ kfree(fs_devices);
+}
+
+static void remove_empty_fs_if_need(struct btrfs_fs_devices *old_fs)
+{
+ struct btrfs_fs_devices *seed_fs;
+
+ if (!list_empty(old_fs-devices))
+ return;
+
+ list_del(old_fs-list);
+
+ /* free the seed clones */
+ seed_fs = old_fs-seed;
+ free_fs_devices(old_fs);
+ while (seed_fs) {
+ old_fs = seed_fs;
+ seed_fs = seed_fs-seed;
+ free_fs_devices(old_fs);
+ }
+
+}
+
+static void replace_invalid_device(struct btrfs_device *new_dev)
+{
+ struct btrfs_device *invalid_dev;
+ struct btrfs_fs_devices *old_fs;
+
+ WARN_ON(!mutex_is_locked(uuid_mutex));
+
+ invalid_dev = insert_valid_device(new_dev);
+ if (!invalid_dev)
+ return;
+
+ old_fs = invalid_dev-fs_devices;
+ mutex_lock(old_fs-device_list_mutex);
+ list_del(invalid_dev-dev_list);
+ rcu_string_free(invalid_dev-name);
+ kfree(invalid_dev);
+ mutex_unlock(old_fs-device_list_mutex
[PATCH v2 2/2] btrfs-progs: fix wrong num_devices for btrfs fi show with seed devices
The @fi_args-num_devices in @get_fs_info() does not include seed devices.
We could just correct it by searching the chunk tree and count how
many dev_items there are in total which includes seed devices.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
changelog
v1-v2: adopt more clear function name
@find_max_device_id()
@search_chunk_tree_for_fs_info()
---
utils.c | 76 +
1 file changed, 76 insertions(+)
diff --git a/utils.c b/utils.c
index f3fca4f..0d3d7fa 100644
--- a/utils.c
+++ b/utils.c
@@ -1812,6 +1812,75 @@ int get_device_info(int fd, u64 devid,
return ret ? -errno : 0;
}
+static u64 find_max_device_id(struct btrfs_ioctl_search_args *search_args,
+ int nr_items)
+{
+ struct btrfs_dev_item *dev_item;
+ char *buf = search_args-buf;
+
+ buf += (nr_items - 1) * (sizeof(struct btrfs_ioctl_search_header)
+ + sizeof(struct btrfs_dev_item));
+ buf += sizeof(struct btrfs_ioctl_search_header);
+
+ dev_item = (struct btrfs_dev_item *)buf;
+
+ return btrfs_stack_device_id(dev_item);
+}
+
+static int search_chunk_tree_for_fs_info(int fd,
+ struct btrfs_ioctl_fs_info_args *fi_args)
+{
+ int ret;
+ int max_items;
+ u64 start_devid = 1;
+ struct btrfs_ioctl_search_args search_args;
+ struct btrfs_ioctl_search_key *search_key = search_args.key;
+
+ fi_args-num_devices = 0;
+
+ max_items = BTRFS_SEARCH_ARGS_BUFSIZE
+ / (sizeof(struct btrfs_ioctl_search_header)
+ + sizeof(struct btrfs_dev_item));
+
+ search_key-tree_id = BTRFS_CHUNK_TREE_OBJECTID;
+ search_key-min_objectid = BTRFS_DEV_ITEMS_OBJECTID;
+ search_key-max_objectid = BTRFS_DEV_ITEMS_OBJECTID;
+ search_key-min_type = BTRFS_DEV_ITEM_KEY;
+ search_key-max_type = BTRFS_DEV_ITEM_KEY;
+ search_key-min_transid = 0;
+ search_key-max_transid = (u64)-1;
+ search_key-nr_items = max_items;
+ search_key-max_offset = (u64)-1;
+
+again:
+ search_key-min_offset = start_devid;
+
+ ret = ioctl(fd, BTRFS_IOC_TREE_SEARCH, search_args);
+ if (ret 0)
+ return -errno;
+
+ fi_args-num_devices += (u64)search_key-nr_items;
+
+ if (search_key-nr_items == max_items) {
+ start_devid = find_max_device_id(search_args,
+ search_key-nr_items) + 1;
+ goto again;
+ }
+
+ /* get the lastest max_id to stay consistent with the num_devices */
+ if (search_key-nr_items == 0)
+ /*
+* last tree_search returns an empty buf, use the devid of
+* the last dev_item of the previous tree_search
+*/
+ fi_args-max_id = start_devid - 1;
+ else
+ fi_args-max_id = find_max_device_id(search_args,
+ search_key-nr_items);
+
+ return 0;
+}
+
/*
* For a given path, fill in the ioctl fs_ and info_ args.
* If the path is a btrfs mountpoint, fill info for all devices.
@@ -1891,6 +1960,13 @@ int get_fs_info(char *path, struct
btrfs_ioctl_fs_info_args *fi_args,
ret = -errno;
goto out;
}
+
+ /*
+* The fs_args-num_devices does not include seed devices
+*/
+ ret = search_chunk_tree_for_fs_info(fd, fi_args);
+ if (ret)
+ goto out;
}
if (!fi_args-num_devices)
--
1.8.1.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
[PATCH] btrfs-progs: use canonical name for device in btrfs fi show when mounted
When using lvm volumes to check fstests: btrfs/006, it fails like:
Label: 'TestLabel.006' uuid: UUID
Total devices EXACTNUM FS bytes used SIZE
devid DEVID size SIZE used SIZE path SCRATCH_DEV
+ devid DEVID size SIZE used SIZE path /dev/dm-4
+ devid DEVID size SIZE used SIZE path /dev/dm-5
+ devid DEVID size SIZE used SIZE path /dev/dm-6
The /dev/dm-* points to lvm volumes, use @canonicalize_path() to convert them
and we will make it through. Of course we should do the same thing for dev stat.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
cmds-device.c | 16 +++-
cmds-filesystem.c | 7 ++-
2 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/cmds-device.c b/cmds-device.c
index 9323986..6cd41e1 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -408,31 +408,37 @@ static int cmd_dev_stats(int argc, char **argv)
path, strerror(errno));
err = 1;
} else {
+ char *canonical_path;
+
+ canonical_path = canonicalize_path((char *)path);
+
if (args.nr_items = BTRFS_DEV_STAT_WRITE_ERRS + 1)
printf([%s].write_io_errs %llu\n,
- path,
+ canonical_path,
(unsigned long long) args.values[
BTRFS_DEV_STAT_WRITE_ERRS]);
if (args.nr_items = BTRFS_DEV_STAT_READ_ERRS + 1)
printf([%s].read_io_errs%llu\n,
- path,
+ canonical_path,
(unsigned long long) args.values[
BTRFS_DEV_STAT_READ_ERRS]);
if (args.nr_items = BTRFS_DEV_STAT_FLUSH_ERRS + 1)
printf([%s].flush_io_errs %llu\n,
- path,
+ canonical_path,
(unsigned long long) args.values[
BTRFS_DEV_STAT_FLUSH_ERRS]);
if (args.nr_items = BTRFS_DEV_STAT_CORRUPTION_ERRS + 1)
printf([%s].corruption_errs %llu\n,
- path,
+ canonical_path,
(unsigned long long) args.values[
BTRFS_DEV_STAT_CORRUPTION_ERRS]);
if (args.nr_items = BTRFS_DEV_STAT_GENERATION_ERRS + 1)
printf([%s].generation_errs %llu\n,
- path,
+ canonical_path,
(unsigned long long) args.values[
BTRFS_DEV_STAT_GENERATION_ERRS]);
+
+ free(canonical_path);
}
}
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index e4b2785..cd6b3c6 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -510,7 +510,10 @@ static int print_one_fs(struct btrfs_ioctl_fs_info_args
*fs_info,
pretty_size(calc_used_bytes(space_info)));
for (i = 0; i fs_info-num_devices; i++) {
+ char *canonical_path;
+
tmp_dev_info = (struct btrfs_ioctl_dev_info_args *)dev_info[i];
+ canonical_path = canonicalize_path((char *)tmp_dev_info-path);
/* Add check for missing devices even mounted */
fd = open((char *)tmp_dev_info-path, O_RDONLY);
@@ -523,7 +526,9 @@ static int print_one_fs(struct btrfs_ioctl_fs_info_args
*fs_info,
tmp_dev_info-devid,
pretty_size(tmp_dev_info-total_bytes),
pretty_size(tmp_dev_info-bytes_used),
- tmp_dev_info-path);
+ canonical_path);
+
+ free(canonical_path);
}
if (missing)
--
1.8.1.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
[PATCH v3] btrfs-progs: skip fs with no seed when build seed/sprout mapping for fi show
There is no need to try to build seed/sprout mapping for those btrfs
without seed devices, so just skip such fs.
We could get the total number of devices from the disk super block, if it
equals the number of items in list @fs_devices-devices, then there shouldn't
be any seed devices.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
changlog
v1-v2: adopt more natural function name:
no_seed_devices == has_seed_devices
v2-v3: skip such fs after copied, otherwise such fs won't be shown
---
cmds-filesystem.c | 25 +++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index e4b2785..97efdee 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -750,6 +750,22 @@ static int find_and_copy_seed(struct btrfs_fs_devices
*seed,
return 1;
}
+static int has_seed_devices(struct btrfs_fs_devices *fs_devices)
+{
+ struct btrfs_device *device;
+ int dev_cnt_total, dev_cnt = 0;
+
+ device = list_first_entry(fs_devices-devices, struct btrfs_device,
+ dev_list);
+
+ dev_cnt_total = device-total_devs;
+
+ list_for_each_entry(device, fs_devices-devices, dev_list)
+ dev_cnt++;
+
+ return dev_cnt_total != dev_cnt;
+}
+
static int map_seed_devices(struct list_head *all_uuids,
char *search, int *found)
{
@@ -799,6 +815,11 @@ static int map_seed_devices(struct list_head *all_uuids,
struct btrfs_device, dev_list);
if (!device)
continue;
+
+ /* skip fs without seeds */
+ if (!has_seed_devices(cur_fs))
+ continue;
+
/*
* open_ctree_* detects seed/sprout mapping
*/
@@ -956,8 +977,8 @@ devs_only:
}
/*
-* scan_for_btrfs() don't build seed/sprout mapping,
-* do mapping build for each scanned fs here
+* The seed/sprout mapping are not detected yet,
+* do mapping build for all umounted fs
*/
ret = map_seed_devices(all_uuids, search, found);
if (ret) {
--
1.8.1.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
[PATCH] btrfs: fix dead lock while running replace and defrag concurrently
This can be reproduced by fstests: btrfs/070
The scenario is like the following:
replace worker thread defrag thread
- -
copy_nocow_pages_worker btrfs_defrag_file
copy_nocow_pages_for_inode...
btrfs_writepages
|A| lock_extent_bits extent_write_cache_pages
|B| lock_page
__extent_writepage
... writepage_delalloc
find_lock_delalloc_range
|B| lock_extent_bits
find_or_create_page
pagecache_get_page
|A| lock_page
This leads to an ABBA pattern deadlock. To fix it,
o we just change it to an AABB pattern which means to @unlock_extent_bits()
before we @lock_page(), and in this way the @extent_read_full_page_nolock()
is no longer in an locked context, so change it back to
@extent_read_full_page()
to regain protection.
o Since we @unlock_extent_bits() earlier, then before @write_page_nocow(),
the extent may not really point at the physical block we want, so we
have to check it before write.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
fs/btrfs/scrub.c | 90 +---
1 file changed, 60 insertions(+), 30 deletions(-)
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index efa0831..4325bb0 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -3310,6 +3310,50 @@ out:
scrub_pending_trans_workers_dec(sctx);
}
+static int check_extent_to_block(struct inode *inode, u64 start, u64 len,
+u64 logical)
+{
+ struct extent_state *cached_state = NULL;
+ struct btrfs_ordered_extent *ordered;
+ struct extent_io_tree *io_tree;
+ struct extent_map *em;
+ u64 lockstart = start, lockend = start + len - 1;
+ int ret = 0;
+
+ io_tree = BTRFS_I(inode)-io_tree;
+
+ lock_extent_bits(io_tree, lockstart, lockend, 0, cached_state);
+ ordered = btrfs_lookup_ordered_range(inode, lockstart, len);
+ if (ordered) {
+ btrfs_put_ordered_extent(ordered);
+ ret = 1;
+ goto out_unlock;
+ }
+
+ em = btrfs_get_extent(inode, NULL, 0, start, len, 0);
+ if (IS_ERR(em)) {
+ ret = PTR_ERR(em);
+ goto out_unlock;
+ }
+
+ /*
+* This extent does not actually cover the logical extent anymore,
+* move on to the next inode.
+*/
+ if (em-block_start logical ||
+ em-block_start + em-block_len logical + len) {
+ free_extent_map(em);
+ ret = 1;
+ goto out_unlock;
+ }
+ free_extent_map(em);
+
+out_unlock:
+ unlock_extent_cached(io_tree, lockstart, lockend, cached_state,
+GFP_NOFS);
+ return ret;
+}
+
static int copy_nocow_pages_for_inode(u64 inum, u64 offset, u64 root,
struct scrub_copy_nocow_ctx *nocow_ctx)
{
@@ -3318,13 +3362,10 @@ static int copy_nocow_pages_for_inode(u64 inum, u64
offset, u64 root,
struct inode *inode;
struct page *page;
struct btrfs_root *local_root;
- struct btrfs_ordered_extent *ordered;
- struct extent_map *em;
- struct extent_state *cached_state = NULL;
struct extent_io_tree *io_tree;
u64 physical_for_dev_replace;
+ u64 nocow_ctx_logical;
u64 len = nocow_ctx-len;
- u64 lockstart = offset, lockend = offset + len - 1;
unsigned long index;
int srcu_index;
int ret = 0;
@@ -3356,30 +3397,13 @@ static int copy_nocow_pages_for_inode(u64 inum, u64
offset, u64 root,
physical_for_dev_replace = nocow_ctx-physical_for_dev_replace;
io_tree = BTRFS_I(inode)-io_tree;
+ nocow_ctx_logical = nocow_ctx-logical;
- lock_extent_bits(io_tree, lockstart, lockend, 0, cached_state);
- ordered = btrfs_lookup_ordered_range(inode, lockstart, len);
- if (ordered) {
- btrfs_put_ordered_extent(ordered);
- goto out_unlock;
- }
-
- em = btrfs_get_extent(inode, NULL, 0, lockstart, len, 0);
- if (IS_ERR(em)) {
- ret = PTR_ERR(em);
- goto out_unlock;
- }
-
- /*
-* This extent does not actually cover the logical extent anymore,
-* move on to the next inode.
-*/
- if (em-block_start nocow_ctx-logical ||
- em-block_start + em-block_len nocow_ctx-logical + len) {
- free_extent_map(em);
- goto out_unlock;
+ ret = check_extent_to_block(inode, offset, len, nocow_ctx_logical);
+ if (ret) {
+ ret = ret 0 ? 0 : ret;
+ goto out;
}
- free_extent_map(em);
while (len
Re: [PATCH 2/2] btrfs-progs: fix wrong num_devices for btrfs fi show with seed devices
On Sat, 2014-11-08 at 19:47 +, Mike Fleetwood wrote: On 7 November 2014 18:16, David Sterba dste...@suse.cz wrote: On Fri, Nov 07, 2014 at 10:07:43AM +0800, Gui Hecheng wrote: The @fi_args-num_devices in @get_fs_info() does not include seed devices. We could just correct it by searching the chunk tree and count how many dev_items there are in total which includes seed devices. Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com --- *Note* This is just a temporary workaround to fix this problem in order to make users happy, because a new ioctl or sysfs interface to handle this problem needs more discussions and efforts. After the work implemented and accepted, we could drop this. Nice, thanks. I agree that this kind of workaround is best possible for the moment, and I'm glad to see that it's not that much code to get the seeding devices right. This would also work with older kernels without the updated sysfs/ioctl interfaces, so this is likely to stay for a long time. +u64 find_max_id(struct btrfs_ioctl_search_args *search_args, int nr_items) That's a very generic name for a function that does a very specialized thing, but I don't have a suggestion right now. Is find_max_device_id a suitable name? I think this one is really more clear. +int correct_fs_info(int fd, struct btrfs_ioctl_fs_info_args *fi_args) Same here, make fs_info correct but in what way? A comment would be good as well. Sorry, no suggestion for this function name. may be @search_chunk_tree_for_fs_info? Thanks for the suggestions. -Gui Thanks, Mike -- 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 v2] btrfs-progs: skip fs with no seed when build seed/sprout mapping for fi show
There is no need to try to build seed/sprout mapping for those btrfs
without seed devices, so just skip such fs.
We could get the total number of devices from the disk super block, if it
equals the number of items in list @fs_devices-devices, then there shouldn't
be any seed devices.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
changlog
v1-v2: adopt more natural function name:
no_seed_devices == has_seed_devices
---
cmds-filesystem.c | 24 ++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index e4b2785..c55c486 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -750,6 +750,22 @@ static int find_and_copy_seed(struct btrfs_fs_devices
*seed,
return 1;
}
+static int has_seed_devices(struct btrfs_fs_devices *fs_devices)
+{
+ struct btrfs_device *device;
+ int dev_cnt_total, dev_cnt = 0;
+
+ device = list_first_entry(fs_devices-devices, struct btrfs_device,
+ dev_list);
+
+ dev_cnt_total = device-total_devs;
+
+ list_for_each_entry(device, fs_devices-devices, dev_list)
+ dev_cnt++;
+
+ return dev_cnt_total != dev_cnt;
+}
+
static int map_seed_devices(struct list_head *all_uuids,
char *search, int *found)
{
@@ -775,6 +791,10 @@ static int map_seed_devices(struct list_head *all_uuids,
*found = 1;
}
+ /* skip fs without seeds */
+ if (!has_seed_devices(cur_fs))
+ continue;
+
/* skip all fs already shown as mounted fs */
if (is_seen_fsid(cur_fs-fsid))
continue;
@@ -956,8 +976,8 @@ devs_only:
}
/*
-* scan_for_btrfs() don't build seed/sprout mapping,
-* do mapping build for each scanned fs here
+* The seed/sprout mapping are not detected yet,
+* do mapping build for all umounted fs
*/
ret = map_seed_devices(all_uuids, search, found);
if (ret) {
--
1.8.1.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
[PATCH] btrfs-progs: skip fs with no seed when build seed/sprout mapping for fi show
There is no need to try to build seed/sprout mapping for those btrfs
without seed devices, so just skip such fs.
We could get the total number of devices from the disk super block, if it
equals the number of items in list @fs_devices-devices, then there shouldn't
be any seed devices.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
cmds-filesystem.c | 25 +++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 67fe52b..0d49cd1 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -795,6 +795,22 @@ out:
return ret;
}
+static int no_seed_devices(struct btrfs_fs_devices *fs_devices)
+{
+ struct btrfs_device *device;
+ int dev_cnt_total, dev_cnt = 0;
+
+ device = list_first_entry(fs_devices-devices, struct btrfs_device,
+ dev_list);
+
+ dev_cnt_total = device-total_devs;
+
+ list_for_each_entry(device, fs_devices-devices, dev_list)
+ dev_cnt++;
+
+ return dev_cnt_total == dev_cnt;
+}
+
static int map_seed_devices(struct list_head *all_uuids)
{
struct btrfs_fs_devices *cur_fs, *cur_seed;
@@ -812,6 +828,11 @@ static int map_seed_devices(struct list_head *all_uuids)
struct btrfs_device, dev_list);
if (!device)
continue;
+
+ /* skip fs without seeds */
+ if (no_seed_devices(cur_fs))
+ continue;
+
/*
* open_ctree_* detects seed/sprout mapping
*/
@@ -976,8 +997,8 @@ devs_only:
}
/*
-* scan_for_btrfs() don't build seed/sprout mapping,
-* do mapping build for each scanned fs here
+* The seed/sprout mapping are not detected yet,
+* do mapping build for all umounted fs
*/
ret = map_seed_devices(all_uuids);
if (ret) {
--
1.8.1.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
[PATCH 2/2] btrfs-progs: fix wrong num_devices for btrfs fi show with seed devices
The @fi_args-num_devices in @get_fs_info() does not include seed devices.
We could just correct it by searching the chunk tree and count how
many dev_items there are in total which includes seed devices.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
*Note*
This is just a temporary workaround to fix this problem in order to
make users happy, because a new ioctl or sysfs interface to handle this
problem needs more discussions and efforts. After the work implemented
and accepted, we could drop this.
---
utils.c | 74 +
1 file changed, 74 insertions(+)
diff --git a/utils.c b/utils.c
index 9bcc1a0..a6dcb8a 100644
--- a/utils.c
+++ b/utils.c
@@ -1781,6 +1781,73 @@ int get_device_info(int fd, u64 devid,
return ret ? -errno : 0;
}
+u64 find_max_id(struct btrfs_ioctl_search_args *search_args, int nr_items)
+{
+ struct btrfs_dev_item *dev_item;
+ char *buf = search_args-buf;
+
+ buf += (nr_items - 1) * (sizeof(struct btrfs_ioctl_search_header)
+ + sizeof(struct btrfs_dev_item));
+ buf += sizeof(struct btrfs_ioctl_search_header);
+
+ dev_item = (struct btrfs_dev_item *)buf;
+
+ return btrfs_stack_device_id(dev_item);
+}
+
+int correct_fs_info(int fd, struct btrfs_ioctl_fs_info_args *fi_args)
+{
+ int ret;
+ int max_items;
+ u64 start_devid = 1;
+ struct btrfs_ioctl_search_args search_args;
+ struct btrfs_ioctl_search_key *search_key = search_args.key;
+
+ fi_args-num_devices = 0;
+
+ max_items = BTRFS_SEARCH_ARGS_BUFSIZE
+ / (sizeof(struct btrfs_ioctl_search_header)
+ + sizeof(struct btrfs_dev_item));
+
+ search_key-tree_id = BTRFS_CHUNK_TREE_OBJECTID;
+ search_key-min_objectid = BTRFS_DEV_ITEMS_OBJECTID;
+ search_key-max_objectid = BTRFS_DEV_ITEMS_OBJECTID;
+ search_key-min_type = BTRFS_DEV_ITEM_KEY;
+ search_key-max_type = BTRFS_DEV_ITEM_KEY;
+ search_key-min_transid = 0;
+ search_key-max_transid = (u64)-1;
+ search_key-nr_items = max_items;
+ search_key-max_offset = (u64)-1;
+
+again:
+ search_key-min_offset = start_devid;
+
+ ret = ioctl(fd, BTRFS_IOC_TREE_SEARCH, search_args);
+ if (ret 0)
+ return -errno;
+
+ fi_args-num_devices += (u64)search_key-nr_items;
+
+ if (search_key-nr_items == max_items) {
+ start_devid = find_max_id(search_args,
+ search_key-nr_items) + 1;
+ goto again;
+ }
+
+ /* get the lastest max_id to stay consistent with the num_devices */
+ if (search_key-nr_items == 0)
+ /*
+* last tree_search returns an empty buf, use the devid of
+* the last dev_item of the previous tree_search
+*/
+ fi_args-max_id = start_devid - 1;
+ else
+ fi_args-max_id = find_max_id(search_args,
+ search_key-nr_items);
+
+ return 0;
+}
+
/*
* For a given path, fill in the ioctl fs_ and info_ args.
* If the path is a btrfs mountpoint, fill info for all devices.
@@ -1860,6 +1927,13 @@ int get_fs_info(char *path, struct
btrfs_ioctl_fs_info_args *fi_args,
ret = -errno;
goto out;
}
+
+ /*
+* The fs_args-num_devices does not include seed devices
+*/
+ ret = correct_fs_info(fd, fi_args);
+ if (ret)
+ goto out;
}
if (!fi_args-num_devices)
--
1.8.1.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
[PATCH 1/2] btrfs-progs: remove BUG_ON on num of devices for btrfs fi show
The following BUG_ON:
BUG_ON(ndevs = fi_args-num_devices)
is not needed, because it always fails with seed devices present.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
utils.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/utils.c b/utils.c
index f10c178..9bcc1a0 100644
--- a/utils.c
+++ b/utils.c
@@ -1889,7 +1889,6 @@ int get_fs_info(char *path, struct
btrfs_ioctl_fs_info_args *fi_args,
i++;
for (; i = fi_args-max_id; ++i) {
- BUG_ON(ndevs = fi_args-num_devices);
ret = get_device_info(fd, i, di_args[ndevs]);
if (ret == -ENODEV)
continue;
--
1.8.1.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: read block failed check_tree_block / Couldn't read chunk tree
On Wed, 2014-10-29 at 11:45 +0800, Anand Jain wrote: this is (most likely) due to patch below, commit 915902c5002485fb13d27c4b699a73fb66cc0f09 btrfs-progs: fix device missing of btrfs fi show with seed devices Could you try to back out the patch from progs and give it a shot ? and pls report what you see. Thanks. Hi Anand, --- # btrfs fi show Label: 'mythstorage' uuid: 9b454272-6800-4b3c-b196-9e180407a6cb Total devices 1 FS bytes used 2.36MiB devid1 size 931.51GiB used 10.04GiB path /dev/sdd1 [errors from open_ctree] Label: none uuid: 8ef575d9-2465-479c-bf9c-067e2e417770 Total devices 3 FS bytes used 3.02TiB devid1 size 2.73TiB used 1.77TiB path /dev/sdb1 devid2 size 2.73TiB used 1.77TiB path /dev/sda1 devid3 size 2.73TiB used 1.77TiB path /dev/sdc1 Btrfs v3.17 -- The original output of btrfs fi show actually detect all devices in all btrfs successfully. I think the patch mentioned above really missed some case in which num of devices found match the num of all devices in one btrfs. In such case, we should not bother trying to detect seed devices. And for now, we did. Unfortunately, the btrfs is some what corrupted by mkfs.ntfs, so @open_ctree call complains about corruption on the chunk root like the following: Check tree block failed, want=5845480062976, have=0 Check tree block failed, want=5845480062976, have=0 Check tree block failed, want=5845480062976, have=65536 Check tree block failed, want=5845480062976, have=0 Check tree block failed, want=5845480062976, have=0 read block failed check_tree_block Couldn't read chunk tree I'll fix this case by avoiding @open_ctree call when all devices in btrfs are detected. But if not all devices are detected, then there are 3 possible cases: 1) seed devices present 2) disk unpluged 3) disk reformatted For case 1), *for now* we need the @open_ctree to detect them(After your former proposal implemented, things may be different). For case 2) 3), @open_ctree complains as above. To clean the not so helpful and annoying complains, I think we could keep @open_ctree silent when called by btrfs fi show by introduce a new flag OPEN_CTREE_SILENT. Thanks Gui On 10/25/14 00:43, Rene Thomas wrote: # btrfs --version Btrfs v3.17 # btrfs fi show Label: 'mythstorage' uuid: 9b454272-6800-4b3c-b196-9e180407a6cb Total devices 1 FS bytes used 2.36MiB devid1 size 931.51GiB used 10.04GiB path /dev/sdd1 Check tree block failed, want=5845480062976, have=0 Check tree block failed, want=5845480062976, have=0 Check tree block failed, want=5845480062976, have=65536 Check tree block failed, want=5845480062976, have=0 Check tree block failed, want=5845480062976, have=0 read block failed check_tree_block Couldn't read chunk tree -- 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: [PATCH] btrfs-progs: fix dev stats error output related to replace handle
On Wed, 2014-10-29 at 18:56 +0800, Anand Jain wrote:
Hi Gui,
We don't need this patch. Actually you should back out this patch to
get this correct.
[PATCH] btrfs-progs: do a separate probe for _transient_ replacing device
OR apply. this
[PATCH] revert btrfs-progs: do a separate probe for _transient_
replacing device
Try it out. Lets know.
Thanks
Oh, yes, I've tried your revert patch and I acknowledge that it fixes
the problem.
So please *ignore* my patch David, sorry for the noise.
-Gui
On 10/23/14 09:56, Gui Hecheng wrote:
Steps to reproduce:
# mkfs.btrfs -f /dev/sdb7
# mount /dev/sdb7 /mnt
# btrfs dev stats /dev/sdb7
output:
[/dev/sdb7].write_io_errs 0
[/dev/sdb7].read_io_errs0
[/dev/sdb7].flush_io_errs 0
[/dev/sdb7].corruption_errs 0
[/dev/sdb7].generation_errs 0
* ERROR: ioctl(BTRFS_IOC_GET_DEV_STATS) on failed: No such device
while the following cmd:
# btrfs dev stats /mnt
yields the right thing:
[/dev/sdb7].write_io_errs 0
[/dev/sdb7].read_io_errs0
[/dev/sdb7].flush_io_errs 0
[/dev/sdb7].corruption_errs 0
[/dev/sdb7].generation_errs 0
This is caused by commit:
commit d0588bfa479409b2a0f6243f894338a01a56221a
btrfs-progs: do a separate probe for transient replacing device
The above commit trys to handle the fi show problem with device under
replacing, but it changes the @get_fs_info() logic which annoys dev stats.
For @get_fs_info():
o If the passed in @path is a mount point, then the @get_device_info() to
probe the replacing device will be glad to accept the device index
var @i as its init value 0 and the following i++ correctly sets @i
to 1 as the start of all devices in btrfs.
o If @path is a block device, then the problem comes...
The device index @i is set to devid of the block device passed in,
and the @get_device_info() will be forced to accept the devid
unwillingly.
Then the following i++ do the evil of skip the block device desired and
an
empty piece is handled next which causes the ERROR above.
To fix this problem, let's just pass 0 to the @get_device_info() explicitly,
and set the index @i to 1 if a mount point is passed in.
Under my own test, this will not affect the original fix of the fi show
problem with device under replacing.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
utils.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/utils.c b/utils.c
index f10c178..0ba2b26 100644
--- a/utils.c
+++ b/utils.c
@@ -1881,12 +1881,15 @@ int get_fs_info(char *path, struct
btrfs_ioctl_fs_info_args *fi_args,
}
/* get the replace target device if it is there */
- ret = get_device_info(fd, i, di_args[ndevs]);
+ ret = get_device_info(fd, 0, di_args[ndevs]);
if (!ret) {
ndevs++;
fi_args-num_devices++;
}
- i++;
+
+ /* if a mount point is passed in, start from devid 1 */
+ if (fi_args-num_devices != 1)
+ i = 1;
for (; i = fi_args-max_id; ++i) {
BUG_ON(ndevs = fi_args-num_devices);
--
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] revert btrfs-progs: do a separate probe for _transient_ replacing device
On Wed, 2014-10-29 at 18:51 +0800, Anand Jain wrote:
There is a compatibility issue with older kernel with the progs commit id as
below.
05cd2907557ba627cfb86e60b214ea6228613a84
Which tree does this commit id belongs to?
I can't find it anywhere?
So as of now writing to revert the above commit id.
The brewing sysfs interface would help to fix the impending issue, which is
seed device would fail show in 'btrfs fi show' output of a sprout device.
Signed-off-by: Anand Jain anand.j...@oracle.com
---
utils.c | 19 +--
1 file changed, 1 insertion(+), 18 deletions(-)
diff --git a/utils.c b/utils.c
index a8691fe..1d1cc77 100644
--- a/utils.c
+++ b/utils.c
@@ -1869,29 +1869,12 @@ int get_fs_info(char *path, struct
btrfs_ioctl_fs_info_args *fi_args,
if (!fi_args-num_devices)
goto out;
- /*
- * with kernel patch
- * btrfs: ioctl BTRFS_IOC_FS_INFO and BTRFS_IOC_DEV_INFO miss-matched
with slots
- * the kernel now returns total_devices which does not include
- * replacing device if running.
- * As we need to get dev info of the replace device if it is running,
- * so just add one to fi_args-num_devices.
- */
-
- di_args = *di_ret = malloc((fi_args-num_devices + 1) *
sizeof(*di_args));
+ di_args = *di_ret = malloc((fi_args-num_devices) * sizeof(*di_args));
if (!di_args) {
ret = -errno;
goto out;
}
- /* get the replace target device if it is there */
- ret = get_device_info(fd, i, di_args[ndevs]);
- if (!ret) {
- ndevs++;
- fi_args-num_devices++;
- }
- i++;
-
for (; i = fi_args-max_id; ++i) {
BUG_ON(ndevs = fi_args-num_devices);
ret = get_device_info(fd, i, di_args[ndevs]);
--
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/2] btrfs-progs: make the search target device routine more clear for fi show
Extract the procedure of searching for a target device for fi show
from the @map_seed_devices() function to make it more clear.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
cmds-filesystem.c | 37 -
1 file changed, 28 insertions(+), 9 deletions(-)
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index bb5881e..6437e57 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -742,14 +742,10 @@ static int find_and_copy_seed(struct btrfs_fs_devices
*seed,
return 1;
}
-static int map_seed_devices(struct list_head *all_uuids,
- char *search, int *found)
+static int search_umounted_fs_uuids(struct list_head *all_uuids,
+ char *search)
{
- struct btrfs_fs_devices *cur_fs, *cur_seed;
- struct btrfs_fs_devices *fs_copy, *seed_copy;
- struct btrfs_fs_devices *opened_fs;
- struct btrfs_device *device;
- struct btrfs_fs_info *fs_info;
+ struct btrfs_fs_devices *cur_fs, *fs_copy;
struct list_head *fs_uuids;
int ret = 0;
@@ -764,7 +760,7 @@ static int map_seed_devices(struct list_head *all_uuids,
if (search) {
if (uuid_search(cur_fs, search) == 0)
continue;
- *found = 1;
+ ret = 1;
}
fs_copy = malloc(sizeof(*fs_copy));
@@ -782,6 +778,22 @@ static int map_seed_devices(struct list_head *all_uuids,
list_add(fs_copy-list, all_uuids);
}
+out:
+ return ret;
+}
+
+static int map_seed_devices(struct list_head *all_uuids)
+{
+ struct btrfs_fs_devices *cur_fs, *cur_seed;
+ struct btrfs_fs_devices *seed_copy;
+ struct btrfs_fs_devices *opened_fs;
+ struct btrfs_device *device;
+ struct btrfs_fs_info *fs_info;
+ struct list_head *fs_uuids;
+ int ret = 0;
+
+ fs_uuids = btrfs_scanned_uuids();
+
list_for_each_entry(cur_fs, all_uuids, list) {
device = list_first_entry(cur_fs-devices,
struct btrfs_device, dev_list);
@@ -943,11 +955,18 @@ devs_only:
return 1;
}
+ found = search_umounted_fs_uuids(all_uuids, search);
+ if (found 0) {
+ fprintf(stderr,
+ ERROR: %d while searching target device\n, ret);
+ return 1;
+ }
+
/*
* scan_for_btrfs() don't build seed/sprout mapping,
* do mapping build for each scanned fs here
*/
- ret = map_seed_devices(all_uuids, search, found);
+ ret = map_seed_devices(all_uuids);
if (ret) {
fprintf(stderr,
ERROR: %d while mapping seed devices\n, ret);
--
1.8.1.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
[PATCH 2/2] btrfs-progs: skip mounted fs when deal with umounted ones for fi show
Stalling problems may happen when exec balance fi show cmds concurrently.
With the following commit:
commit 915902c500
btrfs-progs: fix device missing of btrfs fi show with seed devices
The fi show cmd will bother the mounted fs when only umounted fs should
be handled after @btrfs_can_kernel() has finished showing all mounted ones.
We could skip the mounted fs after @btrfs_can_kernel() is done, then tasks
keeps going on mounted fs while fi show continues on umounted ones separately.
Reported-by: Petr Janecek jane...@ucw.cz
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
cmds-filesystem.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 6437e57..67fe52b 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -53,6 +53,15 @@ struct seen_fsid {
static struct seen_fsid *seen_fsid_hash[SEEN_FSID_HASH_SIZE] = {NULL,};
+static int is_seen_fsid(u8 *fsid)
+{
+ u8 hash = fsid[0];
+ int slot = hash % SEEN_FSID_HASH_SIZE;
+ struct seen_fsid *seen = seen_fsid_hash[slot];
+
+ return seen ? 1 : 0;
+}
+
static int add_seen_fsid(u8 *fsid)
{
u8 hash = fsid[0];
@@ -763,6 +772,10 @@ static int search_umounted_fs_uuids(struct list_head
*all_uuids,
ret = 1;
}
+ /* skip all fs already shown as mounted fs */
+ if (is_seen_fsid(cur_fs-fsid))
+ continue;
+
fs_copy = malloc(sizeof(*fs_copy));
if (!fs_copy) {
ret = -ENOMEM;
--
1.8.1.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: Btrfs-progs release 3.17
On Tue, 2014-10-28 at 16:42 +0800, Anand Jain wrote: On 28/10/2014 12:03, Gui Hecheng wrote: On Thu, 2014-10-23 at 21:36 +0800, Anand Jain wrote: there is no point in re-creating so many btrfs kernel's logic in user space. its just unnecessary, when kernel is already doing it. use some interface to get info from kernel after device is registered, (not necessarily mounted). so progs can be as sleek as possible. to me it started as just one more bug now we have fixed so many many. It all needs one good interface for kernel which provides anything anything from the kernel. Oh, the interface for kernel you described is really interesting. But how to store the seed/sprout relationships so that we can fetch them correctly for umounted btrfs? remember - btrfs-control ioctl READY does not work yet when seed is present. Some how we need to fix that in kernel right. ? for which we need the seed/sprout relation when devices are unmounted. you may get that working/accepted in the kernel first, a simple user space interface (such as nacked /proc/fs/btrfs/devlist interface or discussing sysfs interface (bit risky though)) is all that is needed to get this working. As far as I could understand, so we have to make the ioctl READY to do the seed/sprout detecting relation detecting first. And then, all the user space tool has to do is fetching stuff through a new user interface. I think this is really a neat way. So is it on your schedule to make the ioctl READY 'ready'? If not, I am always ready to serve. Since there may be more discussions and comments on the above work, for now, I just plan to make the current way in 3.17 not so 'slow' and 'noisy'. Thanks, Gui -Gui On 10/23/14 16:52, Gui Hecheng wrote: On Thu, 2014-10-23 at 16:13 +0800, Anand Jain wrote: Some of the disks on my system were missing and I was able to hit this issue. Check tree block failed, want=12582912, have=0 read block failed check_tree_block Couldn't read chunk root warning devid 2 not found already Check tree block failed, want=143360, have=0 read block failed check_tree_block Couldn't read chunk root warning, device 4 is missing warning, device 3 is missing warning, device 2 is missing warning, device 1 is missing Did a bisect and it leads to this following patch. commit 915902c5002485fb13d27c4b699a73fb66cc0f09 btrfs-progs: fix device missing of btrfs fi show with seed devices Also this patch stalls ~2sec in the cmd btrfs fi show, on my system with 48 disks. Also a simple test case hits some warnings... mkfs.btrfs -draid1 -mraid1 /dev/sdb /dev/sdc mount /dev/sdb /btrfs fillfs /btrfs 100 umount /btrfs wipefs -a /dev/sdb modprobe -r btrfs modprobe btrfs mount -o degraded /dev/sdb /btrfs btrfs fi show Label: none uuid: 9844cd05-1c8c-473e-a84b-bac95aab7bc9 Total devices 2 FS bytes used 1.59MiB devid2 size 967.87MiB used 104.75MiB path /dev/sdc *** Some devices missing warning, device 1 is missing warning, device 1 is missing warning devid 1 not found already Hi Anand and Petr, Oh, it seems that there are btrfs with missing devs that are bringing troubles to the @open_ctree_... function. This should be a missing case of the patch above which should only take effects when seeding devices are present. I will try my best to follow this case, suggestions are welcome, Thanks! -Gui On 10/23/14 14:57, Petr Janecek wrote: Hello, You have mentioned two issues when balance and fi show running concurrently my mail was a bit chaotic, but I get the stalls even on idle system. Today I got parent transid verify failed on 1559973888000 wanted 1819 found 1821 parent transid verify failed on 1559973888000 wanted 1819 found 1821 parent transid verify failed on 1559973888000 wanted 1819 found 1821 parent transid verify failed on 1559973888000 wanted 1819 found 1821 Ignoring transid failure leaf parent key incorrect 1559973888000 from 'btrfs fi sh' while I was just copying something, no balance running. [...] [PATCH 1/1] btrfs-progs: code optimize cmd_scan_dev() use btrfs_register_one_device() [PATCH 1/2] btrfs-progs: introduce btrfs_register_all_device() [PATCH 2/2] btrfs-progs: optimize btrfs_scan_lblkid() for multiple calls If you could, pls.. Now on 3.17 apply above 3 patches and see if you see any better performance for the stalling issue. no perceptible change: takes ~40 seconds both before and after applying. Old version 1 sec. can you do same steps on 3.16 and report what you observe So many rejects -- do you have older versions of these patches
Re: Btrfs-progs release 3.17
On Thu, 2014-10-23 at 15:23 +0200, Petr Janecek wrote: Hello Gui, Oh, it seems that there are btrfs with missing devs that are bringing troubles to the @open_ctree_... function. what do you mean by missing devs? I have no degraded fs. Ah, sorry, I'm too focused on the problem that Anand's script pointed out. Ignore this missing devs. The time btrfs fi sh spends scanning disks of a filesystem seems to be proportional to the amount of data stored on them: on a completely idle system, of ~20s total time it spends 10s scanning each of /mnt/b and /mnt/b0, and almost no time on /mnt/b3 (which is the biggest) Filesystem Size Used Avail Use% Mounted on /dev/sdm5.5T 2.4T 2.1T 54% /mnt/b /dev/sda5.5T 2.5T 3.1T 45% /mnt/b0 /dev/sde7.3T 90G 5.4T 2% /mnt/b3 For your original problems: o error messages: The concurrency problem exists as Anand said. As you said, running balance cp lead to such messages, so I think there are some unintentional redundency works over the mounted devices when dealing with umounted ones. I'll try to o stalling: This may be due to concurrency problem either. After the first problem handled, let's see what happens. Thanks, Gui Thanks, Petr -- 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: Btrfs-progs release 3.17
On Thu, 2014-10-23 at 21:36 +0800, Anand Jain wrote: there is no point in re-creating so many btrfs kernel's logic in user space. its just unnecessary, when kernel is already doing it. use some interface to get info from kernel after device is registered, (not necessarily mounted). so progs can be as sleek as possible. to me it started as just one more bug now we have fixed so many many. It all needs one good interface for kernel which provides anything anything from the kernel. Oh, the interface for kernel you described is really interesting. But how to store the seed/sprout relationships so that we can fetch them correctly for umounted btrfs? -Gui On 10/23/14 16:52, Gui Hecheng wrote: On Thu, 2014-10-23 at 16:13 +0800, Anand Jain wrote: Some of the disks on my system were missing and I was able to hit this issue. Check tree block failed, want=12582912, have=0 read block failed check_tree_block Couldn't read chunk root warning devid 2 not found already Check tree block failed, want=143360, have=0 read block failed check_tree_block Couldn't read chunk root warning, device 4 is missing warning, device 3 is missing warning, device 2 is missing warning, device 1 is missing Did a bisect and it leads to this following patch. commit 915902c5002485fb13d27c4b699a73fb66cc0f09 btrfs-progs: fix device missing of btrfs fi show with seed devices Also this patch stalls ~2sec in the cmd btrfs fi show, on my system with 48 disks. Also a simple test case hits some warnings... mkfs.btrfs -draid1 -mraid1 /dev/sdb /dev/sdc mount /dev/sdb /btrfs fillfs /btrfs 100 umount /btrfs wipefs -a /dev/sdb modprobe -r btrfs modprobe btrfs mount -o degraded /dev/sdb /btrfs btrfs fi show Label: none uuid: 9844cd05-1c8c-473e-a84b-bac95aab7bc9 Total devices 2 FS bytes used 1.59MiB devid2 size 967.87MiB used 104.75MiB path /dev/sdc *** Some devices missing warning, device 1 is missing warning, device 1 is missing warning devid 1 not found already Hi Anand and Petr, Oh, it seems that there are btrfs with missing devs that are bringing troubles to the @open_ctree_... function. This should be a missing case of the patch above which should only take effects when seeding devices are present. I will try my best to follow this case, suggestions are welcome, Thanks! -Gui On 10/23/14 14:57, Petr Janecek wrote: Hello, You have mentioned two issues when balance and fi show running concurrently my mail was a bit chaotic, but I get the stalls even on idle system. Today I got parent transid verify failed on 1559973888000 wanted 1819 found 1821 parent transid verify failed on 1559973888000 wanted 1819 found 1821 parent transid verify failed on 1559973888000 wanted 1819 found 1821 parent transid verify failed on 1559973888000 wanted 1819 found 1821 Ignoring transid failure leaf parent key incorrect 1559973888000 from 'btrfs fi sh' while I was just copying something, no balance running. [...] [PATCH 1/1] btrfs-progs: code optimize cmd_scan_dev() use btrfs_register_one_device() [PATCH 1/2] btrfs-progs: introduce btrfs_register_all_device() [PATCH 2/2] btrfs-progs: optimize btrfs_scan_lblkid() for multiple calls If you could, pls.. Now on 3.17 apply above 3 patches and see if you see any better performance for the stalling issue. no perceptible change: takes ~40 seconds both before and after applying. Old version 1 sec. can you do same steps on 3.16 and report what you observe So many rejects -- do you have older versions of these patches? Thanks, Petr -- 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 -- 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: Btrfs-progs release 3.17
On Thu, 2014-10-23 at 16:13 +0800, Anand Jain wrote: Some of the disks on my system were missing and I was able to hit this issue. Check tree block failed, want=12582912, have=0 read block failed check_tree_block Couldn't read chunk root warning devid 2 not found already Check tree block failed, want=143360, have=0 read block failed check_tree_block Couldn't read chunk root warning, device 4 is missing warning, device 3 is missing warning, device 2 is missing warning, device 1 is missing Did a bisect and it leads to this following patch. commit 915902c5002485fb13d27c4b699a73fb66cc0f09 btrfs-progs: fix device missing of btrfs fi show with seed devices Also this patch stalls ~2sec in the cmd btrfs fi show, on my system with 48 disks. Also a simple test case hits some warnings... mkfs.btrfs -draid1 -mraid1 /dev/sdb /dev/sdc mount /dev/sdb /btrfs fillfs /btrfs 100 umount /btrfs wipefs -a /dev/sdb modprobe -r btrfs modprobe btrfs mount -o degraded /dev/sdb /btrfs btrfs fi show Label: none uuid: 9844cd05-1c8c-473e-a84b-bac95aab7bc9 Total devices 2 FS bytes used 1.59MiB devid2 size 967.87MiB used 104.75MiB path /dev/sdc *** Some devices missing warning, device 1 is missing warning, device 1 is missing warning devid 1 not found already Hi Anand and Petr, Oh, it seems that there are btrfs with missing devs that are bringing troubles to the @open_ctree_... function. This should be a missing case of the patch above which should only take effects when seeding devices are present. I will try my best to follow this case, suggestions are welcome, Thanks! -Gui On 10/23/14 14:57, Petr Janecek wrote: Hello, You have mentioned two issues when balance and fi show running concurrently my mail was a bit chaotic, but I get the stalls even on idle system. Today I got parent transid verify failed on 1559973888000 wanted 1819 found 1821 parent transid verify failed on 1559973888000 wanted 1819 found 1821 parent transid verify failed on 1559973888000 wanted 1819 found 1821 parent transid verify failed on 1559973888000 wanted 1819 found 1821 Ignoring transid failure leaf parent key incorrect 1559973888000 from 'btrfs fi sh' while I was just copying something, no balance running. [...] [PATCH 1/1] btrfs-progs: code optimize cmd_scan_dev() use btrfs_register_one_device() [PATCH 1/2] btrfs-progs: introduce btrfs_register_all_device() [PATCH 2/2] btrfs-progs: optimize btrfs_scan_lblkid() for multiple calls If you could, pls.. Now on 3.17 apply above 3 patches and see if you see any better performance for the stalling issue. no perceptible change: takes ~40 seconds both before and after applying. Old version 1 sec. can you do same steps on 3.16 and report what you observe So many rejects -- do you have older versions of these patches? Thanks, Petr -- 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
[PATCH] btrfs-progs: fix dev stats error output related to replace handle
Steps to reproduce:
# mkfs.btrfs -f /dev/sdb7
# mount /dev/sdb7 /mnt
# btrfs dev stats /dev/sdb7
output:
[/dev/sdb7].write_io_errs 0
[/dev/sdb7].read_io_errs0
[/dev/sdb7].flush_io_errs 0
[/dev/sdb7].corruption_errs 0
[/dev/sdb7].generation_errs 0
* ERROR: ioctl(BTRFS_IOC_GET_DEV_STATS) on failed: No such device
while the following cmd:
# btrfs dev stats /mnt
yields the right thing:
[/dev/sdb7].write_io_errs 0
[/dev/sdb7].read_io_errs0
[/dev/sdb7].flush_io_errs 0
[/dev/sdb7].corruption_errs 0
[/dev/sdb7].generation_errs 0
This is caused by commit:
commit d0588bfa479409b2a0f6243f894338a01a56221a
btrfs-progs: do a separate probe for transient replacing device
The above commit trys to handle the fi show problem with device under
replacing, but it changes the @get_fs_info() logic which annoys dev stats.
For @get_fs_info():
o If the passed in @path is a mount point, then the @get_device_info() to
probe the replacing device will be glad to accept the device index
var @i as its init value 0 and the following i++ correctly sets @i
to 1 as the start of all devices in btrfs.
o If @path is a block device, then the problem comes...
The device index @i is set to devid of the block device passed in,
and the @get_device_info() will be forced to accept the devid unwillingly.
Then the following i++ do the evil of skip the block device desired and an
empty piece is handled next which causes the ERROR above.
To fix this problem, let's just pass 0 to the @get_device_info() explicitly,
and set the index @i to 1 if a mount point is passed in.
Under my own test, this will not affect the original fix of the fi show
problem with device under replacing.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
utils.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/utils.c b/utils.c
index f10c178..0ba2b26 100644
--- a/utils.c
+++ b/utils.c
@@ -1881,12 +1881,15 @@ int get_fs_info(char *path, struct
btrfs_ioctl_fs_info_args *fi_args,
}
/* get the replace target device if it is there */
- ret = get_device_info(fd, i, di_args[ndevs]);
+ ret = get_device_info(fd, 0, di_args[ndevs]);
if (!ret) {
ndevs++;
fi_args-num_devices++;
}
- i++;
+
+ /* if a mount point is passed in, start from devid 1 */
+ if (fi_args-num_devices != 1)
+ i = 1;
for (; i = fi_args-max_id; ++i) {
BUG_ON(ndevs = fi_args-num_devices);
--
1.8.1.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] btrfs: fix wrong pad for kernelspace arg of btrfs dev stat
On Thu, 2014-10-16 at 11:26 +0200, Stefan Behrens wrote:
On Thu, 16 Oct 2014 09:53:37 +0800, Gui Hecheng wrote:
The @btrfs_ioctl_get_dev_stats fails to pad to 1k as descripted,
actually it valuates to 1032 bytes.
The corresponding userspace change follows this change.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
include/uapi/linux/btrfs.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index 2f47824..fc4e326 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -416,7 +416,7 @@ struct btrfs_ioctl_get_dev_stats {
/* out values: */
__u64 values[BTRFS_DEV_STAT_VALUES_MAX];
- __u64 unused[128 - 2 - BTRFS_DEV_STAT_VALUES_MAX]; /* pad to 1k */
+ __u64 unused[128 - 3 - BTRFS_DEV_STAT_VALUES_MAX]; /* pad to 1k */
You can't change an existing ioctl interface like this and make it
incompatible, the length of the structure is used in _IOWR(). Just
change the comment from pad to 1k to pad to 1032 bytes instead.
Er... yeah, the problem exists, and such a fix seems cost much.
Since the related tool itself doesn't influence much, just scratch these
two may be a reasonable idea. ^_^
(pad to 1032 bytes seems a bit...)
};
#define BTRFS_QUOTA_CTL_ENABLE 1
--
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 v2] btrfs: ioctl BTRFS_IOC_FS_INFO and BTRFS_IOC_DEV_INFO miss-matched with slots
On Thu, 2014-09-04 at 20:02 +0800, Anand Jain wrote: On 09/04/2014 05:58 PM, David Sterba wrote: On Mon, Aug 18, 2014 at 04:38:18PM +0800, Anand Jain wrote: ioctl BTRFS_IOC_FS_INFO return num_devices which does _not_ include seed device, But the following ioctl BTRFS_IOC_DEV_INFO counts and gets seed disk when probed. So in the userland we hit a count-slot missmatch bug.. get_fs_info() :: BUG_ON(ndevs = fi_args-num_devices); which hits this bug when we have mounted a seed device. So to fix this problem here in this patch ioctl BTRFS_IOC_FS_INFO will provide total_devices instead of num_devices. The ioctl is very unclear what the 'num_device' actually means. Right. Thats also true in kernel. very messy. very confusing. tool btrfs-devlist would help understand whats going on. $ egrep num_device *.c | egrep total_device ioctl.c: fi_args-num_devices = fs_devices-total_devices; super.c: ret = !(fs_devices-num_devices == fs_devices-total_devices); volumes.c:total_devices = btrfs_super_num_devices(disk_super); By the way about BTRFS_IOC_DEVICES_READY ioctl above its long time broken with seed/replace, just waiting to get these patches integrated first so to fix it later. This would fix the problem partly. Partly because ealier num_devices included the replacing device but now total_device does not include the replacing device. Getting a count which includes a transient device is rather too in efficient/wrong indeed, because there can be a race condition where in the time between ioctl BTRFS_IOC_FS_INFO to BTRFS_IOC_DEV_INFO the replace device operation might have been completed. So to fix this problem its better that user land btrfs-progs probes replacing device (at devid 0) separately. v2: Agree with Wang's comment. Its better to show seed disks under the sprout fs, so that user can establish mapping of seed to sprout devices. So here I am making BTRFS_IOC_FS_INFO to return the total_devices which would count the seed devices (but not the replacing device). This is even more confusing. I think we need to add another member to the ioctl struct to reflect the number of regular devices (num_devices) and the true total number of devices including seeding and replaced devices. that will be a better way. thanks. The difference should be accompanied by a flag that would say if there's a seeding or replace in progress. There are some backward compatibility concerns. Setting num_devices to total_devices changes semantics of the ioctl, so I think it should stay as is for now, As I have tested there is not backward compatibility issue. But from semantics perspective .. agreed. but the BUG_ON can be removed and replaced by code that reallocates the buffer or allocates a few more items in advance. We don't know how may seed devices are there for a sprout FS. So thats not possible. Will review resubmit. Thanks for commenting. Hi all, Firtly, thanks for the fix, Anand, how's the new version going? I've been testing the btrfs fi show cmd these days and find that this patch has not been merged into linus's tree yet. Since the suggested way of adding member to the ioctl struct brings compatibility issues, it may need more discussion. But since this fix really incluence much to the user, I consider merging this version first to be a good idea. What do you think, Chris? Thanks, Gui Anand -- 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 -- 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: fix wrong pad for kernelspace arg of btrfs dev stat
The @btrfs_ioctl_get_dev_stats fails to pad to 1k as descripted,
actually it valuates to 1032 bytes.
The corresponding userspace change follows this change.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
include/uapi/linux/btrfs.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index 2f47824..fc4e326 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -416,7 +416,7 @@ struct btrfs_ioctl_get_dev_stats {
/* out values: */
__u64 values[BTRFS_DEV_STAT_VALUES_MAX];
- __u64 unused[128 - 2 - BTRFS_DEV_STAT_VALUES_MAX]; /* pad to 1k */
+ __u64 unused[128 - 3 - BTRFS_DEV_STAT_VALUES_MAX]; /* pad to 1k */
};
#define BTRFS_QUOTA_CTL_ENABLE 1
--
1.8.1.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
[PATCH] btrfs-progs: fix wrong pad for userspace arg of btrfs dev stat
The @btrfs_ioctl_get_dev_stats fails to pad to 1k as descripted,
actually it valuates to 1032 bytes.
Correct it following the kernel patch.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
ioctl.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ioctl.h b/ioctl.h
index f0fc060..6657f8f 100644
--- a/ioctl.h
+++ b/ioctl.h
@@ -429,7 +429,7 @@ struct btrfs_ioctl_get_dev_stats {
/* out values: */
__u64 values[BTRFS_DEV_STAT_VALUES_MAX];
- __u64 unused[128 - 2 - BTRFS_DEV_STAT_VALUES_MAX]; /* pad to 1k */
+ __u64 unused[128 - 3 - BTRFS_DEV_STAT_VALUES_MAX]; /* pad to 1k */
};
/* BTRFS_IOC_SNAP_CREATE is no longer used by the btrfs command */
--
1.8.1.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
[PATCH] btrfs-progs: prevent silent damage when add dev to an invalid mntpnt
Problem:
# mkfs.btrfs -f /dev/sda1
# btrfs dev add /dev/sda1 /dir -f == dir is not a mntpnt
btrfs dev add just report invalid ioctl but it has already made
changes to /dev/sda1 with @btrfs_prepare_device(), so the fs on
/dev/sda1 is damaged.
We could check whether /dev/sda1 is a valid mntpnt by calling
@find_mount_root() to prevent this silent damage.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
cmds-device.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/cmds-device.c b/cmds-device.c
index a728f21..65815c3 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -53,6 +53,7 @@ static int cmd_add_dev(int argc, char **argv)
int discard = 1;
int force = 0;
char estr[100];
+ char rmntpnt[PATH_MAX];
while (1) {
int long_index;
@@ -84,6 +85,22 @@ static int cmd_add_dev(int argc, char **argv)
mntpnt = argv[optind + argc - 1];
+ if (!realpath(mntpnt, rmntpnt)) {
+ fprintf(stderr, ERROR: %s\n, strerror(errno));
+ return 1;
+ }
+
+ ret = find_mount_root(rmntpnt, mntpnt);
+ if (ret 0) {
+ fprintf(stderr, ERROR: find_mount_root failed on '%s': %s\n,
+ rmntpnt, strerror(-ret));
+ return 1;
+ } else if (ret 0) {
+ fprintf(stderr, ERROR: '%s' doesn't belong to btrfs mount
point\n,
+ rmntpnt);
+ return 1;
+ }
+
fdmnt = open_file_or_dir(mntpnt, dirstream);
if (fdmnt 0) {
fprintf(stderr, ERROR: can't access '%s'\n, mntpnt);
--
1.8.1.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
Check ping
Check ping -- 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-progs: fix BUG_ON when all devices under seed fs are missing
Steps to reproduce:
# mkfs.btrfs -f /dev/sda[1-2]
# btrfstune -S 1 /dev/sda1
# mount /dev/sda /mnt
# btrfs dev add /dev/sda3 /mnt
# umount /mnt
# mkfs.ext4 /dev/sda1 // kill seed dev
# mkfs.ext4 /dev/sda2 // kill seed dev
# btrfs-debug-tree /dev/sda3== BUG_ON
Output msg:
volumes.c:1824: btrfs_read_chunk_tree: Assertion `ret` failed.
btrfs-debug-tree[0x41cb36]
btrfs-debug-tree(btrfs_read_chunk_tree+0x3ca)
btrfs-debug-tree(btrfs_setup_chunk_tree_and_device_map
btrfs-debug-tree[0x40f695]
btrfs-debug-tree(open_ctree_fs_info+0x86)
btrfs-debug-tree(main+0x12d)
/lib64/libc.so.6(__libc_start_main+0xf5)
btrfs-debug-tree[0x4062e9]
This BUG_ON complains about a failed @read_one_dev() call when
@open_seed_devices() failed to find the seed @fs_devices object
for a dev_item in chunk tree.
In this case, just insert a shadow @fs_devices with the fsid in
dev_item shall make no harm since no other tools will try to
make use of the stuff that the shadow @fs_devices possesses
after its creation.
After apply this commit, btrfs-debug-tree will report unable
to open the device.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
*Note*
This should be after patch:
Btrfs-progs: fsck: disallow partial opening if critical \
roots corrupted
---
volumes.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/volumes.c b/volumes.c
index eaf3f9f..a9c7662 100644
--- a/volumes.c
+++ b/volumes.c
@@ -1671,8 +1671,15 @@ static int open_seed_devices(struct btrfs_root *root, u8
*fsid)
fs_devices = find_fsid(fsid);
if (!fs_devices) {
- ret = -ENOENT;
- goto out;
+ /* missing all seed devices */
+ fs_devices = kzalloc(sizeof(*fs_devices), GFP_NOFS);
+ if (!fs_devices) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ INIT_LIST_HEAD(fs_devices-devices);
+ list_add(fs_devices-list, fs_uuids);
+ memcpy(fs_devices-fsid, fsid, BTRFS_FSID_SIZE);
}
ret = btrfs_open_devices(fs_devices, O_RDONLY);
--
1.8.1.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 v4] btrfs-progs: fix page align issue for lzo compress in restore
On Tue, 2014-09-23 at 10:25 +0800, Gui Hecheng wrote:
When runing restore under lzo compression, bad compress length
problems are encountered.
It is because there is a page align problem with the @decompress_lzo,
as follows:
|--| ||-| |--|...|--|
page ^page page
|
3 bytes left
When lzo compress pages im RAM, lzo will ensure that
the 4 bytes len will be in one page as a whole.
There is a situation that 3 (or less) bytes are left
at the end of a page, and then the 4 bytes len is
stored at the start of the next page.
But the @decompress_lzo doesn't goto the start of
the next page and continue to read the next 4 bytes
which is across two pages, so a random value is fetched
as a bad compress length.
So we check page alignment every time before we are going to
fetch the next @len and after the former piece of data is decompressed.
If the current page that we reach has less than 4 bytes left,
then we should fetch the next @len at the start of next page.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
Reviewed-by: Marc Dietrich marvi...@gmx.de
---
changelog
v1-v2: adopt alignment check method suggested by Marc
v2-v3: make code more readable
v3-v4: keep type safety
---
cmds-restore.c | 29 +++--
1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/cmds-restore.c b/cmds-restore.c
index 38a131e..fa5d5d1 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -56,7 +56,10 @@ static int get_xattrs = 0;
static int dry_run = 0;
#define LZO_LEN 4
-#define PAGE_CACHE_SIZE 4096
+#define PAGE_CACHE_SIZE 4096UL
+#define PAGE_CACHE_MASK (~(PAGE_CACHE_SIZE - 1))
+#define PAGE_CACHE_ALIGN(addr) (((addr) + PAGE_CACHE_SIZE - 1) \
+ PAGE_CACHE_MASK)
#define lzo1x_worst_compress(x) ((x) + ((x) / 16) + 64 + 3)
static int decompress_zlib(char *inbuf, char *outbuf, u64 compress_len,
@@ -93,6 +96,28 @@ static inline size_t read_compress_length(unsigned char
*buf)
return le32_to_cpu(dlen);
}
+static void align_if_need(size_t *tot_in, size_t *in_len)
+{
+ size_t tot_in_aligned;
+ size_t bytes_left;
+
+ tot_in_aligned = PAGE_CACHE_ALIGN(*tot_in);
+ bytes_left = tot_in_aligned - *tot_in;
+
+ if (bytes_left = LZO_LEN)
+ return;
+
+ /*
+ * The LZO_LEN bytes is guaranteed to be
+ * in one page as a whole, so if a page
+ * has fewer than LZO_LEN bytes left,
+ * the LZO_LEN bytes should be fetched
+ * at the start of the next page
+ */
+ *in_len += bytes_left;
+ *tot_in = tot_in_aligned;
+}
+
static int decompress_lzo(unsigned char *inbuf, char *outbuf, u64
compress_len,
u64 *decompress_len)
{
@@ -135,8 +160,8 @@ static int decompress_lzo(unsigned char *inbuf, char
*outbuf, u64 compress_len,
}
out_len += new_len;
outbuf += new_len;
+ align_if_need(tot_in, in_len);
inbuf += in_len;
- tot_in += in_len;
}
*decompress_len = out_len;
Sorry, please scratch this one, the comments should be reformated.
--
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 v4] btrfs-progs: fix page align issue for lzo compress in restore
When runing restore under lzo compression, bad compress length
problems are encountered.
It is because there is a page align problem with the @decompress_lzo,
as follows:
|--| ||-| |--|...|--|
page ^page page
|
3 bytes left
When lzo compress pages im RAM, lzo will ensure that
the 4 bytes len will be in one page as a whole.
There is a situation that 3 (or less) bytes are left
at the end of a page, and then the 4 bytes len is
stored at the start of the next page.
But the @decompress_lzo doesn't goto the start of
the next page and continue to read the next 4 bytes
which is across two pages, so a random value is fetched
as a bad compress length.
So we check page alignment every time before we are going to
fetch the next @len and after the former piece of data is decompressed.
If the current page that we reach has less than 4 bytes left,
then we should fetch the next @len at the start of next page.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
Reviewed-by: Marc Dietrich marvi...@gmx.de
---
changelog
v1-v2: adopt alignment check method suggested by Marc
v2-v3: make code more readable
v3-v4: keep type safety reformat comments
---
cmds-restore.c | 27 +--
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/cmds-restore.c b/cmds-restore.c
index e09acc4..1fe2df0 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -56,7 +56,10 @@ static int get_xattrs = 0;
static int dry_run = 0;
#define LZO_LEN 4
-#define PAGE_CACHE_SIZE 4096
+#define PAGE_CACHE_SIZE 4096UL
+#define PAGE_CACHE_MASK (~(PAGE_CACHE_SIZE - 1))
+#define PAGE_CACHE_ALIGN(addr) (((addr) + PAGE_CACHE_SIZE - 1) \
+PAGE_CACHE_MASK)
#define lzo1x_worst_compress(x) ((x) + ((x) / 16) + 64 + 3)
static int decompress_zlib(char *inbuf, char *outbuf, u64 compress_len,
@@ -93,6 +96,26 @@ static inline size_t read_compress_length(unsigned char *buf)
return le32_to_cpu(dlen);
}
+static void align_if_need(size_t *tot_in, size_t *in_len)
+{
+ size_t tot_in_aligned;
+ size_t bytes_left;
+
+ tot_in_aligned = PAGE_CACHE_ALIGN(*tot_in);
+ bytes_left = tot_in_aligned - *tot_in;
+
+ if (bytes_left = LZO_LEN)
+ return;
+
+ /*
+* The LZO_LEN bytes is guaranteed to be in one page as a whole,
+* so if a page has fewer than LZO_LEN bytes left, the LZO_LEN bytes
+* should be fetched at the start of the next page
+*/
+ *in_len += bytes_left;
+ *tot_in = tot_in_aligned;
+}
+
static int decompress_lzo(unsigned char *inbuf, char *outbuf, u64 compress_len,
u64 *decompress_len)
{
@@ -135,8 +158,8 @@ static int decompress_lzo(unsigned char *inbuf, char
*outbuf, u64 compress_len,
}
out_len += new_len;
outbuf += new_len;
+ align_if_need(tot_in, in_len);
inbuf += in_len;
- tot_in += in_len;
}
*decompress_len = out_len;
--
1.8.1.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
[PATCH v2] btrfs-progs: fix page align issue for lzo compress in restore
When runing restore under lzo compression, bad compress length
problems are encountered.
It is because there is a page align problem with the @decompress_lzo,
as follows:
|--| ||-| |--|...|--|
page ^page page
|
3 bytes left
When lzo compress pages im RAM, lzo will ensure that
the 4 bytes len will be in one page as a whole.
There is a situation that 3 (or less) bytes are left
at the end of a page, and then the 4 bytes len is
stored at the start of the next page.
But the @decompress_lzo doesn't goto the start of
the next page and continue to read the next 4 bytes
which is across two pages, so a random value is fetched
as a bad compress length.
So we check page alignment every time before we are going to
fetch the next @len and after the former piece of data is decompressed.
If the current page that we reach has less than 4 bytes left,
then we should fetch the next @len at the start of next page.
Signed-off-by: Marc Dietrich marvi...@gmx.de
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
changelog
v1-v2: adopt alignment check method suggested by Marc
---
cmds-restore.c | 27 ++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/cmds-restore.c b/cmds-restore.c
index 38a131e..974f45d 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -57,6 +57,9 @@ static int dry_run = 0;
#define LZO_LEN 4
#define PAGE_CACHE_SIZE 4096
+#define PAGE_CACHE_MASK (~(PAGE_CACHE_SIZE - 1))
+#define PAGE_CACHE_ALIGN(addr) (((addr) + PAGE_CACHE_SIZE - 1) \
+PAGE_CACHE_MASK)
#define lzo1x_worst_compress(x) ((x) + ((x) / 16) + 64 + 3)
static int decompress_zlib(char *inbuf, char *outbuf, u64 compress_len,
@@ -93,6 +96,28 @@ static inline size_t read_compress_length(unsigned char *buf)
return le32_to_cpu(dlen);
}
+static void align_if_need(size_t *tot_in, size_t *in_len)
+{
+ int tot_in_aligned;
+ int bytes_left;
+
+ tot_in_aligned = PAGE_CACHE_ALIGN(*tot_in);
+ bytes_left = tot_in_aligned - *tot_in;
+
+ if (bytes_left = LZO_LEN)
+ return;
+
+ /*
+* The LZO_LEN bytes is guaranteed to be
+* in one page as a whole, so if a page
+* has fewer than LZO_LEN bytes left,
+* the LZO_LEN bytes should be fetched
+* at the start of the next page
+*/
+ *in_len += tot_in_aligned - *tot_in;
+ *tot_in = tot_in_aligned;
+}
+
static int decompress_lzo(unsigned char *inbuf, char *outbuf, u64 compress_len,
u64 *decompress_len)
{
@@ -135,8 +160,8 @@ static int decompress_lzo(unsigned char *inbuf, char
*outbuf, u64 compress_len,
}
out_len += new_len;
outbuf += new_len;
+ align_if_need(tot_in, in_len);
inbuf += in_len;
- tot_in += in_len;
}
*decompress_len = out_len;
--
1.8.1.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: fs corruption report
On Mon, 2014-09-22 at 10:19 +0200, Marc Dietrich wrote: Hi, Am Freitag, 19. September 2014, 09:30:30 schrieb Gui Hecheng: On Thu, 2014-09-18 at 12:47 +, Zooko Wilcox-OHearn wrote: Thank you! I will try to restore using this patch. What branch of what btrfs tools git repo should I apply the patch to? Regards, Zooko At least I think the following repo/v3.17.x branch has all restore-related patches included. git://github.com/kdave/btrfs-progs.git This is a mirror of the latest devel repo that I am on. restore of my corrupted partition finished. No bad compress nor valgrind errors anymore. I'll close the bug. Many thanks! Marc Hi Marc, I'm very glad to hear the result. I've sent a v2 patch which adopts your idea. I also add your sign-off-by, please help me to check whether it matches your thoughts, or more could be done to improve. Thanks, -Gui -- 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 v2] btrfs-progs: fix page align issue for lzo compress in restore
On Mon, 2014-09-22 at 10:44 +0200, Marc Dietrich wrote:
Am Montag, 22. September 2014, 16:29:28 schrieb Gui Hecheng:
When runing restore under lzo compression, bad compress length
problems are encountered.
It is because there is a page align problem with the @decompress_lzo,
as follows:
|--| ||-| |--|...|--|
page ^page page
|
3 bytes left
When lzo compress pages im RAM, lzo will ensure that
the 4 bytes len will be in one page as a whole.
There is a situation that 3 (or less) bytes are left
at the end of a page, and then the 4 bytes len is
stored at the start of the next page.
But the @decompress_lzo doesn't goto the start of
the next page and continue to read the next 4 bytes
which is across two pages, so a random value is fetched
as a bad compress length.
So we check page alignment every time before we are going to
fetch the next @len and after the former piece of data is decompressed.
If the current page that we reach has less than 4 bytes left,
then we should fetch the next @len at the start of next page.
Signed-off-by: Marc Dietrich marvi...@gmx.de
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
changelog
v1-v2: adopt alignment check method suggested by Marc
---
cmds-restore.c | 27 ++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/cmds-restore.c b/cmds-restore.c
index 38a131e..974f45d 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -57,6 +57,9 @@ static int dry_run = 0;
#define LZO_LEN 4
#define PAGE_CACHE_SIZE 4096
+#define PAGE_CACHE_MASK (~(PAGE_CACHE_SIZE - 1))
+#define PAGE_CACHE_ALIGN(addr) (((addr) + PAGE_CACHE_SIZE - 1) \
+PAGE_CACHE_MASK)
#define lzo1x_worst_compress(x) ((x) + ((x) / 16) + 64 + 3)
static int decompress_zlib(char *inbuf, char *outbuf, u64 compress_len,
@@ -93,6 +96,28 @@ static inline size_t read_compress_length(unsigned char
*buf)
return le32_to_cpu(dlen);
}
+static void align_if_need(size_t *tot_in, size_t *in_len)
+{
+ int tot_in_aligned;
+ int bytes_left;
+
+ tot_in_aligned = PAGE_CACHE_ALIGN(*tot_in);
+ bytes_left = tot_in_aligned - *tot_in;
+
+ if (bytes_left = LZO_LEN)
+ return;
+
+ /*
+* The LZO_LEN bytes is guaranteed to be
+* in one page as a whole, so if a page
+* has fewer than LZO_LEN bytes left,
+* the LZO_LEN bytes should be fetched
+* at the start of the next page
+*/
+ *in_len += tot_in_aligned - *tot_in;
in_len += bytes_left; // makes it more readable
Oh, yes, that's my carelessness, Thanks!
+ *tot_in = tot_in_aligned;
+}
+
static int decompress_lzo(unsigned char *inbuf, char *outbuf, u64
compress_len,
u64 *decompress_len)
{
@@ -135,8 +160,8 @@ static int decompress_lzo(unsigned char *inbuf, char
*outbuf, u64 compress_len,
}
out_len += new_len;
outbuf += new_len;
+ align_if_need(tot_in, in_len);
inbuf += in_len;
- tot_in += in_len;
}
*decompress_len = out_len;
otherwise, looks good to me.
Thanks!
Marc
--
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 corruption report
On Mon, 2014-09-22 at 10:49 +0200, Marc Dietrich wrote: Am Montag, 22. September 2014, 16:33:56 schrieb Gui Hecheng: On Mon, 2014-09-22 at 10:19 +0200, Marc Dietrich wrote: Hi, Am Freitag, 19. September 2014, 09:30:30 schrieb Gui Hecheng: On Thu, 2014-09-18 at 12:47 +, Zooko Wilcox-OHearn wrote: Thank you! I will try to restore using this patch. What branch of what btrfs tools git repo should I apply the patch to? Regards, Zooko At least I think the following repo/v3.17.x branch has all restore-related patches included. git://github.com/kdave/btrfs-progs.git This is a mirror of the latest devel repo that I am on. restore of my corrupted partition finished. No bad compress nor valgrind errors anymore. I'll close the bug. Many thanks! Marc Hi Marc, I'm very glad to hear the result. I've sent a v2 patch which adopts your idea. I also add your sign-off-by, please help me to check whether it matches your thoughts, or more could be done to improve. well, I didn't wrote any code. So it's enough to just add my reviewed-by. Thanks! Marc OK, I will add a reviewed-by instead. -- 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 v3] btrfs-progs: fix page align issue for lzo compress in restore
When runing restore under lzo compression, bad compress length
problems are encountered.
It is because there is a page align problem with the @decompress_lzo,
as follows:
|--| ||-| |--|...|--|
page ^page page
|
3 bytes left
When lzo compress pages im RAM, lzo will ensure that
the 4 bytes len will be in one page as a whole.
There is a situation that 3 (or less) bytes are left
at the end of a page, and then the 4 bytes len is
stored at the start of the next page.
But the @decompress_lzo doesn't goto the start of
the next page and continue to read the next 4 bytes
which is across two pages, so a random value is fetched
as a bad compress length.
So we check page alignment every time before we are going to
fetch the next @len and after the former piece of data is decompressed.
If the current page that we reach has less than 4 bytes left,
then we should fetch the next @len at the start of next page.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
Reviewed-by: Marc Dietrich marvi...@gmx.de
---
changelog
v1-v2: adopt alignment check method suggested by Marc
v2-v3: make code more readable
---
cmds-restore.c | 27 ++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/cmds-restore.c b/cmds-restore.c
index 38a131e..5094b05 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -57,6 +57,9 @@ static int dry_run = 0;
#define LZO_LEN 4
#define PAGE_CACHE_SIZE 4096
+#define PAGE_CACHE_MASK (~(PAGE_CACHE_SIZE - 1))
+#define PAGE_CACHE_ALIGN(addr) (((addr) + PAGE_CACHE_SIZE - 1) \
+PAGE_CACHE_MASK)
#define lzo1x_worst_compress(x) ((x) + ((x) / 16) + 64 + 3)
static int decompress_zlib(char *inbuf, char *outbuf, u64 compress_len,
@@ -93,6 +96,28 @@ static inline size_t read_compress_length(unsigned char *buf)
return le32_to_cpu(dlen);
}
+static void align_if_need(size_t *tot_in, size_t *in_len)
+{
+ int tot_in_aligned;
+ int bytes_left;
+
+ tot_in_aligned = PAGE_CACHE_ALIGN(*tot_in);
+ bytes_left = tot_in_aligned - *tot_in;
+
+ if (bytes_left = LZO_LEN)
+ return;
+
+ /*
+* The LZO_LEN bytes is guaranteed to be
+* in one page as a whole, so if a page
+* has fewer than LZO_LEN bytes left,
+* the LZO_LEN bytes should be fetched
+* at the start of the next page
+*/
+ *in_len += bytes_left;
+ *tot_in = tot_in_aligned;
+}
+
static int decompress_lzo(unsigned char *inbuf, char *outbuf, u64 compress_len,
u64 *decompress_len)
{
@@ -135,8 +160,8 @@ static int decompress_lzo(unsigned char *inbuf, char
*outbuf, u64 compress_len,
}
out_len += new_len;
outbuf += new_len;
+ align_if_need(tot_in, in_len);
inbuf += in_len;
- tot_in += in_len;
}
*decompress_len = out_len;
--
1.8.1.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 v3] btrfs-progs: fix page align issue for lzo compress in restore
On Mon, 2014-09-22 at 15:41 +0200, David Sterba wrote:
On Mon, Sep 22, 2014 at 04:58:26PM +0800, Gui Hecheng wrote:
So we check page alignment every time before we are going to
fetch the next @len and after the former piece of data is decompressed.
If the current page that we reach has less than 4 bytes left,
then we should fetch the next @len at the start of next page.
Thanks for the fix.
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -57,6 +57,9 @@ static int dry_run = 0;
#define LZO_LEN 4
#define PAGE_CACHE_SIZE 4096
+#define PAGE_CACHE_MASK (~(PAGE_CACHE_SIZE - 1))
+#define PAGE_CACHE_ALIGN(addr) (((addr) + PAGE_CACHE_SIZE - 1) \
+PAGE_CACHE_MASK)
This is not type-safe, the PAGE_CACHE_SIZE should be unsigned long.
#define lzo1x_worst_compress(x) ((x) + ((x) / 16) + 64 + 3)
static int decompress_zlib(char *inbuf, char *outbuf, u64 compress_len,
@@ -93,6 +96,28 @@ static inline size_t read_compress_length(unsigned char
*buf)
return le32_to_cpu(dlen);
}
+static void align_if_need(size_t *tot_in, size_t *in_len)
+{
+ int tot_in_aligned;
+ int bytes_left;
+
+ tot_in_aligned = PAGE_CACHE_ALIGN(*tot_in);
size_t - int, plus other tricks that happen inside the macro
+ bytes_left = tot_in_aligned - *tot_in;
int = int - size_t
+
+ if (bytes_left = LZO_LEN)
+ return;
+
+ /*
+* The LZO_LEN bytes is guaranteed to be
+* in one page as a whole, so if a page
+* has fewer than LZO_LEN bytes left,
+* the LZO_LEN bytes should be fetched
+* at the start of the next page
+*/
Nitpick, the comment can use the whole width of the line
/*
* The LZO_LEN bytes is guaranteed to be in one page as a whole,
* so if a page has fewer than LZO_LEN bytes left, the LZO_LEN
* bytes should be fetched at the start of the next page
*/
+ *in_len += bytes_left;
+ *tot_in = tot_in_aligned;
+}
Thanks David, I will pay more attention to the type-safe issue and
resend.
-Gui
--
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 v4] btrfs-progs: fix page align issue for lzo compress in restore
When runing restore under lzo compression, bad compress length
problems are encountered.
It is because there is a page align problem with the @decompress_lzo,
as follows:
|--| ||-| |--|...|--|
page ^page page
|
3 bytes left
When lzo compress pages im RAM, lzo will ensure that
the 4 bytes len will be in one page as a whole.
There is a situation that 3 (or less) bytes are left
at the end of a page, and then the 4 bytes len is
stored at the start of the next page.
But the @decompress_lzo doesn't goto the start of
the next page and continue to read the next 4 bytes
which is across two pages, so a random value is fetched
as a bad compress length.
So we check page alignment every time before we are going to
fetch the next @len and after the former piece of data is decompressed.
If the current page that we reach has less than 4 bytes left,
then we should fetch the next @len at the start of next page.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
Reviewed-by: Marc Dietrich marvi...@gmx.de
---
changelog
v1-v2: adopt alignment check method suggested by Marc
v2-v3: make code more readable
v3-v4: keep type safety
---
cmds-restore.c | 29 +++--
1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/cmds-restore.c b/cmds-restore.c
index 38a131e..fa5d5d1 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -56,7 +56,10 @@ static int get_xattrs = 0;
static int dry_run = 0;
#define LZO_LEN 4
-#define PAGE_CACHE_SIZE 4096
+#define PAGE_CACHE_SIZE 4096UL
+#define PAGE_CACHE_MASK (~(PAGE_CACHE_SIZE - 1))
+#define PAGE_CACHE_ALIGN(addr) (((addr) + PAGE_CACHE_SIZE - 1) \
+PAGE_CACHE_MASK)
#define lzo1x_worst_compress(x) ((x) + ((x) / 16) + 64 + 3)
static int decompress_zlib(char *inbuf, char *outbuf, u64 compress_len,
@@ -93,6 +96,28 @@ static inline size_t read_compress_length(unsigned char *buf)
return le32_to_cpu(dlen);
}
+static void align_if_need(size_t *tot_in, size_t *in_len)
+{
+ size_t tot_in_aligned;
+ size_t bytes_left;
+
+ tot_in_aligned = PAGE_CACHE_ALIGN(*tot_in);
+ bytes_left = tot_in_aligned - *tot_in;
+
+ if (bytes_left = LZO_LEN)
+ return;
+
+ /*
+* The LZO_LEN bytes is guaranteed to be
+* in one page as a whole, so if a page
+* has fewer than LZO_LEN bytes left,
+* the LZO_LEN bytes should be fetched
+* at the start of the next page
+*/
+ *in_len += bytes_left;
+ *tot_in = tot_in_aligned;
+}
+
static int decompress_lzo(unsigned char *inbuf, char *outbuf, u64 compress_len,
u64 *decompress_len)
{
@@ -135,8 +160,8 @@ static int decompress_lzo(unsigned char *inbuf, char
*outbuf, u64 compress_len,
}
out_len += new_len;
outbuf += new_len;
+ align_if_need(tot_in, in_len);
inbuf += in_len;
- tot_in += in_len;
}
*decompress_len = out_len;
--
1.8.1.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 3/3] btrfs-progs: fix device missing of btrfs fi show with seeding devices
On Thu, 2014-09-18 at 13:59 +0800, Anand Jain wrote:
Hi Gui,
Thanks for the attempt to fix this. more below..
On 09/18/2014 11:31 AM, Gui Hecheng wrote:
*Note*: this handles the problem under umounted state,
the problem under mounted state is already fixed by Anand.
Steps to reproduce:
# mkfs.btrfs -f /dev/sda1
# btrfstune -S 1 /dev/sda1
# mount /dev/sda1 /mnt
# btrfs dev add /dev/sda2 /mnt
# umount /mnt == (umounted)
# btrfs fi show /dev/sda2
result:
Label: none uuid: XX
Total devices 2 FS bytes used 368.00KiB
devid2 size 9.31GiB used 1.25GiB path /dev/sda2
*** Some devices missing
Btrfs v3.16-67-g69f54ea-dirty
It is because the @btrfs_scan_lblkid procedure is not capable of detecting
seeding devices since the seeding devices have different FSIDs from
derived devices. So when it tries to show all devices under the derived
fs, only the derived devices are shown.
Hmm.. thats not true. btrfs_scan_lblkid() finds all btrfs devices
including the seed/sprout devices. However btrfs_scan_lblkid won't
establish mapping between the seed and sprout devices.
Yes, I think the seed/sprout mapping relation do have to be clarified.
Actually the @open_ctree deal with the seeding devices properly, so
we can make use of it to find seeding devices.
We call @open_ctree on every block device with a btrfs on it,
and all devices under the opening filesystem including the seed devices
will be ready to be shown.
looking at the below code, I doubt if this will work with
nested seed-sprout relations. ? what did I miss ?
Oh, this seems to be my missing point. I will test it in the next
version.
Its better to keep seed sprout mapping part separate from the device
scan using lblkid.
That sounds to be a good idea. So I will keep the original
@btrfs_scan_lblkid() and try to build the seed/sprout mapping after it.
Thanks very much for your comments, please allow me to give a re-shot.
-Gui
Thanks, Anand
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
cmds-filesystem.c | 104
--
1 file changed, 69 insertions(+), 35 deletions(-)
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index dc5185e..f978175 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -28,6 +28,7 @@
#include mntent.h
#include linux/limits.h
#include getopt.h
+#include blkid/blkid.h
#include kerncompat.h
#include ctree.h
@@ -268,10 +269,26 @@ static int cmp_device_id(void *priv, struct list_head
*a,
da-devid db-devid ? 1 : 0;
}
+static void print_devices(struct btrfs_fs_devices *fs_devices, u64
*devs_found)
+{
+ struct btrfs_device *device;
+ struct list_head *cur;
+
+ list_sort(NULL, fs_devices-devices, cmp_device_id);
+ list_for_each(cur, fs_devices-devices) {
+ device = list_entry(cur, struct btrfs_device, dev_list);
+
+ printf(\tdevid %4llu size %s used %s path %s\n,
+ (unsigned long long)device-devid,
+ pretty_size(device-total_bytes),
+ pretty_size(device-bytes_used), device-name);
+ (*devs_found)++;
+ }
+}
+
static void print_one_uuid(struct btrfs_fs_devices *fs_devices)
{
char uuidbuf[BTRFS_UUID_UNPARSED_SIZE];
- struct list_head *cur;
struct btrfs_device *device;
u64 devs_found = 0;
u64 total;
@@ -293,17 +310,10 @@ static void print_one_uuid(struct btrfs_fs_devices
*fs_devices)
(unsigned long long)total,
pretty_size(device-super_bytes_used));
- list_sort(NULL, fs_devices-devices, cmp_device_id);
- list_for_each(cur, fs_devices-devices) {
- device = list_entry(cur, struct btrfs_device, dev_list);
-
- printf(\tdevid %4llu size %s used %s path %s\n,
- (unsigned long long)device-devid,
- pretty_size(device-total_bytes),
- pretty_size(device-bytes_used), device-name);
+ if (fs_devices-seed)
+ print_devices(fs_devices-seed, devs_found);
+ print_devices(fs_devices, devs_found);
- devs_found++;
- }
if (devs_found total) {
printf(\t*** Some devices missing\n);
}
@@ -489,6 +499,53 @@ out:
return ret;
}
+static int scan_all_fs_lblkid(char *search_target)
+{
+ blkid_dev_iterate iter = NULL;
+ blkid_dev dev = NULL;
+ blkid_cache cache = NULL;
+ char path[PATH_MAX];
+ struct btrfs_fs_info *fs_info;
+ int found = 0;
+
+ if (blkid_get_cache(cache, 0) 0) {
+ printf(ERROR: lblkid cache get failed\n);
+ return -1;
+ }
+ blkid_probe_all(cache);
+ iter = blkid_dev_iterate_begin(cache
Re: [PATCH] btrfs-progs: fix page align issue for lzo compress in restore
On Thu, 2014-09-18 at 10:25 +0200, Marc Dietrich wrote:
Hello Gui,
Am Donnerstag, 18. September 2014, 11:34:43 schrieb Gui Hecheng:
When runing restore under lzo compression, bad compress length
problems are encountered.
It is because there is a page align problem with the @decompress_lzo,
as follows:
|--| ||-| |--|...|--|
page ^page page
|
3 bytes left
When lzo compress pages im RAM, lzo will ensure that
the 4 bytes len will be in one page as a whole.
There is a situation that 3 (or less) bytes are left
at the end of a page, and then the 4 bytes len is
stored at the start of the next page.
But the @decompress_lzo doesn't goto the start of
the next page and continue to read the next 4 bytes
which is across two pages, so a random value is fetched
as a bad compress length.
So we just switch to the page-aligned start position to read
the len of next piece of data when bad compress length is encounterd.
If we still get bad compress length in this case, then there is a
real bad compress length, and we shall report error.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
cmds-restore.c | 20
1 file changed, 20 insertions(+)
diff --git a/cmds-restore.c b/cmds-restore.c
index 38a131e..8b230ab 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -57,6 +57,9 @@ static int dry_run = 0;
#define LZO_LEN 4
#define PAGE_CACHE_SIZE 4096
+#define PAGE_CACHE_MASK (~(PAGE_CACHE_SIZE - 1))
+#define PAGE_CACHE_ALIGN(addr) (((addr) + PAGE_CACHE_SIZE - 1) \
+PAGE_CACHE_MASK)
#define lzo1x_worst_compress(x) ((x) + ((x) / 16) + 64 + 3)
static int decompress_zlib(char *inbuf, char *outbuf, u64 compress_len,
@@ -101,6 +104,8 @@ static int decompress_lzo(unsigned char *inbuf, char
*outbuf, u64 compress_len,
size_t out_len = 0;
size_t tot_len;
size_t tot_in;
+ size_t tot_in_aligned;
+ int aligned = 0;
int ret;
ret = lzo_init();
@@ -117,6 +122,20 @@ static int decompress_lzo(unsigned char *inbuf, char
*outbuf, u64 compress_len,
in_len = read_compress_length(inbuf);
if ((tot_in + LZO_LEN + in_len) tot_len) {
+ /*
+* The LZO_LEN bytes is guaranteed to be
+* in one page as a whole, so if a page
+* has fewer than LZO_LEN bytes left,
+* the LZO_LEN bytes should be fetched
+* at the start of the next page
+*/
+ if (!aligned) {
+ tot_in_aligned = PAGE_CACHE_ALIGN(tot_in);
+ inbuf += (tot_in_aligned - tot_in);
+ tot_in = tot_in_aligned;
+ aligned = 1;
+ continue;
+ }
Small question, shouldn't the aligned check be moved out of the if block?
First, we could have a bad length caused by the alignment which could result
in a stream length less than tot_len.
Ah, you have reminded me of a missing case to be covered.
Second, if we know that the length record never crosses a page, why not
always check for proper alignment. I think the overhead should be minimal.
I don't think alignment should be checked always, because in the
normal case the lzo stuff is compact:
[len][data][len][data]...
It is never aligned to anything and we never knows where next @len
starts before we read the former one. The alignement-related issue is a
rare case.
Thanks for your comments.
-Gui
Marc
fprintf(stderr, bad compress length %lu\n,
(unsigned long)in_len);
return -1;
@@ -137,6 +156,7 @@ static int decompress_lzo(unsigned char *inbuf, char
*outbuf, u64 compress_len,
outbuf += new_len;
inbuf += in_len;
tot_in += in_len;
+ aligned = 0;
}
*decompress_len = out_len;
--
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-progs: super-recover: fix double free fs_devices memory
On Thu, 2014-09-18 at 05:01 -0400, Wang Shilong wrote:
super-recover collects btrfs devices infomation using existed
functions scan_one_devices().
Problem is fs_devices is freed twice in close_ctree() and
free_recover_superblock() for super correction path.
Fix this problem by checking whether fs_devices memory
have been freed before we free it.
Cc: Eric Sandeen sand...@redhat.com
Cc: Chris Murphy li...@colorremedies.com
Signed-off-by: Wang Shilong wangshilong1...@gmail.com
---
super-recover.c | 13 +++--
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/super-recover.c b/super-recover.c
index 767de4b..419b86a 100644
--- a/super-recover.c
+++ b/super-recover.c
@@ -69,21 +69,11 @@ void init_recover_superblock(struct
btrfs_recover_superblock *recover)
static
void free_recover_superblock(struct btrfs_recover_superblock *recover)
{
- struct btrfs_device *device;
struct super_block_record *record;
if (!recover-fs_devices)
return;
- while (!list_empty(recover-fs_devices-devices)) {
- device = list_entry(recover-fs_devices-devices.next,
- struct btrfs_device, dev_list);
- list_del_init(device-dev_list);
- free(device-name);
- free(device);
- }
- free(recover-fs_devices);
-
while (!list_empty(recover-good_supers)) {
record = list_entry(recover-good_supers.next,
struct super_block_record, list);
@@ -341,6 +331,9 @@ int btrfs_recover_superblocks(const char *dname,
no_recover:
recover_err_str(ret);
free_recover_superblock(recover);
+ /* check if we have freed fs_deivces in close_ctree() */
+ if (!root)
+ btrfs_close_devices(recover.fs_devices);
return ret;
}
nice catch! +20, recorded. ^_^
--
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-progs: fix page align issue for lzo compress inrestore
On Thu, 2014-09-18 at 11:25 +0200, Marc Dietrich wrote:
Am Donnerstag, 18. September 2014, 17:10:54 schrieb Gui Hecheng:
On Thu, 2014-09-18 at 10:25 +0200, Marc Dietrich wrote:
Hello Gui,
Am Donnerstag, 18. September 2014, 11:34:43 schrieb Gui Hecheng:
When runing restore under lzo compression, bad compress length
problems are encountered.
It is because there is a page align problem with the @decompress_lzo,
as follows:
|--| ||-| |--|...|--|
page ^page page
|
3 bytes left
When lzo compress pages im RAM, lzo will ensure that
the 4 bytes len will be in one page as a whole.
There is a situation that 3 (or less) bytes are left
at the end of a page, and then the 4 bytes len is
stored at the start of the next page.
But the @decompress_lzo doesn't goto the start of
the next page and continue to read the next 4 bytes
which is across two pages, so a random value is fetched
as a bad compress length.
So we just switch to the page-aligned start position to read
the len of next piece of data when bad compress length is encounterd.
If we still get bad compress length in this case, then there is a
real bad compress length, and we shall report error.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
cmds-restore.c | 20
1 file changed, 20 insertions(+)
diff --git a/cmds-restore.c b/cmds-restore.c
index 38a131e..8b230ab 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -57,6 +57,9 @@ static int dry_run = 0;
#define LZO_LEN 4
#define PAGE_CACHE_SIZE 4096
+#define PAGE_CACHE_MASK (~(PAGE_CACHE_SIZE - 1))
+#define PAGE_CACHE_ALIGN(addr) (((addr) + PAGE_CACHE_SIZE - 1) \
+
PAGE_CACHE_MASK)
#define lzo1x_worst_compress(x) ((x) + ((x) / 16) + 64 + 3)
static int decompress_zlib(char *inbuf, char *outbuf, u64 compress_len,
@@ -101,6 +104,8 @@ static int decompress_lzo(unsigned char *inbuf,
char *outbuf, u64 compress_len,
size_t out_len = 0;
size_t tot_len;
size_t tot_in;
+ size_t tot_in_aligned;
+ int aligned = 0;
int ret;
ret = lzo_init();
@@ -117,6 +122,20 @@ static int decompress_lzo(unsigned char *inbuf,
char *outbuf, u64 compress_len,
in_len = read_compress_length(inbuf);
if ((tot_in + LZO_LEN + in_len) tot_len) {
+ /*
+* The LZO_LEN bytes is guaranteed to be
+* in one page as a whole, so if a page
+* has fewer than LZO_LEN bytes left,
+* the LZO_LEN bytes should be fetched
+* at the start of the next page
+*/
+ if (!aligned) {
+ tot_in_aligned =
PAGE_CACHE_ALIGN(tot_in);
+ inbuf += (tot_in_aligned - tot_in);
+ tot_in = tot_in_aligned;
+ aligned = 1;
+ continue;
+ }
Small question, shouldn't the aligned check be moved out of the if block?
First, we could have a bad length caused by the alignment which could
result
in a stream length less than tot_len.
Ah, you have reminded me of a missing case to be covered.
Second, if we know that the length record never crosses a page, why not
always check for proper alignment. I think the overhead should be minimal.
I don't think alignment should be checked always, because in the
normal case the lzo stuff is compact:
[len][data][len][data]...
It is never aligned to anything and we never knows where next @len
starts before we read the former one. The alignement-related issue is a
rare case.
sorry, my wording was wrong. I mean always check for page crossing of the
length
record and move forward if yes.
Ah, this time I see, that is a good idea. Thanks!
-Gui
Marc
--
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 corruption report
On Thu, 2014-09-18 at 12:47 +, Zooko Wilcox-OHearn wrote: Thank you! I will try to restore using this patch. What branch of what btrfs tools git repo should I apply the patch to? Regards, Zooko At least I think the following repo/v3.17.x branch has all restore-related patches included. git://github.com/kdave/btrfs-progs.git This is a mirror of the latest devel repo that I am on. -Gui -- 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: remove scan_for_btrfs()
From: Anand Jain anand.j...@oracle.com
With the changes as in the previous patch, now scan_for_btrfs()
is an unused function. So delete it.
Signed-off-by: Anand Jain anand.j...@oracle.com
---
utils.c | 15 ---
utils.h | 1 -
2 files changed, 16 deletions(-)
diff --git a/utils.c b/utils.c
index 0ae0475..0cd97c7 100644
--- a/utils.c
+++ b/utils.c
@@ -2173,21 +2173,6 @@ int btrfs_scan_lblkid(int update_kernel)
return 0;
}
-/*
- * scans devs for the btrfs
-*/
-int scan_for_btrfs(int where, int update_kernel)
-{
- int ret = 0;
-
- switch (where) {
- case BTRFS_SCAN_LBLKID:
- ret = btrfs_scan_lblkid(update_kernel);
- break;
- }
- return ret;
-}
-
int is_vol_small(char *file)
{
int fd = -1;
diff --git a/utils.h b/utils.h
index 13f2e60..12466a6 100644
--- a/utils.h
+++ b/utils.h
@@ -105,7 +105,6 @@ u64 btrfs_device_size(int fd, struct stat *st);
/* Helper to always get proper size of the destination string */
#define strncpy_null(dest, src) __strncpy__null(dest, src, sizeof(dest))
int test_dev_for_mkfs(char *file, int force_overwrite, char *estr);
-int scan_for_btrfs(int where, int update_kernel);
int get_label_mounted(const char *mount_path, char *labelp);
int test_num_disk_vs_raid(u64 metadata_profile, u64 data_profile,
u64 dev_cnt, int mixed, char *estr);
--
1.8.1.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
[PATCH 1/3] btrfs-progs: remove BTRFS_SCAN_PROC scan method
From: Anand Jain anand.j...@oracle.com
The libblkid scan method which was introduced later, will also
scan devices under /proc/partitions. So we don't have to do
the explicit scan of the same.
Remove the scan method BTRFS_SCAN_PROC.
Signed-off-by: Anand Jain anand.j...@oracle.com
---
cmds-device.c | 5 ++---
cmds-filesystem.c | 10 +-
disk-io.c | 2 +-
utils.c | 5 +
utils.h | 5 ++---
5 files changed, 11 insertions(+), 16 deletions(-)
diff --git a/cmds-device.c b/cmds-device.c
index a7183e3..a728f21 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -200,13 +200,13 @@ static int cmd_rm_dev(int argc, char **argv)
static const char * const cmd_scan_dev_usage[] = {
btrfs device scan [(-d|--all-devices)|device [device...]],
Scan devices for a btrfs filesystem,
+-d|--all-devices (deprecated),
NULL
};
static int cmd_scan_dev(int argc, char **argv)
{
int i, fd, e;
- int where = BTRFS_SCAN_LBLKID;
int devstart = 1;
int all = 0;
int ret = 0;
@@ -224,7 +224,6 @@ static int cmd_scan_dev(int argc, char **argv)
break;
switch (c) {
case 'd':
- where = BTRFS_SCAN_PROC;
all = 1;
break;
default:
@@ -237,7 +236,7 @@ static int cmd_scan_dev(int argc, char **argv)
if (all || argc == 1) {
printf(Scanning for Btrfs filesystems\n);
- ret = scan_for_btrfs(where, BTRFS_UPDATE_KERNEL);
+ ret = btrfs_scan_lblkid(BTRFS_UPDATE_KERNEL);
if (ret)
fprintf(stderr, ERROR: error %d while scanning\n,
ret);
goto out;
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 69c1ca5..dc5185e 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -505,7 +505,7 @@ static int cmd_show(int argc, char **argv)
struct list_head *cur_uuid;
char *search = NULL;
int ret;
- int where = BTRFS_SCAN_LBLKID;
+ int where = -1; // default, search both kernel and udev
int type = 0;
char mp[BTRFS_PATH_NAME_MAX + 1];
char path[PATH_MAX];
@@ -526,7 +526,7 @@ static int cmd_show(int argc, char **argv)
break;
switch (c) {
case 'd':
- where = BTRFS_SCAN_PROC;
+ where = BTRFS_SCAN_LBLKID;
break;
case 'm':
where = BTRFS_SCAN_MOUNTED;
@@ -550,7 +550,7 @@ static int cmd_show(int argc, char **argv)
* right away
*/
if (type == BTRFS_ARG_BLKDEV) {
- if (where == BTRFS_SCAN_PROC) {
+ if (where == BTRFS_SCAN_LBLKID) {
/* we need to do this because
* legacy BTRFS_SCAN_DEV
* provides /dev/dm-x paths
@@ -586,7 +586,7 @@ static int cmd_show(int argc, char **argv)
}
}
- if (where == BTRFS_SCAN_PROC)
+ if (where == BTRFS_SCAN_LBLKID)
goto devs_only;
/* show mounted btrfs */
@@ -601,7 +601,7 @@ static int cmd_show(int argc, char **argv)
goto out;
devs_only:
- ret = scan_for_btrfs(where, !BTRFS_UPDATE_KERNEL);
+ ret = btrfs_scan_lblkid(!BTRFS_UPDATE_KERNEL);
if (ret) {
fprintf(stderr, ERROR: %d while scanning\n, ret);
diff --git a/disk-io.c b/disk-io.c
index c7901f4..9fe8769 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -1005,7 +1005,7 @@ int btrfs_scan_fs_devices(int fd, const char *path,
}
if (total_devs != 1) {
- ret = scan_for_btrfs(BTRFS_SCAN_PROC, run_ioctl);
+ ret = btrfs_scan_lblkid(run_ioctl);
if (ret)
return ret;
}
diff --git a/utils.c b/utils.c
index fb78dd6..0ae0475 100644
--- a/utils.c
+++ b/utils.c
@@ -1150,7 +1150,7 @@ int check_mounted_where(int fd, const char *file, char
*where, int size,
/* scan other devices */
if (is_btrfs total_devs 1) {
- if ((ret = scan_for_btrfs(BTRFS_SCAN_PROC,
!BTRFS_UPDATE_KERNEL)))
+ if ((ret = btrfs_scan_lblkid(!BTRFS_UPDATE_KERNEL)))
return ret;
}
@@ -2181,9 +2181,6 @@ int scan_for_btrfs(int where, int update_kernel)
int ret = 0;
switch (where) {
- case BTRFS_SCAN_PROC:
- ret = btrfs_scan_block_devices(update_kernel);
- break;
case BTRFS_SCAN_LBLKID:
ret = btrfs_scan_lblkid(update_kernel);
break;
diff --git a/utils.h b/utils.h
index 01b3259..13f2e60 100644
--- a/utils.h
+++ b/utils.h
@@ -26,9 +26,8 @@
#define BTRFS_MKFS_SYSTEM_GROUP_SIZE (4 * 1024 * 1024)
#define
[PATCH 3/3] btrfs-progs: fix device missing of btrfs fi show with seeding devices
*Note*: this handles the problem under umounted state,
the problem under mounted state is already fixed by Anand.
Steps to reproduce:
# mkfs.btrfs -f /dev/sda1
# btrfstune -S 1 /dev/sda1
# mount /dev/sda1 /mnt
# btrfs dev add /dev/sda2 /mnt
# umount /mnt == (umounted)
# btrfs fi show /dev/sda2
result:
Label: none uuid: XX
Total devices 2 FS bytes used 368.00KiB
devid2 size 9.31GiB used 1.25GiB path /dev/sda2
*** Some devices missing
Btrfs v3.16-67-g69f54ea-dirty
It is because the @btrfs_scan_lblkid procedure is not capable of detecting
seeding devices since the seeding devices have different FSIDs from
derived devices. So when it tries to show all devices under the derived
fs, only the derived devices are shown.
Actually the @open_ctree deal with the seeding devices properly, so
we can make use of it to find seeding devices.
We call @open_ctree on every block device with a btrfs on it,
and all devices under the opening filesystem including the seed devices
will be ready to be shown.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
cmds-filesystem.c | 104 --
1 file changed, 69 insertions(+), 35 deletions(-)
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index dc5185e..f978175 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -28,6 +28,7 @@
#include mntent.h
#include linux/limits.h
#include getopt.h
+#include blkid/blkid.h
#include kerncompat.h
#include ctree.h
@@ -268,10 +269,26 @@ static int cmp_device_id(void *priv, struct list_head *a,
da-devid db-devid ? 1 : 0;
}
+static void print_devices(struct btrfs_fs_devices *fs_devices, u64 *devs_found)
+{
+ struct btrfs_device *device;
+ struct list_head *cur;
+
+ list_sort(NULL, fs_devices-devices, cmp_device_id);
+ list_for_each(cur, fs_devices-devices) {
+ device = list_entry(cur, struct btrfs_device, dev_list);
+
+ printf(\tdevid %4llu size %s used %s path %s\n,
+ (unsigned long long)device-devid,
+ pretty_size(device-total_bytes),
+ pretty_size(device-bytes_used), device-name);
+ (*devs_found)++;
+ }
+}
+
static void print_one_uuid(struct btrfs_fs_devices *fs_devices)
{
char uuidbuf[BTRFS_UUID_UNPARSED_SIZE];
- struct list_head *cur;
struct btrfs_device *device;
u64 devs_found = 0;
u64 total;
@@ -293,17 +310,10 @@ static void print_one_uuid(struct btrfs_fs_devices
*fs_devices)
(unsigned long long)total,
pretty_size(device-super_bytes_used));
- list_sort(NULL, fs_devices-devices, cmp_device_id);
- list_for_each(cur, fs_devices-devices) {
- device = list_entry(cur, struct btrfs_device, dev_list);
-
- printf(\tdevid %4llu size %s used %s path %s\n,
- (unsigned long long)device-devid,
- pretty_size(device-total_bytes),
- pretty_size(device-bytes_used), device-name);
+ if (fs_devices-seed)
+ print_devices(fs_devices-seed, devs_found);
+ print_devices(fs_devices, devs_found);
- devs_found++;
- }
if (devs_found total) {
printf(\t*** Some devices missing\n);
}
@@ -489,6 +499,53 @@ out:
return ret;
}
+static int scan_all_fs_lblkid(char *search_target)
+{
+ blkid_dev_iterate iter = NULL;
+ blkid_dev dev = NULL;
+ blkid_cache cache = NULL;
+ char path[PATH_MAX];
+ struct btrfs_fs_info *fs_info;
+ int found = 0;
+
+ if (blkid_get_cache(cache, 0) 0) {
+ printf(ERROR: lblkid cache get failed\n);
+ return -1;
+ }
+ blkid_probe_all(cache);
+ iter = blkid_dev_iterate_begin(cache);
+ blkid_dev_set_search(iter, TYPE, btrfs);
+ while (blkid_dev_next(iter, dev) == 0) {
+ dev = blkid_verify(cache, dev);
+ if (!dev)
+ continue;
+ strncpy(path, blkid_dev_devname(dev), PATH_MAX);
+ fs_info = open_ctree_fs_info(path, 0, 0, OPEN_CTREE_PARTIAL);
+ if (!fs_info)
+ continue;
+
+ if (search_target
+!uuid_search(fs_info-fs_devices, search_target)) {
+ close_ctree(fs_info-fs_root);
+ continue;
+ }
+
+ if (search_target)
+ found = 1;
+ print_one_uuid(fs_info-fs_devices);
+
+ close_ctree(fs_info-fs_root);
+ }
+ blkid_dev_iterate_end(iter);
+ blkid_put_cache(cache);
+
+ if (search_target !found)
+ return 1;
+
+ return 0
[PATCH] btrfs-progs: fix page align issue for lzo compress in restore
When runing restore under lzo compression, bad compress length
problems are encountered.
It is because there is a page align problem with the @decompress_lzo,
as follows:
|--| ||-| |--|...|--|
page ^page page
|
3 bytes left
When lzo compress pages im RAM, lzo will ensure that
the 4 bytes len will be in one page as a whole.
There is a situation that 3 (or less) bytes are left
at the end of a page, and then the 4 bytes len is
stored at the start of the next page.
But the @decompress_lzo doesn't goto the start of
the next page and continue to read the next 4 bytes
which is across two pages, so a random value is fetched
as a bad compress length.
So we just switch to the page-aligned start position to read
the len of next piece of data when bad compress length is encounterd.
If we still get bad compress length in this case, then there is a
real bad compress length, and we shall report error.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
cmds-restore.c | 20
1 file changed, 20 insertions(+)
diff --git a/cmds-restore.c b/cmds-restore.c
index 38a131e..8b230ab 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -57,6 +57,9 @@ static int dry_run = 0;
#define LZO_LEN 4
#define PAGE_CACHE_SIZE 4096
+#define PAGE_CACHE_MASK (~(PAGE_CACHE_SIZE - 1))
+#define PAGE_CACHE_ALIGN(addr) (((addr) + PAGE_CACHE_SIZE - 1) \
+PAGE_CACHE_MASK)
#define lzo1x_worst_compress(x) ((x) + ((x) / 16) + 64 + 3)
static int decompress_zlib(char *inbuf, char *outbuf, u64 compress_len,
@@ -101,6 +104,8 @@ static int decompress_lzo(unsigned char *inbuf, char
*outbuf, u64 compress_len,
size_t out_len = 0;
size_t tot_len;
size_t tot_in;
+ size_t tot_in_aligned;
+ int aligned = 0;
int ret;
ret = lzo_init();
@@ -117,6 +122,20 @@ static int decompress_lzo(unsigned char *inbuf, char
*outbuf, u64 compress_len,
in_len = read_compress_length(inbuf);
if ((tot_in + LZO_LEN + in_len) tot_len) {
+ /*
+* The LZO_LEN bytes is guaranteed to be
+* in one page as a whole, so if a page
+* has fewer than LZO_LEN bytes left,
+* the LZO_LEN bytes should be fetched
+* at the start of the next page
+*/
+ if (!aligned) {
+ tot_in_aligned = PAGE_CACHE_ALIGN(tot_in);
+ inbuf += (tot_in_aligned - tot_in);
+ tot_in = tot_in_aligned;
+ aligned = 1;
+ continue;
+ }
fprintf(stderr, bad compress length %lu\n,
(unsigned long)in_len);
return -1;
@@ -137,6 +156,7 @@ static int decompress_lzo(unsigned char *inbuf, char
*outbuf, u64 compress_len,
outbuf += new_len;
inbuf += in_len;
tot_in += in_len;
+ aligned = 0;
}
*decompress_len = out_len;
--
1.8.1.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: fs corruption report
On Mon, 2014-09-01 at 15:25 +, Zooko Wilcox-OHearn wrote: I'm more than happy to try out patches and even focus my own brain on diagnosing it, if I can. I'm hoping to regain access to some of my files on my btrfs partition, and also I would enjoy helping get this improved. :-) So if you want me to try an experiment, just email me. Unfortunately I can't just give you a copy of the partition, since it has confidential information on it. Regards, Zooko Hi, Zooko, Marc, I'm glad that I could send some feedbacks, The following piece deals with the lzo decompress problem with restore https://patchwork.kernel.org/patch/4928831/ Hope that it helps. -Gui -- 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 to deal with The chunks in memory can not match the metadata of the fs. Repair failed.?
On Mon, 2014-09-15 at 16:59 +0200, Karl-Philipp Richter wrote: Hi together, I'm currently encountering another issue with `btrfs rescue chunk-recover`. After some seconds of maximal I/O activity, the program is idle for some seconds and then prints The chunks in memory can not match the metadata of the fs. Repair failed. Fail to recover the chunk tree. I don't find any mention of this output except in the source code (this is as well the only thing $SEARCH_ENGINE points me to). What is the recommended next step? It would be nice to see the situation (The chunks in memory can not match the metadata of the fs) or the output mentioned in `man btrfs rescue`. Any help is appreciated. -Kalle Hi Kalle, If you are encountering the above msg, it means the chunk tree has some inconsistent stuff with other trees like device tree extent tree. You can specify the '-v' option for chunk-recover to see more details. And also, the btrfs check cmd may tell more about the story. Thanks, Gui -- 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: [PATCH v2] btrfs-progs: deal with conflict options for btrfs fi show
On Fri, 2014-09-12 at 14:56 +0900, Satoru Takeuchi wrote:
Hi Gui,
(2014/09/12 10:15), Gui Hecheng wrote:
For btrfs fi show, -d|--all-devices -m|--mounted will
overwrite each other, so if specified both, let the user
know that he should not use them at the same time.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
changelog:
v1-v2: add option conflict descriptions to manpage and usage.
---
Documentation/btrfs-filesystem.txt | 9 ++---
cmds-filesystem.c | 12 ++--
2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/Documentation/btrfs-filesystem.txt
b/Documentation/btrfs-filesystem.txt
index c9c0b00..d3d2dcc 100644
--- a/Documentation/btrfs-filesystem.txt
+++ b/Documentation/btrfs-filesystem.txt
@@ -20,15 +20,18 @@ SUBCOMMAND
*df* path [path...]::
Show space usage information for a mount point.
-*show* [--mounted|--all-devices|path|uuid|device|label]::
+*show* [-m|--mounted|-d|--all-devices|path|uuid|device|label]::
This line seems to be too long. Please see also the
following thread.
https://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg36270.html
Hi Satoru,
Ah, there is a patch that is changing the same document before but not
merged yet. So I think I will rebase my additional words about the
option conflict after the former patch merged.
--
Hi David,
Sorry to bother, would you please give a glance at the v1 patch and
ignore this v2 first. And I will add the option conflict stuff in
another patch after the former path below merged. Is that OK?
https://patchwork.kernel.org/patch/4711831/
-Gui
Thanks,
Satoru
Show the btrfs filesystem with some additional info.
+
If no option nor path|uuid|device|label is passed, btrfs shows
information of all the btrfs filesystem both mounted and unmounted.
-If '--mounted' is passed, it would probe btrfs kernel to list mounted btrfs
+If '-m|--mounted' is passed, it would probe btrfs kernel to list mounted
btrfs
filesystem(s);
-If '--all-devices' is passed, all the devices under /dev are scanned;
+If '-d|--all-devices' is passed, all the devices under /dev are scanned;
otherwise the devices list is extracted from the /proc/partitions file.
+Don't combine -m|--mounted and -d|--all-devices, because these two options
+will overwrite each other, and only one scan way will be adopted,
+probe the kernel to scan or scan devices under /dev.
*sync* path::
Force a sync for the filesystem identified by path.
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 69c1ca5..51c4c55 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -495,6 +495,7 @@ static const char * const cmd_show_usage[] = {
-d|--all-devices show only disks under /dev containing btrfs
filesystem,
-m|--mounted show only mounted btrfs,
If no argument is given, structure of all present filesystems is
shown.,
+ Don't combine -d|--all-devices and -m|--mounted, refer to manpage for
details.,
NULL
};
@@ -526,16 +527,23 @@ static int cmd_show(int argc, char **argv)
break;
switch (c) {
case 'd':
- where = BTRFS_SCAN_PROC;
+ where = ~BTRFS_SCAN_LBLKID;
+ where |= BTRFS_SCAN_PROC;
break;
case 'm':
- where = BTRFS_SCAN_MOUNTED;
+ where = ~BTRFS_SCAN_LBLKID;
+ where |= BTRFS_SCAN_MOUNTED;
break;
default:
usage(cmd_show_usage);
}
}
+ if ((where BTRFS_SCAN_PROC) (where BTRFS_SCAN_MOUNTED)) {
+ fprintf(stderr, Don't use -d|--all-devices and -m|--mounted
options at the same time.\n);
+ usage(cmd_show_usage);
+ }
+
if (check_argc_max(argc, optind + 1))
usage(cmd_show_usage);
--
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 v2] btrfs-progs: deal with conflict options for btrfs fi show
For btrfs fi show, -d|--all-devices -m|--mounted will
overwrite each other, so if specified both, let the user
know that he should not use them at the same time.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
changelog:
v1-v2: add option conflict descriptions to manpage and usage.
---
Documentation/btrfs-filesystem.txt | 9 ++---
cmds-filesystem.c | 12 ++--
2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/Documentation/btrfs-filesystem.txt
b/Documentation/btrfs-filesystem.txt
index c9c0b00..d3d2dcc 100644
--- a/Documentation/btrfs-filesystem.txt
+++ b/Documentation/btrfs-filesystem.txt
@@ -20,15 +20,18 @@ SUBCOMMAND
*df* path [path...]::
Show space usage information for a mount point.
-*show* [--mounted|--all-devices|path|uuid|device|label]::
+*show* [-m|--mounted|-d|--all-devices|path|uuid|device|label]::
Show the btrfs filesystem with some additional info.
+
If no option nor path|uuid|device|label is passed, btrfs shows
information of all the btrfs filesystem both mounted and unmounted.
-If '--mounted' is passed, it would probe btrfs kernel to list mounted btrfs
+If '-m|--mounted' is passed, it would probe btrfs kernel to list mounted btrfs
filesystem(s);
-If '--all-devices' is passed, all the devices under /dev are scanned;
+If '-d|--all-devices' is passed, all the devices under /dev are scanned;
otherwise the devices list is extracted from the /proc/partitions file.
+Don't combine -m|--mounted and -d|--all-devices, because these two options
+will overwrite each other, and only one scan way will be adopted,
+probe the kernel to scan or scan devices under /dev.
*sync* path::
Force a sync for the filesystem identified by path.
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 69c1ca5..51c4c55 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -495,6 +495,7 @@ static const char * const cmd_show_usage[] = {
-d|--all-devices show only disks under /dev containing btrfs
filesystem,
-m|--mounted show only mounted btrfs,
If no argument is given, structure of all present filesystems is
shown.,
+ Don't combine -d|--all-devices and -m|--mounted, refer to manpage for
details.,
NULL
};
@@ -526,16 +527,23 @@ static int cmd_show(int argc, char **argv)
break;
switch (c) {
case 'd':
- where = BTRFS_SCAN_PROC;
+ where = ~BTRFS_SCAN_LBLKID;
+ where |= BTRFS_SCAN_PROC;
break;
case 'm':
- where = BTRFS_SCAN_MOUNTED;
+ where = ~BTRFS_SCAN_LBLKID;
+ where |= BTRFS_SCAN_MOUNTED;
break;
default:
usage(cmd_show_usage);
}
}
+ if ((where BTRFS_SCAN_PROC) (where BTRFS_SCAN_MOUNTED)) {
+ fprintf(stderr, Don't use -d|--all-devices and -m|--mounted
options at the same time.\n);
+ usage(cmd_show_usage);
+ }
+
if (check_argc_max(argc, optind + 1))
usage(cmd_show_usage);
--
1.8.1.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
[PATCH] btrfs-progs: deal with conflict options for btrfs fi show
For btrfs fi show, -d|--all-devices -m|--mounted will
overwrite each other, so if specified both, let the user
know that he should not use them at the same time.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
cmds-filesystem.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 69c1ca5..78aeacc 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -526,16 +526,23 @@ static int cmd_show(int argc, char **argv)
break;
switch (c) {
case 'd':
- where = BTRFS_SCAN_PROC;
+ where = ~BTRFS_SCAN_LBLKID;
+ where |= BTRFS_SCAN_PROC;
break;
case 'm':
- where = BTRFS_SCAN_MOUNTED;
+ where = ~BTRFS_SCAN_LBLKID;
+ where |= BTRFS_SCAN_MOUNTED;
break;
default:
usage(cmd_show_usage);
}
}
+ if ((where BTRFS_SCAN_PROC) (where BTRFS_SCAN_MOUNTED)) {
+ fprintf(stderr, don't use -d|--all-devices and -m|--mounted
options at the same time\n);
+ usage(cmd_show_usage);
+ }
+
if (check_argc_max(argc, optind + 1))
usage(cmd_show_usage);
--
1.8.1.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: fs corruption report
On Mon, 2014-09-01 at 15:25 +, Zooko Wilcox-OHearn wrote: I'm more than happy to try out patches and even focus my own brain on diagnosing it, if I can. I'm hoping to regain access to some of my files on my btrfs partition, and also I would enjoy helping get this improved. :-) So if you want me to try an experiment, just email me. Unfortunately I can't just give you a copy of the partition, since it has confidential information on it. Regards, Zooko Hi Zooko, Marc, Firstly, thanks for your backtrace info, Marc. Sorry to reply late, since I'm offline these days. For the restore problem, I'm sure that the lzo decompress routine lacks the ability to handle some specific extent pattern. Here is my test result: I'm using a specific file for test /usr/lib/modules/$(uname -r)/kernel/net/irda/irda.ko. You can get it easily on your own box. # mkfs -t btrfs dev # mount -o compress-force=lzo dev mnt # cp irda.ko mnt # umount dev # btrfs restore -v dev restore_dir report: # bad compress length # failed to inflate btrfs-progs version: v3.16.x With the same file under no-compress zlib-compress, the restore will output a correct copy of irda.ko. I'm not sure whether the problem above has something to do with your problem. Hope that the messages above are helpful. -Gui -- 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-progs: remove btrfs_release_path before btrfs_free_path
The btrfs_free_path calls btrfs_release_path internally.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
disk-io.c | 1 -
file-item.c | 1 -
inode-map.c | 2 --
3 files changed, 4 deletions(-)
diff --git a/disk-io.c b/disk-io.c
index 9e44f10..0f9f374 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -628,7 +628,6 @@ struct btrfs_root *btrfs_read_fs_root_no_cache(struct
btrfs_fs_info *fs_info,
memcpy(root-root_key, location, sizeof(*location));
ret = 0;
out:
- btrfs_release_path(path);
btrfs_free_path(path);
if (ret) {
free(root);
diff --git a/file-item.c b/file-item.c
index 6f3708b..b46d7f1 100644
--- a/file-item.c
+++ b/file-item.c
@@ -306,7 +306,6 @@ found:
csum_size);
btrfs_mark_buffer_dirty(path-nodes[0]);
fail:
- btrfs_release_path(path);
btrfs_free_path(path);
return ret;
}
diff --git a/inode-map.c b/inode-map.c
index 3e138b5..1321bfb 100644
--- a/inode-map.c
+++ b/inode-map.c
@@ -90,12 +90,10 @@ int btrfs_find_free_objectid(struct btrfs_trans_handle
*trans,
// FIXME -ENOSPC
found:
root-last_inode_alloc = *objectid;
- btrfs_release_path(path);
btrfs_free_path(path);
BUG_ON(*objectid search_start);
return 0;
error:
- btrfs_release_path(path);
btrfs_free_path(path);
return ret;
}
--
1.8.1.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
[PATCH] btrfs-progs: remove wrong set_argv0 for restore
Before this patch, you could see the following after exec restore # :too few arguments The tool name btrfs restore is missing. The @set_argv0() function is introduced by: commit a184abc70f7b1468e6036ab576f1587ee0574668 btrfs-progs: move the check_argc_* functions into utils.c ... Also add a new function set_argv0 to set the correct tool name: *btrfs-image*: too few arguments But @set_argv0() only applies to the independent tools with the name pattern btrfs-***. Since restore is now is subcommand under btrfs, there is no need to use @set_argv0() before check_argc_* to repair the prompt tool name before too few arguments. Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com --- cmds-restore.c | 1 - 1 file changed, 1 deletion(-) diff --git a/cmds-restore.c b/cmds-restore.c index f909429..38a131e 100644 --- a/cmds-restore.c +++ b/cmds-restore.c @@ -1229,7 +1229,6 @@ int cmd_restore(int argc, char **argv) } } - set_argv0(argv); if (!list_roots check_argc_min(argc - optind, 2)) usage(cmd_restore_usage); else if (list_roots check_argc_min(argc - optind, 1)) -- 1.8.1.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 3/3] btrfs-progs: remove meaningless debug info for restore
On Fri, 2014-08-29 at 16:49 +0200, David Sterba wrote: On Thu, Aug 28, 2014 at 10:25:55AM +0800, Gui Hecheng wrote: The printf of @offset enlightens the user little. And the restore cmd is not a debugging tool, so just remove the debug-info-like printf. I'd like to let the restore command be more verbose as it's potentially working with a broken filesystem and getting details can help. There's -v option for increasing verbosity, please move the printf there. OK，since the -v option seems to serve as a debug helper. Thaks. -- 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: fix next_leaf in restore as it improperly skips some slots
When entering the next level node, the @next_leaf in restore forgets to start at the first slot. Just reset it to the first one. Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com --- cmds-restore.c | 1 + 1 file changed, 1 insertion(+) diff --git a/cmds-restore.c b/cmds-restore.c index e94592c..918f1fb 100644 --- a/cmds-restore.c +++ b/cmds-restore.c @@ -191,6 +191,7 @@ again: level++; if (level == BTRFS_MAX_LEVEL) return 1; + offset = 1; continue; } -- 1.8.1.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
[PATCH 3/3] btrfs-progs: remove meaningless debug info for restore
The printf of @offset enlightens the user little. And the restore cmd is not a debugging tool, so just remove the debug-info-like printf. Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com --- cmds-restore.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/cmds-restore.c b/cmds-restore.c index 918f1fb..648fbc9 100644 --- a/cmds-restore.c +++ b/cmds-restore.c @@ -310,8 +310,6 @@ static int copy_one_extent(struct btrfs_root *root, int fd, if (compress == BTRFS_COMPRESS_NONE) bytenr += offset; - if (offset) - printf(offset is %Lu\n, offset); /* we found a hole */ if (disk_size == 0) return 0; -- 1.8.1.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
[PATCH 1/3] btrfs-progs: fix len of read_extent_buffer for inline extent in restore
Steps to reproduce:
# mkfs.btrfs -f dev
# mount -o compress-force=lzo dev mnt
# for ((i=0;i4000;i++)); do
echo -n 'A' mnt/inline_data
done
# umount mnt
# valgrind --tool=memcheck --leak-check=full\
btrfs restore dev dest_dir
output:
==32118== Invalid read of size 1
==32118==at 0x4A0A4E4: memcpy@@GLIBC_2.14
==32118==by 0x43DC91: read_extent_buffer
==32118==by 0x421401: search_dir (cmds-restore.c:240)
==32118==by 0x422CBB: cmd_restore (cmds-restore.c:1317)
==32118==by 0x404709: main (btrfs.c:248)
==32118== Address 0x4c4f4ac is not stack'd, malloc'd or...
It is because when deal with inline extent, the read_extent_buffer
is now reading a len of @ram_bytes which is the len of the uncompressed
data. But actually here we want the len of the inline item.
So in the compressed situation, use the len of the inline item.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
cmds-restore.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/cmds-restore.c b/cmds-restore.c
index bb72311..e94592c 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -231,13 +231,15 @@ static int copy_one_inline(int fd, struct btrfs_path
*path, u64 pos)
unsigned long ptr;
int ret;
int len;
+ int inline_item_len;
int compress;
fi = btrfs_item_ptr(leaf, path-slots[0],
struct btrfs_file_extent_item);
ptr = btrfs_file_extent_inline_start(fi);
len = btrfs_file_extent_inline_len(leaf, path-slots[0], fi);
- read_extent_buffer(leaf, buf, ptr, len);
+ inline_item_len = btrfs_file_extent_inline_item_len(leaf,
btrfs_item_nr(path-slots[0]));
+ read_extent_buffer(leaf, buf, ptr, inline_item_len);
compress = btrfs_file_extent_compression(leaf, fi);
if (compress == BTRFS_COMPRESS_NONE) {
--
1.8.1.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: fs corruption report
On Mon, 2014-08-25 at 05:08 +, Zooko Wilcox-OHearn wrote: Dear people of linux-btrfs: Thank you for btrfs! It is a beautiful thing. I say that in spite of the fact that it seems to have failed and eaten some of my data. I'm writing with two purposes: to get help and advice in recovering my data, to help debug the software. I was running linux 3.12.26 and btrfsprogs 3.14, and I started getting error messages like these in my syslog: syslog.7:Aug 16 02:32:35 spark kernel: [48524.140611] btrfs no csum found for inode 15537898 start 4096 It happened only for one of the three partitions on this SSD, and smartctl indicated no problem with the disk: SMART overall-health self-assessment test result: PASSED … Num Test_DescriptionStatus Remaining LifeTime(hours) LBA_of_first_error # 1 Extended offlineCompleted without error 00% 6406 - # 2 Extended captiveCompleted without error 00% 6405 - I upgraded my kernel to 3.16.1 and tried the various techniques suggested in https://btrfs.wiki.kernel.org/index.php/Btrfsck and https://btrfs.wiki.kernel.org/index.php/Problem_FAQ , including `btrfsck check --repair --init-csum-tree`. This didn't fix it. I made an image of the filesystem in case someone wants to diagnose it (78 MB), and I also a made a dd copy of the affected partition. The `btrfs restore` command aborts even though I've passed the -i flag. In fact, I see that on subsequent runs it aborts at different places. Looking at the source code (http://git.kernel.org/cgit/linux/kernel/git/mason/btrfs-progs.git/tree/cmds-restore.c?id=c17d0a73c11d7cdbdf1582408ec6d168876160ea#n819) I don't see how -6 from decompress could cause it to stop when I have set `ignore_errors`, so next I ran it under valgrind. Aha. When it is run under valgrind it consistently stops (killing valgrind, in fact!) in the same way on every run. Here's the tail of stdout and stderr when it aborted when run under valgrind: Restoring ./sda6-btrfs-restore-3/@home/zooko/.mozilla/firefox/ltjwtkwe.ketotic.org/thumbnails/18af64f6d2871b0f24e325d8a298.png Restoring ./sda6-btrfs-restofailed to inflate: -6 Full valgrind outputs from such a run is attached to this letter. I've spent a little time looking at the stack traces in the valgrind log, and I *guess* that there is corruption such that the decompression fails, and I guess it would be possible to make cmds-restore handle corrupted compressedtext better, so that it would end up skipping whatever files and directories were unrestorable due to corruption. However, I don't immediately see how to proceed. Regards, Hi Zooko， Here are some pieces for your information: For the first: ==5569== Syscall param pwrite64(buf) points to uninitialised byte(s) ==5569==at 0x56ABD03: __pwrite_nocancel (syscall-template.S:81) ==5569==by 0x41F346: search_dir (cmds-restore.c:392) It is handled by https://patchwork.kernel.org/patch/4755441/ For the second: ==5569== Invalid read of size 1 ==5569==at 0x4C2F95E: memcpy@@GLIBC_2.14 ==5569==by 0x4388E6: read_extent_buffer (string3.h:51) ==5569==by 0x41ED6C: search_dir (cmds-restore.c:233) It should be handled by https://patchwork.kernel.org/patch/4792381/ And it handles Marc's similar problem too. And for the last one and the crucial one... ==5569== Invalid read of size 4 ==5569==at 0x41E394: decompress (cmds-restore.c:93) ==5569==by 0x41F291: search_dir (cmds-restore.c:378) along with ==5569== Invalid read of size 1 ==5569==at 0x548DDB6: lzo1x_decompress_safe ==5569==by 0x41E3BD: decompress (cmds-restore.c:122) ==5569==by 0x41F291: search_dir (cmds-restore.c:378) Sorry, I'm not able to reproduce it yet, it may be just what you've guessed that corruption happens. But I am sure that there are bugs around the decompress routine, because I've got failed to inflates too with a non-corrupted btrfs. I'm going to track it down. Thanks, -Gui Zooko Wilcox-O'Hearn Founder, CEO, and Customer Support Rep https://LeastAuthority.com Freedom matters. -- 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 corruption report
BTW，there is a develop branch from the btrfs-progs's maintainer David: http://github.com/kdave/btrfs-progs.git Maybe you'd like to try it, it may make some differences. -Gui On Mon, 2014-08-25 at 05:08 +, Zooko Wilcox-OHearn wrote: Dear people of linux-btrfs: Thank you for btrfs! It is a beautiful thing. I say that in spite of the fact that it seems to have failed and eaten some of my data. I'm writing with two purposes: to get help and advice in recovering my data, to help debug the software. I was running linux 3.12.26 and btrfsprogs 3.14, and I started getting error messages like these in my syslog: syslog.7:Aug 16 02:32:35 spark kernel: [48524.140611] btrfs no csum found for inode 15537898 start 4096 It happened only for one of the three partitions on this SSD, and smartctl indicated no problem with the disk: SMART overall-health self-assessment test result: PASSED … Num Test_DescriptionStatus Remaining LifeTime(hours) LBA_of_first_error # 1 Extended offlineCompleted without error 00% 6406 - # 2 Extended captiveCompleted without error 00% 6405 - I upgraded my kernel to 3.16.1 and tried the various techniques suggested in https://btrfs.wiki.kernel.org/index.php/Btrfsck and https://btrfs.wiki.kernel.org/index.php/Problem_FAQ , including `btrfsck check --repair --init-csum-tree`. This didn't fix it. I made an image of the filesystem in case someone wants to diagnose it (78 MB), and I also a made a dd copy of the affected partition. The `btrfs restore` command aborts even though I've passed the -i flag. In fact, I see that on subsequent runs it aborts at different places. Looking at the source code (http://git.kernel.org/cgit/linux/kernel/git/mason/btrfs-progs.git/tree/cmds-restore.c?id=c17d0a73c11d7cdbdf1582408ec6d168876160ea#n819) I don't see how -6 from decompress could cause it to stop when I have set `ignore_errors`, so next I ran it under valgrind. Aha. When it is run under valgrind it consistently stops (killing valgrind, in fact!) in the same way on every run. Here's the tail of stdout and stderr when it aborted when run under valgrind: Restoring ./sda6-btrfs-restore-3/@home/zooko/.mozilla/firefox/ltjwtkwe.ketotic.org/thumbnails/18af64f6d2871b0f24e325d8a298.png Restoring ./sda6-btrfs-restofailed to inflate: -6 Full valgrind outputs from such a run is attached to this letter. I've spent a little time looking at the stack traces in the valgrind log, and I *guess* that there is corruption such that the decompression fails, and I guess it would be possible to make cmds-restore handle corrupted compressedtext better, so that it would end up skipping whatever files and directories were unrestorable due to corruption. However, I don't immediately see how to proceed. Regards, Zooko Wilcox-O'Hearn Founder, CEO, and Customer Support Rep https://LeastAuthority.com Freedom matters. -- 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: btrfs restore memory corruption (bug: 82701)
On Mon, 2014-08-25 at 10:58 +0200, Marc Dietrich wrote: Am Freitag 22 August 2014, 10:42:18 schrieb Marc Dietrich: Am Freitag, 22. August 2014, 14:43:45 schrieb Gui Hecheng: On Thu, 2014-08-21 at 16:19 +0200, Marc Dietrich wrote: Am Donnerstag, 21. August 2014, 17:52:16 schrieb Gui Hecheng: On Mon, 2014-08-18 at 11:25 +0200, Marc Dietrich wrote: Hi, I did a checkout of the latest btrfs progs to repair my damaged filesystem. Running btrfs restore gives me several failed to inflate: -6 and crashes with some memory corruption. I ran it again with valgrind and got: valgrind --log-file=x2 -v --leak-check=yes btrfs restore /dev/sda9 /mnt/backup ==8528== Memcheck, a memory error detector ==8528== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al. ==8528== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info ==8528== Command: btrfs restore /dev/sda9 /mnt/backup ==8528== Parent PID: 8453 ==8528== ==8528== Syscall param pwrite64(buf) points to uninitialised byte(s) ==8528==at 0x59BE3C3: __pwrite_nocancel (in /lib64/libpthread-2.18.so) ==8528==by 0x41F22F: search_dir (cmds-restore.c:392) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x4204B8: cmd_restore (cmds-restore.c:1284) ==8528==by 0x4043FE: main (btrfs.c:286) ==8528== Address 0x66956a0 is 7,056 bytes inside a block of size 8,192 alloc'd ==8528==at 0x4C277AB: malloc (in /usr/lib64/valgrind/vgpreload_memcheck- amd64-linux.so) ==8528==by 0x41EEAD: search_dir (cmds-restore.c:316) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x4204B8: cmd_restore (cmds-restore.c:1284) ==8528==by 0x4043FE: main (btrfs.c:286) ---[snip]- leaks ... -- For the leak below... I've no idea why the @decompress_lzo() is not statisfied with @inbuf with the exact size of the disk bytes. Or maybe the compressed data had just sufferred damages... BTW, when you wrote your data, did that kernel has the following commit for btrfs? commit: 59516f6017c589e7316418fda6128ba8f829a77f mmh, I used the master branch which is still on 3.14.2 (from k.org). Ah, there is a development branch on another repo (repo.or.cz). Why oh why? Guy, sorry to quote an earlier mail, I forgot to add you as CC on you latest post and I'm not subscribed to the list. There is a development branch for btrfs-progs from david: http://github.com/kdave/btrfs-progs.git if you would like to try. ok, thanks will try. But here, what I mean is your *kernel* version when you wrote your data. I'm using btrfs since 3.14 or so (and maybe also some random distro kernel based on 3.11). The partition contained a lot of larger git trees and virtual machines - yes, not ideal for btrfs but a nice testcase ... There is a change for btrfs-restore which depends on a kernel commit. If you wrote your data with a older kernel and apply the 3.14.2 btrfs-progs to restore, then there may be wandering stuffs. wow. That should never happend I think. Userspace should always be able to fix corruptions made by earlier kernels (except disk layout changes maybe). Now, I am just suspecting such a scenario. Possbile. So how to proceed? If I checkout the latest brtfs from the repo above and restore again, are you still interested in the results? Ah, I think you could clone the progs from the repo and apply the two small pieces that I mentioned before. Yes, I am still trying to follow the issues with restore. It seems btrfs-restore needs more effect from btrfs developers since it doesn't survive tough scenarioes. It seems there are lots of people reporting corruptions on the list and also lots of fixes posted. Maybe it's better to restart from new (format a the partiton) and report problems happen after that. What do you think? Oh, I think you've just found a really good case for btrfs-restore. Maybe you could keep a image of that, just like Zooko did here: https://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg36701.html Thanks, -Gui Marc -- To unsubscribe from this list: send the line unsubscribe
Re: btrfs restore memory corruption (bug: 82701)
On Thu, 2014-08-21 at 16:19 +0200, Marc Dietrich wrote: Am Donnerstag, 21. August 2014, 17:52:16 schrieb Gui Hecheng: On Mon, 2014-08-18 at 11:25 +0200, Marc Dietrich wrote: Hi, I did a checkout of the latest btrfs progs to repair my damaged filesystem. Running btrfs restore gives me several failed to inflate: -6 and crashes with some memory corruption. I ran it again with valgrind and got: valgrind --log-file=x2 -v --leak-check=yes btrfs restore /dev/sda9 /mnt/backup ==8528== Memcheck, a memory error detector ==8528== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al. ==8528== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info ==8528== Command: btrfs restore /dev/sda9 /mnt/backup ==8528== Parent PID: 8453 ==8528== ==8528== Syscall param pwrite64(buf) points to uninitialised byte(s) ==8528==at 0x59BE3C3: __pwrite_nocancel (in /lib64/libpthread-2.18.so) ==8528==by 0x41F22F: search_dir (cmds-restore.c:392) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x4204B8: cmd_restore (cmds-restore.c:1284) ==8528==by 0x4043FE: main (btrfs.c:286) ==8528== Address 0x66956a0 is 7,056 bytes inside a block of size 8,192 alloc'd ==8528==at 0x4C277AB: malloc (in /usr/lib64/valgrind/vgpreload_memcheck- amd64-linux.so) ==8528==by 0x41EEAD: search_dir (cmds-restore.c:316) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x4204B8: cmd_restore (cmds-restore.c:1284) ==8528==by 0x4043FE: main (btrfs.c:286) ---[snip]- ==8528== Invalid read of size 1 ==8528==at 0x4C2BF15: memcpy@@GLIBC_2.14 (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==8528==by 0x43818F: read_extent_buffer (string3.h:51) ==8528==by 0x41EC66: search_dir (cmds-restore.c:233) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x4204B8: cmd_restore (cmds-restore.c:1284) ==8528==by 0x4043FE: main (btrfs.c:286) ==8528== Address 0x684c186 is 1,110 bytes inside a block of size 4,224 free'd ==8528==at 0x4C28ADC: free (in /usr/lib64/valgrind/vgpreload_memcheck- amd64-linux.so) ==8528==by 0x437895: free_extent_buffer (extent_io.c:618) ==8528==by 0x41E053: next_leaf (cmds-restore.c:202) ==8528==by 0x41E50F: search_dir (cmds-restore.c:731) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x4204B8: cmd_restore (cmds-restore.c:1284) ==8528==by 0x4043FE: main (btrfs.c:286) ==8528== ==8528== Invalid read of size 8 ==8528==at 0x4C2BF40: memcpy@@GLIBC_2.14 (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==8528==by 0x43818F: read_extent_buffer (string3.h:51) ==8528==by 0x41EC66: search_dir (cmds-restore.c:233) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x4204B8: cmd_restore (cmds-restore.c:1284) ==8528==by 0x4043FE: main (btrfs.c:286) ==8528== Address 0x684c178 is 1,096 bytes inside a block of size 4,224 free'd ==8528==at 0x4C28ADC: free (in /usr/lib64/valgrind/vgpreload_memcheck- amd64-linux.so) ==8528==by 0x437895: free_extent_buffer (extent_io.c:618) ==8528==by 0x41E053: next_leaf (cmds-restore.c:202) ==8528==by 0x41E50F: search_dir (cmds-restore.c:731) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x4204B8: cmd_restore (cmds
Re: btrfs restore memory corruption (bug: 82701)
On Fri, 2014-08-22 at 10:42 +0200, Marc Dietrich wrote:
Am Freitag, 22. August 2014, 14:43:45 schrieb Gui Hecheng:
On Thu, 2014-08-21 at 16:19 +0200, Marc Dietrich wrote:
Am Donnerstag, 21. August 2014, 17:52:16 schrieb Gui Hecheng:
On Mon, 2014-08-18 at 11:25 +0200, Marc Dietrich wrote:
Hi,
I did a checkout of the latest btrfs progs to repair my damaged
filesystem.
Running btrfs restore gives me several failed to inflate: -6 and
crashes
with some memory corruption. I ran it again with valgrind and got:
valgrind --log-file=x2 -v --leak-check=yes btrfs restore /dev/sda9
/mnt/backup
==8528== Memcheck, a memory error detector
==8528== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et
al.
==8528== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright
info
==8528== Command: btrfs restore /dev/sda9 /mnt/backup
==8528== Parent PID: 8453
==8528==
==8528== Syscall param pwrite64(buf) points to uninitialised byte(s)
==8528==at 0x59BE3C3: __pwrite_nocancel (in
/lib64/libpthread-2.18.so)
==8528==by 0x41F22F: search_dir (cmds-restore.c:392)
==8528==by 0x41F8D0: search_dir (cmds-restore.c:895)
==8528==by 0x41F8D0: search_dir (cmds-restore.c:895)
==8528==by 0x41F8D0: search_dir (cmds-restore.c:895)
==8528==by 0x41F8D0: search_dir (cmds-restore.c:895)
==8528==by 0x41F8D0: search_dir (cmds-restore.c:895)
==8528==by 0x4204B8: cmd_restore (cmds-restore.c:1284)
==8528==by 0x4043FE: main (btrfs.c:286)
==8528== Address 0x66956a0 is 7,056 bytes inside a block of size
8,192
alloc'd
==8528==at 0x4C277AB: malloc (in
/usr/lib64/valgrind/vgpreload_memcheck- amd64-linux.so)
==8528==by 0x41EEAD: search_dir (cmds-restore.c:316)
==8528==by 0x41F8D0: search_dir (cmds-restore.c:895)
==8528==by 0x41F8D0: search_dir (cmds-restore.c:895)
==8528==by 0x41F8D0: search_dir (cmds-restore.c:895)
==8528==by 0x41F8D0: search_dir (cmds-restore.c:895)
==8528==by 0x41F8D0: search_dir (cmds-restore.c:895)
==8528==by 0x4204B8: cmd_restore (cmds-restore.c:1284)
==8528==by 0x4043FE: main (btrfs.c:286)
---[snip]-
leaks ...
--
For the leak below...
I've no idea why the @decompress_lzo() is not statisfied with @inbuf
with the exact size of the disk bytes.
Or maybe the compressed data had just sufferred damages...
BTW, when you wrote your data, did that kernel has the following commit
for btrfs?
commit: 59516f6017c589e7316418fda6128ba8f829a77f
mmh, I used the master branch which is still on 3.14.2 (from k.org).
Ah, there is a development branch on another repo (repo.or.cz). Why oh why?
There is a development branch for btrfs-progs from david:
http://github.com/kdave/btrfs-progs.git if you would like to try.
But here, what I mean is your *kernel* version when you wrote your data.
There is a change for btrfs-restore which depends on a kernel commit.
If you wrote your data with a older kernel and apply the 3.14.2
btrfs-progs to restore, then there may be wandering stuffs.
Now, I am just suspecting such a scenario.
Thanks,
-Gui
If *NO*, then you may try the following and see if it makes any
difference:
-
diff --git a/cmds-restore.c b/cmds-restore.c
index dde7de8..ae1ea72 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -297,7 +297,7 @@ static int copy_one_extent(struct btrfs_root *root,
int fd,
ram_size = btrfs_file_extent_ram_bytes(leaf, fi);
offset = btrfs_file_extent_offset(leaf, fi);
num_bytes = btrfs_file_extent_num_bytes(leaf, fi);
- size_left = disk_size;
+ size_left = num_bytes;
if (compress == BTRFS_COMPRESS_NONE)
bytenr += offset;
@@ -376,7 +376,7 @@ again:
goto out;
}
- ret = decompress(inbuf, outbuf, disk_size, ram_size, compress);
+ ret = decompress(inbuf, outbuf, num_bytes, ram_size, compress);
if (ret) {
num_copies =
btrfs_num_copies(root-fs_info-mapping_tree,
bytenr, length);
*NOTE*: the above is just a trial, it is actually not proper, but please
don't worry, it does no harm.
well, my restore finished after 1 week (~ 400 GB of compressed data), from
which 100 GB got lost. It wasn't important data so I'm willing to redo the
complete restore again if you (or the the btrfs team) is interested in fixing
these bugs in the near future.
I will upload the latest valgrind log for the finial run to the bugzilla
Re: [PATCH] btrfs-progs: init uninitialized output buf for btrfs-restore
On Thu, 2014-08-21 at 10:14 +0200, Marc Dietrich wrote: Hi Gui, Am Donnerstag, 21. August 2014, 11:35:36 schrieb Gui Hecheng: A memory problem reported by valgrind as follows: === Syscall param pwrite64(buf) points to uninitialised byte(s) When running: # valgrind --leak-check=yes btrfs restore /dev/sda9 /mnt/backup Because the output buf size is alloced with malloc, but the length of output data is shorter than the sizeof(buf), so valgrind report uninitialised byte(s). We could use calloc to repalce malloc and clear this WARNING away. yes, the warning vanished. But the reads from free'd memory make me more worring... Ah, yeah, I am looking into it, hope that I can do some help :) Marc -- 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: btrfs restore memory corruption (bug: 82701)
On Mon, 2014-08-18 at 11:25 +0200, Marc Dietrich wrote: Hi, I did a checkout of the latest btrfs progs to repair my damaged filesystem. Running btrfs restore gives me several failed to inflate: -6 and crashes with some memory corruption. I ran it again with valgrind and got: valgrind --log-file=x2 -v --leak-check=yes btrfs restore /dev/sda9 /mnt/backup ==8528== Memcheck, a memory error detector ==8528== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al. ==8528== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info ==8528== Command: btrfs restore /dev/sda9 /mnt/backup ==8528== Parent PID: 8453 ==8528== ==8528== Syscall param pwrite64(buf) points to uninitialised byte(s) ==8528==at 0x59BE3C3: __pwrite_nocancel (in /lib64/libpthread-2.18.so) ==8528==by 0x41F22F: search_dir (cmds-restore.c:392) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x4204B8: cmd_restore (cmds-restore.c:1284) ==8528==by 0x4043FE: main (btrfs.c:286) ==8528== Address 0x66956a0 is 7,056 bytes inside a block of size 8,192 alloc'd ==8528==at 0x4C277AB: malloc (in /usr/lib64/valgrind/vgpreload_memcheck- amd64-linux.so) ==8528==by 0x41EEAD: search_dir (cmds-restore.c:316) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x4204B8: cmd_restore (cmds-restore.c:1284) ==8528==by 0x4043FE: main (btrfs.c:286) ---[snip]- ==8528== Invalid read of size 1 ==8528==at 0x4C2BF15: memcpy@@GLIBC_2.14 (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==8528==by 0x43818F: read_extent_buffer (string3.h:51) ==8528==by 0x41EC66: search_dir (cmds-restore.c:233) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x4204B8: cmd_restore (cmds-restore.c:1284) ==8528==by 0x4043FE: main (btrfs.c:286) ==8528== Address 0x684c186 is 1,110 bytes inside a block of size 4,224 free'd ==8528==at 0x4C28ADC: free (in /usr/lib64/valgrind/vgpreload_memcheck- amd64-linux.so) ==8528==by 0x437895: free_extent_buffer (extent_io.c:618) ==8528==by 0x41E053: next_leaf (cmds-restore.c:202) ==8528==by 0x41E50F: search_dir (cmds-restore.c:731) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x4204B8: cmd_restore (cmds-restore.c:1284) ==8528==by 0x4043FE: main (btrfs.c:286) ==8528== ==8528== Invalid read of size 8 ==8528==at 0x4C2BF40: memcpy@@GLIBC_2.14 (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==8528==by 0x43818F: read_extent_buffer (string3.h:51) ==8528==by 0x41EC66: search_dir (cmds-restore.c:233) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x4204B8: cmd_restore (cmds-restore.c:1284) ==8528==by 0x4043FE: main (btrfs.c:286) ==8528== Address 0x684c178 is 1,096 bytes inside a block of size 4,224 free'd ==8528==at 0x4C28ADC: free (in /usr/lib64/valgrind/vgpreload_memcheck- amd64-linux.so) ==8528==by 0x437895: free_extent_buffer (extent_io.c:618) ==8528==by 0x41E053: next_leaf (cmds-restore.c:202) ==8528==by 0x41E50F: search_dir (cmds-restore.c:731) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x4204B8: cmd_restore (cmds-restore.c:1284) ==8528==by 0x4043FE: main (btrfs.c:286) ==8528== ==8528== Invalid read of size 8 ==8528==at 0x4C2BF52: memcpy@@GLIBC_2.14 (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==8528==by 0x43818F: read_extent_buffer (string3.h:51) ==8528==by 0x41EC66:
[PATCH 1/2] btrfs-progs: cleanup duplicate assignment of variable leaf for btrfs-restore
The value of variable leaf in while loop don't have to be set
for every round. Just move it outside.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
cmds-restore.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/cmds-restore.c b/cmds-restore.c
index a6f535c..f417e0b 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -962,8 +962,9 @@ static int do_list_roots(struct btrfs_root *root)
return -1;
}
+ leaf = path-nodes[0];
+
while (1) {
- leaf = path-nodes[0];
slot = path-slots[0];
if (slot = btrfs_header_nritems(leaf)) {
ret = btrfs_next_leaf(root, path);
--
1.8.1.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
[PATCH 2/2] btrfs-progs: remove unused flags for btrfs_path
The three flags of @btrfs_path:
btrfs_path {
unsigned int keep_locks:1;
unsigned int skip_locking:1;
unsigned int leave_spinning:1;
}
have little meaning, because the userspace @btrfs_search_slot()
is free of locking and no other routines will decide their behavior
on these. So just remove them.
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
cmds-restore.c | 2 --
ctree.h| 3 ---
extent-tree.c | 6 --
3 files changed, 11 deletions(-)
diff --git a/cmds-restore.c b/cmds-restore.c
index f417e0b..bfd883d 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -566,7 +566,6 @@ static int copy_file(struct btrfs_root *root, int fd,
struct btrfs_key *key,
fprintf(stderr, Ran out of memory\n);
return -ENOMEM;
}
- path-skip_locking = 1;
ret = btrfs_lookup_inode(NULL, root, path, key, 0);
if (ret == 0) {
@@ -704,7 +703,6 @@ static int search_dir(struct btrfs_root *root, struct
btrfs_key *key,
fprintf(stderr, Ran out of memory\n);
return -ENOMEM;
}
- path-skip_locking = 1;
key-offset = 0;
key-type = BTRFS_DIR_INDEX_KEY;
diff --git a/ctree.h b/ctree.h
index 35d3633..fe1cd53 100644
--- a/ctree.h
+++ b/ctree.h
@@ -552,9 +552,6 @@ struct btrfs_path {
* and to force calls to keep space in the nodes
*/
unsigned int search_for_split:1;
- unsigned int keep_locks:1;
- unsigned int skip_locking:1;
- unsigned int leave_spinning:1;
unsigned int skip_check_block:1;
};
diff --git a/extent-tree.c b/extent-tree.c
index c46c92b..5443ec8 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -1418,7 +1418,6 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
return -ENOMEM;
path-reada = 1;
- path-leave_spinning = 1;
ret = insert_inline_extent_backref(trans, root-fs_info-extent_root,
path, bytenr, num_bytes, parent,
@@ -1440,7 +1439,6 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
btrfs_release_path(path);
path-reada = 1;
- path-leave_spinning = 1;
/* now insert the actual backref */
ret = insert_extent_backref(trans, root-fs_info-extent_root,
@@ -2195,7 +2193,6 @@ static int __free_extent(struct btrfs_trans_handle *trans,
return -ENOMEM;
path-reada = 1;
- path-leave_spinning = 1;
is_data = owner_objectid = BTRFS_FIRST_FREE_OBJECTID;
if (is_data)
@@ -2239,7 +2236,6 @@ static int __free_extent(struct btrfs_trans_handle *trans,
is_data);
BUG_ON(ret);
btrfs_release_path(path);
- path-leave_spinning = 1;
key.objectid = bytenr;
@@ -2304,7 +2300,6 @@ static int __free_extent(struct btrfs_trans_handle *trans,
BUG_ON(ret 0);
btrfs_release_path(path);
- path-leave_spinning = 1;
key.objectid = bytenr;
key.type = BTRFS_EXTENT_ITEM_KEY;
@@ -2711,7 +2706,6 @@ static int alloc_reserved_tree_block(struct
btrfs_trans_handle *trans,
path = btrfs_alloc_path();
BUG_ON(!path);
- path-leave_spinning = 1;
ret = btrfs_insert_empty_item(trans, fs_info-extent_root, path,
ins, size);
BUG_ON(ret);
--
1.8.1.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: btrfs restore memory corruption (bug: 82701)
On Mon, 2014-08-18 at 11:25 +0200, Marc Dietrich wrote: Hi, I did a checkout of the latest btrfs progs to repair my damaged filesystem. Running btrfs restore gives me several failed to inflate: -6 and crashes with some memory corruption. I ran it again with valgrind and got: valgrind --log-file=x2 -v --leak-check=yes btrfs restore /dev/sda9 /mnt/backup ==8528== Memcheck, a memory error detector ==8528== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al. ==8528== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info ==8528== Command: btrfs restore /dev/sda9 /mnt/backup ==8528== Parent PID: 8453 ==8528== ==8528== Syscall param pwrite64(buf) points to uninitialised byte(s) Hi, Marc For this one, It is because we use malloc to alloc space for buf which contains the stuff we are going to write out. But the actual length of the output stuff is shorter than sizeof(buf). To deal with this piece, I think use calloc instead of malloc will clear this WARNING away. I will send a patch for this. -Gui ==8528==at 0x59BE3C3: __pwrite_nocancel (in /lib64/libpthread-2.18.so) ==8528==by 0x41F22F: search_dir (cmds-restore.c:392) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x4204B8: cmd_restore (cmds-restore.c:1284) ==8528==by 0x4043FE: main (btrfs.c:286) ==8528== Address 0x66956a0 is 7,056 bytes inside a block of size 8,192 alloc'd ==8528==at 0x4C277AB: malloc (in /usr/lib64/valgrind/vgpreload_memcheck- amd64-linux.so) ==8528==by 0x41EEAD: search_dir (cmds-restore.c:316) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x4204B8: cmd_restore (cmds-restore.c:1284) ==8528==by 0x4043FE: main (btrfs.c:286) ==8528== ==8528== Invalid read of size 1 ==8528==at 0x4C2BF15: memcpy@@GLIBC_2.14 (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==8528==by 0x43818F: read_extent_buffer (string3.h:51) ==8528==by 0x41EC66: search_dir (cmds-restore.c:233) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x4204B8: cmd_restore (cmds-restore.c:1284) ==8528==by 0x4043FE: main (btrfs.c:286) ==8528== Address 0x684c186 is 1,110 bytes inside a block of size 4,224 free'd ==8528==at 0x4C28ADC: free (in /usr/lib64/valgrind/vgpreload_memcheck- amd64-linux.so) ==8528==by 0x437895: free_extent_buffer (extent_io.c:618) ==8528==by 0x41E053: next_leaf (cmds-restore.c:202) ==8528==by 0x41E50F: search_dir (cmds-restore.c:731) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x4204B8: cmd_restore (cmds-restore.c:1284) ==8528==by 0x4043FE: main (btrfs.c:286) ==8528== ==8528== Invalid read of size 8 ==8528==at 0x4C2BF40: memcpy@@GLIBC_2.14 (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==8528==by 0x43818F: read_extent_buffer (string3.h:51) ==8528==by 0x41EC66: search_dir (cmds-restore.c:233) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x4204B8: cmd_restore (cmds-restore.c:1284) ==8528==by 0x4043FE: main (btrfs.c:286) ==8528== Address 0x684c178 is 1,096 bytes inside a block of size 4,224 free'd ==8528==at 0x4C28ADC: free (in /usr/lib64/valgrind/vgpreload_memcheck- amd64-linux.so) ==8528==by 0x437895: free_extent_buffer (extent_io.c:618) ==8528==by 0x41E053: next_leaf (cmds-restore.c:202) ==8528==by 0x41E50F: search_dir (cmds-restore.c:731) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x41F8D0: search_dir (cmds-restore.c:895) ==8528==by 0x4204B8: cmd_restore (cmds-restore.c:1284) ==8528==
[PATCH] btrfs-progs: init uninitialized output buf for btrfs-restore
A memory problem reported by valgrind as follows:
=== Syscall param pwrite64(buf) points to uninitialised byte(s)
When running:
# valgrind --leak-check=yes btrfs restore /dev/sda9 /mnt/backup
Because the output buf size is alloced with malloc, but the length of
output data is shorter than the sizeof(buf), so valgrind report
uninitialised byte(s).
We could use calloc to repalce malloc and clear this WARNING away.
Reported-by: Marc Dietrich marvi...@gmx.de
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
cmds-restore.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/cmds-restore.c b/cmds-restore.c
index cbda6bb..bb72311 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -251,7 +251,7 @@ static int copy_one_inline(int fd, struct btrfs_path *path,
u64 pos)
}
ram_size = btrfs_file_extent_ram_bytes(leaf, fi);
- outbuf = malloc(ram_size);
+ outbuf = calloc(1, ram_size);
if (!outbuf) {
fprintf(stderr, No memory\n);
return -ENOMEM;
@@ -320,7 +320,7 @@ static int copy_one_extent(struct btrfs_root *root, int fd,
}
if (compress != BTRFS_COMPRESS_NONE) {
- outbuf = malloc(ram_size);
+ outbuf = calloc(1, ram_size);
if (!outbuf) {
fprintf(stderr, No memory\n);
free(inbuf);
--
1.8.1.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
[PATCH 1/3] btrfs-progs: check option conflict for btrfs-convert
The -d, -i, -n options make no sense to rollback.
Check the improper usages such as:
# btrfs-convert -r -d dev
Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
btrfs-convert.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/btrfs-convert.c b/btrfs-convert.c
index 952e3e6..2ea6a09 100644
--- a/btrfs-convert.c
+++ b/btrfs-convert.c
@@ -2699,6 +2699,7 @@ int main(int argc, char *argv[])
int noxattr = 0;
int datacsum = 1;
int rollback = 0;
+ int usage_error = 0;
char *file;
while(1) {
int c = getopt(argc, argv, dinr);
@@ -2729,6 +2730,18 @@ int main(int argc, char *argv[])
return 1;
}
+ if (rollback) {
+ if (!datacsum || noxattr || !packing) {
+ fprintf(stderr, Usage error: -d, -i, -n options do not
apply to rollback\n);
+ usage_error++;
+ }
+ }
+
+ if (usage_error) {
+ print_usage();
+ return 1;
+ }
+
file = argv[optind];
ret = check_mounted(file);
if (ret 0) {
--
1.8.1.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
