Re: [PATCH] Btrfs: update drop progress before stopping snapshot dropping
Thanks for posting that patch, Josef. On Mon, Jul 15, 2013 at 6:59 PM, Josef Bacik wrote: > > Alex pointed out a problem and fix that exists in the drop one snapshot at > a > time patch. If we decide we need to exit for whatever reason (umount for > example) we will just exit the snapshot dropping without updating the drop > progress. So the next time we go to resume we will BUG_ON() because we > can't > find the extent we left off at because we never updated it. This patch > fixes > the problem. > > Cc: sta...@vger.kernel.org > Reported-by: Alex Lyakas > Signed-off-by: Josef Bacik > --- > fs/btrfs/extent-tree.c | 14 -- > 1 files changed, 8 insertions(+), 6 deletions(-) > > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c > index bc00b24..8c204e1 100644 > --- a/fs/btrfs/extent-tree.c > +++ b/fs/btrfs/extent-tree.c > @@ -7584,11 +7584,6 @@ int btrfs_drop_snapshot(struct btrfs_root *root, > wc->reada_count = BTRFS_NODEPTRS_PER_BLOCK(root); > > while (1) { > - if (!for_reloc && btrfs_need_cleaner_sleep(root)) { > - pr_debug("btrfs: drop snapshot early exit\n"); > - err = -EAGAIN; > - goto out_end_trans; > - } > > ret = walk_down_tree(trans, root, path, wc); > if (ret < 0) { > @@ -7616,7 +7611,8 @@ int btrfs_drop_snapshot(struct btrfs_root *root, > } > > BUG_ON(wc->level == 0); > - if (btrfs_should_end_transaction(trans, tree_root)) { > + if (btrfs_should_end_transaction(trans, tree_root) || > + (!for_reloc && btrfs_need_cleaner_sleep(root))) { > ret = btrfs_update_root(trans, tree_root, > &root->root_key, > root_item); > @@ -7627,6 +7623,12 @@ int btrfs_drop_snapshot(struct btrfs_root *root, > } > > btrfs_end_transaction_throttle(trans, tree_root); > + if (!for_reloc && btrfs_need_cleaner_sleep(root)) > { > + pr_debug("btrfs: drop snapshot early > exit\n"); > + err = -EAGAIN; > + goto out_free; > + } > + > trans = btrfs_start_transaction(tree_root, 0); > if (IS_ERR(trans)) { > err = PTR_ERR(trans); > -- > 1.7.7.6 > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
btrfs zero divide (was: Re: Linux 3.10 problem reports (yes, plural))
On Tue, 30 Jul 2013, Thorsten Glaser wrote: > NEW problem: btrfs doesn’t work at all. I had to reboot my > buildd into 3.2 using echo s/u/s/o >/proc/sysrq-trigger as > the attempt to mount it left the system hanging there. > [0.00] Linux version 3.10-1-m68k (debian-ker...@lists.debian.org) > (gcc version 4.8.1 (Debian 4.8.1-7+m68k.1) ) #1 Debian 3.10.3-1 (2013-07-27) > [6.72] bio: create slab at 1 > [6.74] Btrfs loaded > [6.83] device label ara5-butter devid 1 transid 376178 /dev/nfhd8p3 > [7.15] EXT4-fs (nfhd8p1): mounted filesystem with ordered data mode. > Opts: (null) > [ 14.52] udevd[228]: starting version 175 > [ 17.82] device label ara5-butter devid 1 transid 376178 /dev/nfhd8p3 > [ 20.85] Adding 3670012k swap on /dev/nfhd8p2. Priority:-1 extents:1 > across:3670012k > [ 21.38] EXT4-fs (nfhd8p1): re-mounted. Opts: (null) > [ 31.30] EXT4-fs (nfhd8p1): re-mounted. Opts: errors=remount-ro > [ 38.46] device label ara5-butter devid 1 transid 376178 /dev/nfhd8p3 > [ 38.53] btrfs: setting nodatacow, compression disabled > [ 38.54] btrfs: enabling auto recovery > [ 38.57] btrfs: disk space caching is enabled > [ 38.60] *** ZERO DIVIDE *** FORMAT=2 > [ 38.63] Current process id is 722 > [ 38.66] BAD KERNEL TRAP: > [ 38.68] Modules linked in: evdev mac_hid ext4 crc16 jbd2 mbcache btrfs > xor lzo_compress zlib_deflate raid6_pq crc32c libcrc32c > [ 38.73] PC: [<319535b2>] __btrfs_map_block+0x11c/0x119a [btrfs] Woops, adding the btrfs devs to CC. > [ 38.77] SR: 2000 SP: 30c1fab4 a2: 30f0faf0 > [ 38.80] d0: d1: 1000d2: d3: > [ 38.83] d4: 0001d5: a0: 3085c72ca1: 3085c72c > [ 38.85] Process mount (pid: 722, task=30f0faf0) > [ 38.87] Frame format=2 instr addr=319535ae > [ 38.88] Stack from 30c1faec: > [ 38.88] 0020 1000 01401000 > 30253928 300ffc00 > [ 38.88] 00a843ac 3026f640 0001 0009e250 00d106c0 > 00011220 > [ 38.88] 1000 301c6830 0009e32a 00ff 0009 3085c72c > > [ 38.88] 30c1fd14 0020 30c1fd14 0009e26c > 0020 0003 > [ 38.88] 0009dd8a 300b0b6c 30253928 00a843ac 1000 > > [ 38.88] a008 3194e76a 30253928 00a843ac 1000 > 0002 > [ 39.19] Call Trace: [<1000>] kernel_pg_dir+0x0/0x1000 > [ 39.21] [<0001>] res_func+0x1020/0x141a > [ 39.25] [<0009e250>] bvec_alloc+0xa2/0xbe > [ 39.27] [<00011220>] sasin+0x87c/0x944 > [ 39.29] [<1000>] kernel_pg_dir+0x0/0x1000 > [ 39.33] [<0009e32a>] bio_alloc_bioset+0xbe/0x12e > [ 39.36] [<0009e26c>] bio_alloc_bioset+0x0/0x12e > [ 39.38] [<0009dd8a>] bio_add_page+0x4a/0x58 > [ 39.42] [<1000>] kernel_pg_dir+0x0/0x1000 > [ 39.47] [] via_nubus_irq+0x1c/0xa2 > [ 39.50] [<3194e76a>] submit_extent_page.isra.44+0x170/0x1bc [btrfs] > [ 39.53] [<1000>] kernel_pg_dir+0x0/0x1000 > [ 39.56] [<1000>] kernel_pg_dir+0x0/0x1000 > [ 39.60] [<31959778>] btrfs_map_bio+0x60/0x48c [btrfs] > [ 39.63] [<31931b72>] btree_submit_bio_hook+0x0/0xae [btrfs] > [ 39.66] [<3194eaa0>] end_bio_extent_readpage+0x0/0x69c [btrfs] > [ 39.71] [<1000>] kernel_pg_dir+0x0/0x1000 > [ 39.73] [<31931944>] btrfs_bio_wq_end_io+0x16/0x50 [btrfs] > [ 39.76] [<31931bce>] btree_submit_bio_hook+0x5c/0xae [btrfs] > [ 39.78] [<3194bd36>] submit_one_bio+0x7c/0xb2 [btrfs] > [ 39.81] [<3194f174>] __extent_read_full_page+0x0/0x70a [btrfs] > [ 39.83] [<00058828>] unlock_page+0x0/0x26 > [ 39.84] [<31951736>] read_extent_buffer_pages+0x1a8/0x218 [btrfs] > [ 39.89] [<00027d81>] devkmsg_read+0x213/0x39a > [ 39.93] [<31959006>] btrfs_num_copies+0x0/0x142 [btrfs] > [ 39.97] [<31930a66>] > btree_read_extent_buffer_pages.constprop.52+0x42/0xca [btrfs] > [ 40.03] [<3192f7c2>] btree_get_extent+0x0/0x102 [btrfs] > [ 40.06] [<00027d81>] devkmsg_read+0x213/0x39a > [ 40.09] [<1000>] kernel_pg_dir+0x0/0x1000 > [ 40.10] [<3193221e>] read_tree_block+0x38/0x48 [btrfs] > [ 40.13] [<00027d81>] devkmsg_read+0x213/0x39a > [ 40.14] [<319321e6>] read_tree_block+0x0/0x48 [btrfs] > [ 40.17] [<31933d00>] open_ctree+0xe80/0x15e6 [btrfs] > [ 40.20] [<1000>] kernel_pg_dir+0x0/0x1000 > [ 40.22] [<00027d81>] devkmsg_read+0x213/0x39a > [ 40.23] [<1000>] kernel_pg_dir+0x0/0x1000 > [ 40.26] [<000f280a>] resource_string.isra.12+0x2b4/0x2ee > [ 40.28] [<1000>] kernel_pg_dir+0x0/0x1000 > [ 40.32] [<1000>] kernel_pg_dir+0x0/0x1000 > [ 40.35] [<000e
Re: Cloning a Btrfs partition
On Mon, July 29, 2013 at 17:32 (+0200), BJ Quinn wrote: > Thanks for the response! Not sure I want to roll a custom kernel on this > particular system. Any idea on when it might make it to 3.10 stable or > 3.11? Or should I just revert back to 3.9? I missed that it's in fact in 3.11 and if I got Liu Bo right he's going to send it to 3.10 stable soon. Thanks, -Jan > Thanks! > > -BJ > > - Original Message - > > From: "Jan Schmidt" > Sent: Monday, July 29, 2013 3:21:51 AM > > Hi BJ, > > [original message rewrapped] > > On Thu, July 25, 2013 at 18:32 (+0200), BJ Quinn wrote: >> (Apologies for the double post -- forgot to send as plain text the first >> time >> around, so the list rejected it.) >> >> I see that there's now a btrfs send / receive and I've tried using it, but >> I'm getting the oops I've pasted below, after which the FS becomes >> unresponsive (no I/O to the drive, no CPU usage, but all attempts to access >> the FS results in a hang). I have an internal drive (single drive) that >> contains 82GB of compressed data with a couple hundred snapshots. I tried >> taking the first snapshot and making a read only copy (btrfs subvolume >> snapshot -r) and then I connected an external USB drive and ran btrfs send / >> receive to that external drive. It starts working and gets a couple of GB in >> (I'd expect the first snapshot to be about 20GB) and then gets the following >> error. I had to use the latest copy of btrfs-progs from git, because the >> package installed on my system (btrfs-progs-0.20-0.2.git91d9eec) simply >> returned "invalid argument" when trying to run btrfs send / receive. Thanks >> in advance for any info you may have. > > The problem has been introduced with rbtree ulists in 3.10, commit > > Btrfs: add a rb_tree to improve performance of ulist search > > You should be safe to revert that commit, it's a performance optimization > attempt. Alternatively, you can apply the published fix > > Btrfs: fix crash regarding to ulist_add_merge > > It has not made it into 3.10 stable or 3.11, yet, but is contained in Josef's > btrfs-next > > git://git.kernel.org/pub/scm/linux/kernel/git/josef/btrfs-next.git > > Thanks, > -Jan > -- > 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: optimize function btrfs_read_chunk_tree
After reading all device items from the chunk tree, don't exit the loop and then navigate down the tree again to find the chunk items. Instead just read all device items and chunk items with a single tree search. This is possible because all device items are found before any chunk item in the chunks tree. Signed-off-by: Filipe David Borba Manana --- V2: Simplified logic inside the loop (suggested by Josef Bacik on irc). V3: Updated comment to comply with kernel coding style. fs/btrfs/volumes.c | 30 +++--- 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 090f57c..125a60e 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -5676,14 +5676,15 @@ int btrfs_read_chunk_tree(struct btrfs_root *root) mutex_lock(&uuid_mutex); lock_chunks(root); - /* first we search for all of the device items, and then we -* read in all of the chunk items. This way we can create chunk -* mappings that reference all of the devices that are afound + /* +* Read all device items, and then all the chunk items. All +* device items are found before any chunk item (their object id +* is smaller than the lowest possible object id for a chunk +* item - BTRFS_FIRST_CHUNK_TREE_OBJECTID). */ key.objectid = BTRFS_DEV_ITEMS_OBJECTID; key.offset = 0; key.type = 0; -again: ret = btrfs_search_slot(NULL, root, &key, path, 0, 0); if (ret < 0) goto error; @@ -5699,17 +5700,13 @@ again: break; } btrfs_item_key_to_cpu(leaf, &found_key, slot); - if (key.objectid == BTRFS_DEV_ITEMS_OBJECTID) { - if (found_key.objectid != BTRFS_DEV_ITEMS_OBJECTID) - break; - if (found_key.type == BTRFS_DEV_ITEM_KEY) { - struct btrfs_dev_item *dev_item; - dev_item = btrfs_item_ptr(leaf, slot, + if (found_key.type == BTRFS_DEV_ITEM_KEY) { + struct btrfs_dev_item *dev_item; + dev_item = btrfs_item_ptr(leaf, slot, struct btrfs_dev_item); - ret = read_one_dev(root, leaf, dev_item); - if (ret) - goto error; - } + ret = read_one_dev(root, leaf, dev_item); + if (ret) + goto error; } else if (found_key.type == BTRFS_CHUNK_ITEM_KEY) { struct btrfs_chunk *chunk; chunk = btrfs_item_ptr(leaf, slot, struct btrfs_chunk); @@ -5719,11 +5716,6 @@ again: } path->slots[0]++; } - if (key.objectid == BTRFS_DEV_ITEMS_OBJECTID) { - key.objectid = 0; - btrfs_release_path(path); - goto again; - } ret = 0; error: unlock_chunks(root); -- 1.7.9.5 -- 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: optimize function btrfs_read_chunk_tree
After reading all device items from the chunk tree, don't exit the loop and then navigate down the tree again to find the chunk items. Instead just read all device items and chunk items with a single tree search. This is possible because all device items are found before any chunk item in the chunks tree. This is a port of the corresponding kernel patch to keep both kernel and btrfs-progs identical: https://patchwork.kernel.org/patch/2835529/ Signed-off-by: Filipe David Borba Manana --- V2: Simplified logic inside the loop (suggested by Josef Bacik on irc). V3: Updated comment to comply with kernel coding style. volumes.c | 28 ++-- 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/volumes.c b/volumes.c index 0ff2283..0cb3505 100644 --- a/volumes.c +++ b/volumes.c @@ -1718,14 +1718,15 @@ int btrfs_read_chunk_tree(struct btrfs_root *root) if (!path) return -ENOMEM; - /* first we search for all of the device items, and then we -* read in all of the chunk items. This way we can create chunk -* mappings that reference all of the devices that are afound + /* +* Read all device items, and then all the chunk items. All +* device items are found before any chunk item (their object id +* is smaller than the lowest possible object id for a chunk +* item - BTRFS_FIRST_CHUNK_TREE_OBJECTID). */ key.objectid = BTRFS_DEV_ITEMS_OBJECTID; key.offset = 0; key.type = 0; -again: ret = btrfs_search_slot(NULL, root, &key, path, 0, 0); while(1) { leaf = path->nodes[0]; @@ -1739,16 +1740,12 @@ again: break; } btrfs_item_key_to_cpu(leaf, &found_key, slot); - if (key.objectid == BTRFS_DEV_ITEMS_OBJECTID) { - if (found_key.objectid != BTRFS_DEV_ITEMS_OBJECTID) - break; - if (found_key.type == BTRFS_DEV_ITEM_KEY) { - struct btrfs_dev_item *dev_item; - dev_item = btrfs_item_ptr(leaf, slot, + if (found_key.type == BTRFS_DEV_ITEM_KEY) { + struct btrfs_dev_item *dev_item; + dev_item = btrfs_item_ptr(leaf, slot, struct btrfs_dev_item); - ret = read_one_dev(root, leaf, dev_item); - BUG_ON(ret); - } + ret = read_one_dev(root, leaf, dev_item); + BUG_ON(ret); } else if (found_key.type == BTRFS_CHUNK_ITEM_KEY) { struct btrfs_chunk *chunk; chunk = btrfs_item_ptr(leaf, slot, struct btrfs_chunk); @@ -1757,11 +1754,6 @@ again: } path->slots[0]++; } - if (key.objectid == BTRFS_DEV_ITEMS_OBJECTID) { - key.objectid = 0; - btrfs_release_path(root, path); - goto again; - } ret = 0; error: -- 1.7.9.5 -- 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: add missing path alloc return value check
Also remove unused path in extent-tree.c:finish_current_insert(). Signed-off-by: Filipe David Borba Manana --- V2: added 1 more path alloc check and removed unnecessary path allocation in extent-tree.c:finish_current_insert(). V3: added missing path alloc checks to dir-item.c, file-item.c and btrfs-corrupt-block.c too. V4: added missing patch alloc checks to cmd-checks.c and root-tree.c. btrfs-corrupt-block.c |2 ++ cmds-check.c |2 ++ dir-item.c|2 ++ extent-tree.c |8 file-item.c |2 ++ root-tree.c |2 ++ 6 files changed, 14 insertions(+), 4 deletions(-) diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c index 8176fad..22facd4 100644 --- a/btrfs-corrupt-block.c +++ b/btrfs-corrupt-block.c @@ -159,6 +159,8 @@ static int corrupt_extent(struct btrfs_trans_handle *trans, int should_del = rand() % 3; path = btrfs_alloc_path(); + if (!path) + return -ENOMEM; key.objectid = bytenr; key.type = (u8)-1; diff --git a/cmds-check.c b/cmds-check.c index 8015288..c1a0df9 100644 --- a/cmds-check.c +++ b/cmds-check.c @@ -4561,6 +4561,8 @@ static int fixup_extent_refs(struct btrfs_trans_handle *trans, flags = BTRFS_BLOCK_FLAG_FULL_BACKREF; path = btrfs_alloc_path(); + if (!path) + return -ENOMEM; /* step one, make sure all of the backrefs agree */ ret = verify_backrefs(trans, info, path, rec); diff --git a/dir-item.c b/dir-item.c index f00485a..0ab3c5e 100644 --- a/dir-item.c +++ b/dir-item.c @@ -123,6 +123,8 @@ int btrfs_insert_dir_item(struct btrfs_trans_handle *trans, struct btrfs_root btrfs_set_key_type(&key, BTRFS_DIR_ITEM_KEY); key.offset = btrfs_name_hash(name, name_len); path = btrfs_alloc_path(); + if (!path) + return -ENOMEM; data_size = sizeof(*dir_item) + name_len; dir_item = insert_with_overflow(trans, root, path, &key, data_size, name, name_len); diff --git a/extent-tree.c b/extent-tree.c index f597e16..e4adaa3 100644 --- a/extent-tree.c +++ b/extent-tree.c @@ -1487,6 +1487,8 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans, } path = btrfs_alloc_path(); + if (!path) + return -ENOMEM; path->reada = 1; key.objectid = bytenr; @@ -1577,6 +1579,8 @@ int btrfs_set_block_flags(struct btrfs_trans_handle *trans, BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA); path = btrfs_alloc_path(); + if (!path) + return -ENOMEM; path->reada = 1; key.objectid = bytenr; @@ -2078,7 +2082,6 @@ static int finish_current_insert(struct btrfs_trans_handle *trans, u64 end; u64 priv; struct btrfs_fs_info *info = extent_root->fs_info; - struct btrfs_path *path; struct pending_extent_op *extent_op; struct btrfs_key key; int ret; @@ -2086,8 +2089,6 @@ static int finish_current_insert(struct btrfs_trans_handle *trans, btrfs_fs_incompat(extent_root->fs_info, BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA); - path = btrfs_alloc_path(); - while(1) { ret = find_first_extent_bit(&info->extent_ins, 0, &start, &end, EXTENT_LOCKED); @@ -2121,7 +2122,6 @@ static int finish_current_insert(struct btrfs_trans_handle *trans, GFP_NOFS); kfree(extent_op); } - btrfs_free_path(path); return 0; } diff --git a/file-item.c b/file-item.c index 9c787f0..82bf99e 100644 --- a/file-item.c +++ b/file-item.c @@ -417,6 +417,8 @@ int btrfs_del_csums(struct btrfs_trans_handle *trans, root = root->fs_info->csum_root; path = btrfs_alloc_path(); + if (!path) + return -ENOMEM; while (1) { key.objectid = BTRFS_EXTENT_CSUM_OBJECTID; diff --git a/root-tree.c b/root-tree.c index ba380bd..efcdb7b 100644 --- a/root-tree.c +++ b/root-tree.c @@ -264,6 +264,8 @@ int btrfs_add_root_ref(struct btrfs_trans_handle *trans, path = btrfs_alloc_path(); + if (!path) + return -ENOMEM; key.objectid = root_id; key.type = type; -- 1.7.9.5 -- 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 qgroup assign -> "ERROR: bad relation requested"
On Sun, 28 Jul 2013 15:36:41 +0800 Wang Shilong wrote: > Btrfs group assign requires parent's level > children's level, For > your example below, you can do like: > > btrfs qgroup create 1/1 > btrfs qgroup assign 1177 1/1 > btrfs qgroup assign 1178 1/1 Cool, thanks. And is it possible (i.e. at some later point) to verify that 1177, 1178 were assigned to 1/1 (or in general, what is assigned to 1/1)? -- Tomasz Chmielewski http://wpkg.org -- 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 qgroup assign -> "ERROR: bad relation requested"
Hello, > On Sun, 28 Jul 2013 15:36:41 +0800 > Wang Shilong wrote: > >> Btrfs group assign requires parent's level > children's level, For >> your example below, you can do like: >> >> btrfs qgroup create 1/1 >> btrfs qgroup assign 1177 1/1 >> btrfs qgroup assign 1178 1/1 > > Cool, thanks. > > And is it possible (i.e. at some later point) to verify that 1177, 1178 > were assigned to 1/1 (or in general, what is assigned to 1/1)? I have implemented the above function, but they haven't gone into upstream Btrfs-progs. You can try this: http://github.com/miaoxie/btrfs-progs.git qgroup Maybe i need rebase the patch-set later, But you can try it firstly, if you want to see every qgroup's parent, you can add option '-p': btrfs qgroup show -p If you want to see what is assigned to 1/1, you can use: btrfs group show -c Further, if you want to see a specified path's parent qgroup, you can use it: btrfs group show -F/-f Also '-l' and '-e' print limit size of qgroup. Thanks, Wang > > > -- > Tomasz Chmielewski > http://wpkg.org > -- 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: Recovering from btrfs error Couldn't read chunk root.
Hello, > Unrecoverable? > I know i cant mount and have access but my data are still there intact > ( as i was using them till the reboot) i shouldn't be able to > extract/recover them to another disc? With any magic command without > mounting? > Any other solutions? As We Know chunk Tree really plays a vital role in btrfs filesystem, And it seems your chunk Tree is totally damaged, And it is ok to recover chunk tree if block groups are ok, Unfortunately, dd seems to destroy some of them. Maybe you can refers to commands btrfs-restore's help, i guess it doesn't work either. Thank, Wang > > http://bpaste.net/show/118112/ > > On 29 July 2013 15:06, Wang Shilong wrote: >> >> 在 2013-7-29,上午2:12,Kyriakos 写道: >> >>> Just tried it as you said with the -v option enabled >>> This is my output: >>> >>> http://bpaste.net/show/118112/ >>> >>> >> This is a *long* email, and seems that btrfs list refuse it. >> >>> Device extent: devid = 1, start = 1667558801408, len = 1073741824, >>> chunk offset = 1663255445504 >>> Couldn't map the block 626309926912 >>> btrfs: volumes.c:1020: btrfs_num_copies: Assertion `!(ce->start > >>> logical || ce->start + ce->size < logical)' failed. >>> Aborted (core dumped) >> >> Strange enough, we don't find any chunks during scanning process. >> >> And seems this is unrecoverable ~_~ >> >> >> Wang, >>> >>> >>> Any thoughts? >>> >>> On 28 July 2013 08:17, Wang Shilong wrote: Hello, It seems Btrfs Chunk Tree is damaged, so you can not mount Btrfs filesystem any more. However, you can try the latest Btrfs-progs, Miao Xie implements chunk tree recover function. The url is: git://git.kernel.org/pub/scm/linux/kernel/git/mason/btrfs-progs.git you can try it: btrfs chunk-recover -v This is Time-consuming, because it will scan the whole disk. And also, please catch output of processing(this is helpful to us if the recovery fails, -v option enable this). Thanks, Wang >> -- 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 all callers of read_tree_block
Hi Josef, On Tue, Apr 23, 2013 at 9:20 PM, Josef Bacik wrote: > We kept leaking extent buffers when mounting a broken file system and it turns > out it's because not everybody uses read_tree_block properly. You need to > check > and make sure the extent_buffer is uptodate before you use it. This patch > fixes > everybody who calls read_tree_block directly to make sure they check that it > is > uptodate and free it and return an error if it is not. With this we no longer > leak EB's when things go horribly wrong. Thanks, > > Signed-off-by: Josef Bacik > --- > fs/btrfs/backref.c | 10 -- > fs/btrfs/ctree.c | 21 - > fs/btrfs/disk-io.c | 19 +-- > fs/btrfs/extent-tree.c |4 +++- > fs/btrfs/relocation.c | 18 +++--- > 5 files changed, 59 insertions(+), 13 deletions(-) > > diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c > index 23e927b..04b5b30 100644 > --- a/fs/btrfs/backref.c > +++ b/fs/btrfs/backref.c > @@ -423,7 +423,10 @@ static int __add_missing_keys(struct btrfs_fs_info > *fs_info, > BUG_ON(!ref->wanted_disk_byte); > eb = read_tree_block(fs_info->tree_root, > ref->wanted_disk_byte, > fs_info->tree_root->leafsize, 0); > - BUG_ON(!eb); > + if (!eb || !extent_buffer_uptodate(eb)) { > + free_extent_buffer(eb); > + return -EIO; > + } > btrfs_tree_read_lock(eb); > if (btrfs_header_level(eb) == 0) > btrfs_item_key_to_cpu(eb, &ref->key_for_search, 0); > @@ -913,7 +916,10 @@ again: > info_level); > eb = read_tree_block(fs_info->extent_root, >ref->parent, bsz, > 0); > - BUG_ON(!eb); > + if (!eb || !extent_buffer_uptodate(eb)) { > + free_extent_buffer(eb); > + return -EIO; > + } > ret = find_extent_in_eb(eb, bytenr, > *extent_item_pos, > &eie); > ref->inode_list = eie; > diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c > index 566d99b..2bc3440 100644 > --- a/fs/btrfs/ctree.c > +++ b/fs/btrfs/ctree.c > @@ -1281,7 +1281,8 @@ get_old_root(struct btrfs_root *root, u64 time_seq) > free_extent_buffer(eb_root); > blocksize = btrfs_level_size(root, old_root->level); > old = read_tree_block(root, logical, blocksize, 0); > - if (!old) { > + if (!old || !extent_buffer_uptodate(old)) { > + free_extent_buffer(old); > pr_warn("btrfs: failed to read tree block %llu from > get_old_root\n", > logical); > WARN_ON(1); > @@ -1526,8 +1527,10 @@ int btrfs_realloc_node(struct btrfs_trans_handle > *trans, > if (!cur) { > cur = read_tree_block(root, blocknr, > blocksize, gen); > - if (!cur) > + if (!cur || !extent_buffer_uptodate(cur)) { > + free_extent_buffer(cur); > return -EIO; > + } > } else if (!uptodate) { > err = btrfs_read_buffer(cur, gen); > if (err) { > @@ -1692,6 +1695,8 @@ static noinline struct extent_buffer > *read_node_slot(struct btrfs_root *root, >struct extent_buffer *parent, int slot) > { > int level = btrfs_header_level(parent); > + struct extent_buffer *eb; > + > if (slot < 0) > return NULL; > if (slot >= btrfs_header_nritems(parent)) > @@ -1699,9 +1704,15 @@ static noinline struct extent_buffer > *read_node_slot(struct btrfs_root *root, > > BUG_ON(level == 0); > > - return read_tree_block(root, btrfs_node_blockptr(parent, slot), > - btrfs_level_size(root, level - 1), > - btrfs_node_ptr_generation(parent, slot)); > + eb = read_tree_block(root, btrfs_node_blockptr(parent, slot), > +btrfs_level_size(root, level - 1), > +btrfs_node_ptr_generation(parent, slot)); > + if (eb && !extent_buffer_uptodate(eb)) { > + free_extent_buffer(eb); > + eb = NULL; > + } > + > + return eb; > } > > /* > diff --git a/fs/bt
Re: [PATCH] Btrfs: fix all callers of read_tree_block
On Tue, Jul 30, 2013 at 06:11:39PM +0300, Alex Lyakas wrote: > Hi Josef, > > On Tue, Apr 23, 2013 at 9:20 PM, Josef Bacik wrote: > > We kept leaking extent buffers when mounting a broken file system and it > > turns > > out it's because not everybody uses read_tree_block properly. You need to > > check > > and make sure the extent_buffer is uptodate before you use it. This patch > > fixes > > everybody who calls read_tree_block directly to make sure they check that > > it is > > uptodate and free it and return an error if it is not. With this we no > > longer > > leak EB's when things go horribly wrong. Thanks, > > > > Signed-off-by: Josef Bacik > > --- > > fs/btrfs/backref.c | 10 -- > > fs/btrfs/ctree.c | 21 - > > fs/btrfs/disk-io.c | 19 +-- > > fs/btrfs/extent-tree.c |4 +++- > > fs/btrfs/relocation.c | 18 +++--- > > 5 files changed, 59 insertions(+), 13 deletions(-) > > > > diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c > > index 23e927b..04b5b30 100644 > > --- a/fs/btrfs/backref.c > > +++ b/fs/btrfs/backref.c > > @@ -423,7 +423,10 @@ static int __add_missing_keys(struct btrfs_fs_info > > *fs_info, > > BUG_ON(!ref->wanted_disk_byte); > > eb = read_tree_block(fs_info->tree_root, > > ref->wanted_disk_byte, > > fs_info->tree_root->leafsize, 0); > > - BUG_ON(!eb); > > + if (!eb || !extent_buffer_uptodate(eb)) { > > + free_extent_buffer(eb); > > + return -EIO; > > + } > > btrfs_tree_read_lock(eb); > > if (btrfs_header_level(eb) == 0) > > btrfs_item_key_to_cpu(eb, &ref->key_for_search, 0); > > @@ -913,7 +916,10 @@ again: > > info_level); > > eb = read_tree_block(fs_info->extent_root, > >ref->parent, > > bsz, 0); > > - BUG_ON(!eb); > > + if (!eb || !extent_buffer_uptodate(eb)) { > > + free_extent_buffer(eb); > > + return -EIO; > > + } > > ret = find_extent_in_eb(eb, bytenr, > > *extent_item_pos, > > &eie); > > ref->inode_list = eie; > > diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c > > index 566d99b..2bc3440 100644 > > --- a/fs/btrfs/ctree.c > > +++ b/fs/btrfs/ctree.c > > @@ -1281,7 +1281,8 @@ get_old_root(struct btrfs_root *root, u64 time_seq) > > free_extent_buffer(eb_root); > > blocksize = btrfs_level_size(root, old_root->level); > > old = read_tree_block(root, logical, blocksize, 0); > > - if (!old) { > > + if (!old || !extent_buffer_uptodate(old)) { > > + free_extent_buffer(old); > > pr_warn("btrfs: failed to read tree block %llu from > > get_old_root\n", > > logical); > > WARN_ON(1); > > @@ -1526,8 +1527,10 @@ int btrfs_realloc_node(struct btrfs_trans_handle > > *trans, > > if (!cur) { > > cur = read_tree_block(root, blocknr, > > blocksize, gen); > > - if (!cur) > > + if (!cur || !extent_buffer_uptodate(cur)) { > > + free_extent_buffer(cur); > > return -EIO; > > + } > > } else if (!uptodate) { > > err = btrfs_read_buffer(cur, gen); > > if (err) { > > @@ -1692,6 +1695,8 @@ static noinline struct extent_buffer > > *read_node_slot(struct btrfs_root *root, > >struct extent_buffer *parent, int slot) > > { > > int level = btrfs_header_level(parent); > > + struct extent_buffer *eb; > > + > > if (slot < 0) > > return NULL; > > if (slot >= btrfs_header_nritems(parent)) > > @@ -1699,9 +1704,15 @@ static noinline struct extent_buffer > > *read_node_slot(struct btrfs_root *root, > > > > BUG_ON(level == 0); > > > > - return read_tree_block(root, btrfs_node_blockptr(parent, slot), > > - btrfs_level_size(root, level - 1), > > - btrfs_node_ptr_generation(parent, slot)); > > + eb = read_tree_block(root, btrfs_node_blockptr(parent, slot), > > +btrfs_level_size(root, le
Re: btrfs zero divide
Geert Uytterhoeven dixit: > 0: 222e ff74 movel %fp@(-140),%d1 > 4: 2a2e ff5c movel %fp@(-164),%d5 > 8: 2c2e ff60 movel %fp@(-160),%d6 > c: 4c45 1402 < divul %d5,%d2,%d1 > > 10: 2d40 ff64 movel %d0,%fp@(-156) > 14: 2d41 ff68 movel %d1,%fp@(-152) > 18: 2205movel %d5,%d1 > 1a: 4c2e 1800 ff68 mulsl %fp@(-152),%d1 > 20: 4c04 0800 mulsl %d4,%d0 > 24: 2041moveal %d1,%a0 > 26: d1c0addal %d0,%a0 > 28: 2206movel %d6,%d1 > 2a: 4c2e 1400 ff68 mulul %fp@(-152),%d0,%d1 This is gcc-4.8 compiled, btw… in case that’s a known issue. bye, //mirabilos -- “It is inappropriate to require that a time represented as seconds since the Epoch precisely represent the number of seconds between the referenced time and the Epoch.” -- IEEE Std 1003.1b-1993 (POSIX) Section B.2.2.2 -- 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 zero divide (was: Re: Linux 3.10 problem reports (yes, plural))
On Tue, Jul 30, 2013 at 11:07:30AM +0200, Geert Uytterhoeven wrote: > On Tue, 30 Jul 2013, Thorsten Glaser wrote: > > NEW problem: btrfs doesn’t work at all. I had to reboot my > > buildd into 3.2 using echo s/u/s/o >/proc/sysrq-trigger as > > the attempt to mount it left the system hanging there. > > > [0.00] Linux version 3.10-1-m68k (debian-ker...@lists.debian.org) > > (gcc version 4.8.1 (Debian 4.8.1-7+m68k.1) ) #1 Debian 3.10.3-1 (2013-07-27) > > > [6.72] bio: create slab at 1 > > [6.74] Btrfs loaded > > [6.83] device label ara5-butter devid 1 transid 376178 /dev/nfhd8p3 > > [7.15] EXT4-fs (nfhd8p1): mounted filesystem with ordered data > > mode. Opts: (null) > > [ 14.52] udevd[228]: starting version 175 > > [ 17.82] device label ara5-butter devid 1 transid 376178 /dev/nfhd8p3 > > [ 20.85] Adding 3670012k swap on /dev/nfhd8p2. Priority:-1 extents:1 > > across:3670012k > > [ 21.38] EXT4-fs (nfhd8p1): re-mounted. Opts: (null) > > [ 31.30] EXT4-fs (nfhd8p1): re-mounted. Opts: errors=remount-ro > > [ 38.46] device label ara5-butter devid 1 transid 376178 /dev/nfhd8p3 > > [ 38.53] btrfs: setting nodatacow, compression disabled > > [ 38.54] btrfs: enabling auto recovery > > [ 38.57] btrfs: disk space caching is enabled > > [ 38.60] *** ZERO DIVIDE *** FORMAT=2 > > [ 38.63] Current process id is 722 > > [ 38.66] BAD KERNEL TRAP: > > [ 38.68] Modules linked in: evdev mac_hid ext4 crc16 jbd2 mbcache > > btrfs xor lzo_compress zlib_deflate raid6_pq crc32c libcrc32c > > [ 38.73] PC: [<319535b2>] __btrfs_map_block+0x11c/0x119a [btrfs] > > Woops, adding the btrfs devs to CC. > Can you gdb btrfs.ko and do list *(__btrfs_map_block+0x11c) so I can see where this is? I've not seen this yet, just so I'm clear this is blowing up because we're doing blah / 0 right? I've looked at all the places we do divides in this function and it doesn't look like we're doing this anywhere but I could be blind. Thanks, Josef -- 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 zero divide (was: Re: Linux 3.10 problem reports (yes, plural))
On Tue, 2013-07-30 at 13:13 -0400, Josef Bacik wrote: > I've looked at all the places we do divides in this function and it > doesn't look like we're doing this anywhere but I could be blind, do_div seems a likely suspect... /* * stripe_nr counts the total number of stripes we have to stride * to get to this block */ do_div(stripe_nr, stripe_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
Fix leak in __btrfs_map_block error path
If we bail out when the stripe alloc fails, we need to undo the earlier allocation of raid_map. Signed-off-by: Dave Jones diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 78b8717..6a0f52f 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -4671,6 +4671,7 @@ static int __btrfs_map_block(struct btrfs_fs_info *fs_info, int rw, } bbio = kzalloc(btrfs_bio_size(num_alloc_stripes), GFP_NOFS); if (!bbio) { + kfree(raid_map); ret = -ENOMEM; goto out; } -- 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] xfstests: generic/315: allow a little tolerance for our used check
On Tue, Jul 30, 2013 at 11:48:10AM +0800, Jeff Liu wrote: > On 07/30/2013 02:55 AM, Josef Bacik wrote: > > > So df in btrfs is tricky at best, and relying on it for accurate > > information is > > not great, but it's the best way to verify this test. To get around btrfs > > being > > inconsistent sometimes just use _within_tolerance to check our new df value > > to > > make sure that our truncate did something. With this patch I no longer see > > transient failures of this test. Thanks, > > > > Signed-off-by: Josef Bacik > > --- > > tests/generic/315 |3 ++- > > 1 files changed, 2 insertions(+), 1 deletions(-) > > > > diff --git a/tests/generic/315 b/tests/generic/315 > > index 7cfc40d..9c01b5e 100644 > > --- a/tests/generic/315 > > +++ b/tests/generic/315 > > @@ -73,7 +73,8 @@ sync > > > > # Preallocated disk space should be released > > avail_done=`df -P $TEST_DIR | awk 'END {print $4}'` > > -[ "$avail_done" -eq "$avail_begin" ] || _fail "Available disk space > > ($avail_done KiB)" > > +_within_tolerance "df" $avail_done $avail_begin 1% > > +[ $? -eq 0 ] || _fail "Available disk space ($avail_done KiB) wanted > > ($avail_begin KiB)" > > Looks good to me. > Reviewed-by: Jie Liu Applied. -- 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: add missing error check to find_parent_nodes
Signed-off-by: Filipe David Borba Manana --- fs/btrfs/backref.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c index 8bc5e8c..1ba87c5 100644 --- a/fs/btrfs/backref.c +++ b/fs/btrfs/backref.c @@ -935,6 +935,8 @@ again: } ret = find_extent_in_eb(eb, bytenr, *extent_item_pos, &eie); + if (ret < 0) + goto out; ref->inode_list = eie; free_extent_buffer(eb); } -- 1.7.9.5 -- 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 zero divide
Josef Bacik dixit: >Can you gdb btrfs.ko and do > >list *(__btrfs_map_block+0x11c) Not easily (the kernel image is from a .deb package), and even in a compile tree gdb just says: No symbol table is loaded. Use the "file" command. With a bit of cheating and a cross-compiler, this is: (gdb) list *0x106e 0x106e is in __btrfs_map_block (/var/lib/gforge/chroot/home/users/tg/Xl/linux-3.10.1/fs/btrfs/volumes.c:4447). 4442stripe_nr = offset; 4443/* * stripe_nr counts the total number of stripes we have to stride 4445 * to get to this block 4446 */ 4447do_div(stripe_nr, stripe_len); 4448 4449stripe_offset = stripe_nr * stripe_len; 4450BUG_ON(offset < stripe_offset); 4451 The code is somewhat matching: 0x1062 <+268>: movel %fp@(-140),%d1 0x1066 <+272>: movel %fp@(-164),%d5 0x106a <+276>: movel %fp@(-160),%d6 0x106e <+280>: divul %d5,%d2,%d1 0x1072 <+284>: movel %d0,%fp@(-156) 0x1076 <+288>: movel %d1,%fp@(-152) 0x107a <+292>: movel %d5,%d1 0x107c <+294>: mulsl %fp@(-152),%d1 0x1082 <+300>: mulsl %d4,%d0 0x1086 <+304>: moveal %d1,%a0 0x1088 <+306>: addal %d0,%a0 0x108a <+308>: movel %d6,%d1 0x108c <+310>: mulul %fp@(-152),%d0,%d1 According to the registers… [ 38.80] d0: d1: 1000d2: d3: [ 38.83] d4: 0001d5: a0: 3085c72ca1: 3085c72c … this is (if I parse this right) 1000 / (64-bit D2:D1 divided by 32-bit D5 store into D1, remainder into D2). Joe Perches dixit quod… > do_div seems a likely suspect... I do admit I don’t understand arch/m68k/include/asm/div64.h being not a real m68k coder, but “it works elsewhere”… (And I loathe GCC inline asm with a passion!) >From the code expansion, I assume (__upper = __n.n32[0]) is always zero (as we get only one divul instruction). This looks a bit weird because the numbers in question are all 64 bit (stripe_nr, offset, logical). Hm, actually… from a test program… #include typedef unsigned long long u64; int main(void) { u64 stripe_nr; u64 stripe_len; stripe_nr = 1234; stripe_len = 2; printf("in : %llu / %llu\n", stripe_nr, stripe_len); ({ union { unsigned long n32[2]; unsigned long long n64; } __n; unsigned long __rem, __upper; __n.n64 = (stripe_nr); if ((__upper = __n.n32[0])) { asm ("divul.l %2,%1:%0" : "=d" (__n.n32[0]), "=d" (__upper) : "d" (stripe_len), "0" (__n.n32[0])); } asm ("divu.l %2,%1:%0" : "=d" (__n.n32[1]), "=d" (__rem) : "d" (stripe_len), "1" (__upper), "0" (__n.n32[1])); (stripe_nr) = __n.n64; __rem; }); printf("out: %llu R %llu\n", stripe_nr, stripe_len); return (0); } … I think we get two divul instructions, just with a lot of moves between them. Hmpf. The frame pointer would be useful to know, to know the proper values used for these operations… … Aaaah okay. Some reading *(gdb.info):: later, indeed: (gdb) info line 4446 Line 4446 of "/var/lib/gforge/chroot/home/users/tg/Xl/linux-3.10.1/fs/btrfs/volumes.c" is at address 0x104a <__btrfs_map_block+244> but contains no code. (gdb) info line 4448 Line 4448 of "/var/lib/gforge/chroot/home/users/tg/Xl/linux-3.10.1/fs/btrfs/volumes.c" is at address 0x107a <__btrfs_map_block+292> but contains no code. (gdb) disas /r 0x104a,0x107a Dump of assembler code from 0x104a to 0x107a: 0x104a <__btrfs_map_block+244>: 20 02 movel %d2,%d0 0x104c <__btrfs_map_block+246>: 24 2e ff 70 movel %fp@(-144),%d2 0x1050 <__btrfs_map_block+250>: 4a 80 tstl %d0 0x1052 <__btrfs_map_block+252>: 67 0e beqs 0x1062 <__btrfs_map_block+268> 0x1054 <__btrfs_map_block+254>: 20 02 movel %d2,%d0 0x1056 <__btrfs_map_block+256>: 2c 2e ff 5c movel %fp@(-164),%d6 0x105a <__btrfs_map_block+260>: 2e 2e ff 60 movel %fp@(-160),%d7 0x105e <__btrfs_map_block+264>: 4c 46 00 02 divull %d6,%d2,%d0 0x1062 <__btrfs_map_block+268>: 22 2e ff 74 movel %fp@(-140),%d1 0x1066 <__btrfs_map_block+272>: 2a 2e ff 5c movel %fp@(-164),%d5 0x106a <__btrfs_map_block+276>: 2c 2e ff 60 movel %fp@(-160),%d6 0x106e <__btrfs_map_block+280>: 4c 45 14 02 divul %d5,%d2,%d1 0x1072 <__btrfs_map_block+284>: 2d 40 ff 64 movel %d0,%fp@(-156) 0x1076 <__btrfs_map_block+288>: 2d 41 ff 68 movel %d1,%fp@(-152) End of assembler dump. Now, can anyone more fluent in m68k asm make out a problem with it? bye, //mirabilos -- I believe no one can invent an algorithm. One just happens to hit upon it when God enlightens him. Or only God invents algorithms, we merely copy them. If you don't believe in God, just consider God as Nature if you won't deny existence. -- Coywolf Qi Hunt -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a m
[PATCH] Btrfs: check to see if we have an inline item properly
If our item isn't big enough to have an actual inline item when we have skinny metadata enabled just return 1 in find_inline_backref so we can move on to the next item. This probably wasn't causing a problem since we check the values of ptr and end properly, but just in case this will keep us from doing extra work. Thanks, Signed-off-by: Josef Bacik --- fs/btrfs/relocation.c |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c index 5a23d87..8168fb3 100644 --- a/fs/btrfs/relocation.c +++ b/fs/btrfs/relocation.c @@ -641,6 +641,11 @@ int find_inline_backref(struct extent_buffer *leaf, int slot, WARN_ON(item_size < sizeof(*ei) + sizeof(*bi)); return 1; } + if (key.type == BTRFS_METADATA_ITEM_KEY && + item_size <= sizeof(*ei)) { + WARN_ON(item_size < sizeof(*ei)); + return 1; + } if (key.type == BTRFS_EXTENT_ITEM_KEY) { bi = (struct btrfs_tree_block_info *)(ei + 1); -- 1.7.7.6 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Btrfs: change how we queue blocks for backref checking
Previously we only added blocks to the list to have their backrefs checked if the level of the block is right above the one we are searching for. This is because we want to make sure we don't add the entire path up to the root to the lists to make sure we process things one at a time. This assumes that if any blocks in the path to the root are going to be not checked (shared in other words) then they will be in the level right above the current block on up. This isn't quite right though since we can have blocks higher up the list that are shared because they are attached to a reloc root. But we won't add this block to be checked and then later on we will BUG_ON(!upper->checked). So instead keep track of wether or not we've queued a block to be checked in this current search, and if we haven't go ahead and queue it to be checked. This patch fixed the panic I was seeing where we BUG_ON(!upper->checked). Thanks, Signed-off-by: Josef Bacik --- fs/btrfs/relocation.c | 14 +++--- 1 files changed, 7 insertions(+), 7 deletions(-) diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c index 8168fb3..2ec59c4 100644 --- a/fs/btrfs/relocation.c +++ b/fs/btrfs/relocation.c @@ -696,6 +696,7 @@ struct backref_node *build_backref_tree(struct reloc_control *rc, int cowonly; int ret; int err = 0; + bool need_check = true; path1 = btrfs_alloc_path(); path2 = btrfs_alloc_path(); @@ -919,6 +920,7 @@ again: cur->bytenr); lower = cur; + need_check = true; for (; level < BTRFS_MAX_LEVEL; level++) { if (!path2->nodes[level]) { BUG_ON(btrfs_root_bytenr(&root->root_item) != @@ -962,14 +964,12 @@ again: /* * add the block to pending list if we -* need check its backrefs. only block -* at 'cur->level + 1' is added to the -* tail of pending list. this guarantees -* we check backrefs from lower level -* blocks to upper level blocks. +* need check its backrefs, we only do this once +* while walking up a tree as we will catch +* anything else later on. */ - if (!upper->checked && - level == cur->level + 1) { + if (!upper->checked && need_check) { + need_check = false; list_add_tail(&edge->list[UPPER], &list); } else -- 1.7.7.6 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: btrfs zero divide
On Tue, Jul 30, 2013 at 07:02:29PM +, Thorsten Glaser wrote: > Josef Bacik dixit: > > >Can you gdb btrfs.ko and do > > > >list *(__btrfs_map_block+0x11c) > > Not easily (the kernel image is from a .deb package), > and even in a compile tree gdb just says: > No symbol table is loaded. Use the "file" command. > > With a bit of cheating and a cross-compiler, this is: > > (gdb) list *0x106e > 0x106e is in __btrfs_map_block > (/var/lib/gforge/chroot/home/users/tg/Xl/linux-3.10.1/fs/btrfs/volumes.c:4447). > 4442stripe_nr = offset; > 4443/* > * stripe_nr counts the total number of stripes we have to > stride > 4445 * to get to this block > 4446 */ > 4447do_div(stripe_nr, stripe_len); > 4448 > 4449stripe_offset = stripe_nr * stripe_len; > 4450BUG_ON(offset < stripe_offset); > 4451 > So stripe_len shouldn't be 0, if it is you have bigger problems :). Is this a corrupt fs or something? If there was some sort of corruption that occured then I suppose stripe_len could be 0 and we'd need to catch that somewhere higher up the stack and error out. Is there a way you could check and see if that's the case? Thanks, Josef -- 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 zero divide
On Tue, 2013-07-30 at 16:40 -0400, Josef Bacik wrote: > So stripe_len shouldn't be 0, if it is you have bigger problems :). Is this a > corrupt fs or something? If there was some sort of corruption that occured > then > I suppose stripe_len could be 0 and we'd need to catch that somewhere higher > up > the stack and error out. Is there a way you could check and see if that's the > case? Thanks, Maybe use a temporary check in do_div Something like this maybe. (uncompiled/untested) --- include/asm-generic/div64.h | 43 +-- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/include/asm-generic/div64.h b/include/asm-generic/div64.h index 8f4e319..cce75fe 100644 --- a/include/asm-generic/div64.h +++ b/include/asm-generic/div64.h @@ -19,16 +19,25 @@ #include #include +#include +#include #if BITS_PER_LONG == 64 -# define do_div(n,base) ({ \ +# define do_div(n, base) \ +({ \ uint32_t __base = (base); \ uint32_t __rem; \ - __rem = ((uint64_t)(n)) % __base; \ - (n) = ((uint64_t)(n)) / __base; \ + if (__base == 0) { \ + WARN(1, "Attempted division by 0\n"); \ + dump_stack(); \ + __rem = 0; \ + } else {\ + __rem = ((uint64_t)(n)) % __base; \ + (n) = ((uint64_t)(n)) / __base; \ + } \ __rem; \ - }) +}) #elif BITS_PER_LONG == 32 @@ -37,16 +46,22 @@ extern uint32_t __div64_32(uint64_t *dividend, uint32_t divisor); /* The unnecessary pointer compare is there * to check for type safety (n must be 64bit) */ -# define do_div(n,base) ({ \ - uint32_t __base = (base); \ - uint32_t __rem; \ - (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \ - if (likely(((n) >> 32) == 0)) { \ - __rem = (uint32_t)(n) % __base; \ - (n) = (uint32_t)(n) / __base; \ - } else \ - __rem = __div64_32(&(n), __base); \ - __rem; \ +# define do_div(n, base) \ +({ \ + uint32_t __base = (base); \ + uint32_t __rem; \ + (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \ + if (__base == 0) { \ + WARN(1, "Attempted division by 0\n"); \ + dump_stack(); \ + __rem = 0; \ + } else if (likely(((n) >> 32) == 0)) { \ + __rem = (uint32_t)(n) % __base; \ + (n) = (uint32_t)(n) / __base; \ + } else {\ + __rem = __div64_32(&(n), __base); \ + } \ + __rem; \ }) #else /* BITS_PER_LONG == ?? */ -- 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 zero divide
Josef Bacik dixit: >So stripe_len shouldn't be 0, if it is you have bigger problems :). ☺ >Is this a corrupt fs or something? If there was some sort of I don’t think so, I can access and use that filesystem under 3.2 just fine (it’s what I created it under, too, so it’s possible that it’s indeed corrupt and Linux 3.2 is just the same corrupt to happen to make it work, e.g. wrong endianness used for stripe_len which makes the upper 32 bit of that 64-bit value (usually 0) become the lower 32 bit, or something like that). I have access to that system, and it’s currently running as a Debian/m68k buildd using said filesystem, but I can run commands you tell me to diagnose/analyse it if it won’t get corrupted by those. Joe Perches dixit: >Maybe use a temporary check in do_div Mh. If nobody finds anything I’ll try that. (Doing things like compiling a kernel and testing it takes about two days timeboxed and some hours of active human effort, though, so I’d like to avoid guessing. Plus it’ll disrupt running the Debian buildd…) On second thoughts, this sort of check sounds like a good idea to add to that file in general, depending on some debugging CPPFLAG or Kconfig option. But I’m not the authority on that. bye, //mirabilos -- you introduced a merge commit│ % g rebase -i HEAD^^ sorry, no idea and rebasing just fscked │ Segmentation should have cloned into a clean repo │ fault (core dumped) if I rebase that now, it's really ugh │ wuahh -- 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: add missing error check to find_parent_nodes
> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c > index 8bc5e8c..1ba87c5 100644 > --- a/fs/btrfs/backref.c > +++ b/fs/btrfs/backref.c > @@ -935,6 +935,8 @@ again: > } > ret = find_extent_in_eb(eb, bytenr, > *extent_item_pos, &eie); > + if (ret < 0) > + goto out; > ref->inode_list = eie; > free_extent_buffer(eb); > } The eb needs to be freed before the goto. And that there's now *three* eb freeing paths in this block should be a hint that it should be refactored into a function that frees the eb before returning. - z -- 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: add missing error check to find_parent_nodes
Signed-off-by: Filipe David Borba Manana --- V2: Ensure extent buffer is freed on error. fs/btrfs/backref.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c index 8bc5e8c..980e85a 100644 --- a/fs/btrfs/backref.c +++ b/fs/btrfs/backref.c @@ -935,8 +935,10 @@ again: } ret = find_extent_in_eb(eb, bytenr, *extent_item_pos, &eie); - ref->inode_list = eie; free_extent_buffer(eb); + if (ret < 0) + goto out; + ref->inode_list = eie; } ret = ulist_add_merge(refs, ref->parent, (uintptr_t)ref->inode_list, -- 1.7.9.5 -- 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: add missing error check to find_parent_nodes
On Wed, Jul 31, 2013 at 12:00 AM, Zach Brown wrote: >> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c >> index 8bc5e8c..1ba87c5 100644 >> --- a/fs/btrfs/backref.c >> +++ b/fs/btrfs/backref.c >> @@ -935,6 +935,8 @@ again: >> } >> ret = find_extent_in_eb(eb, bytenr, >> *extent_item_pos, >> &eie); >> + if (ret < 0) >> + goto out; >> ref->inode_list = eie; >> free_extent_buffer(eb); >> } > > The eb needs to be freed before the goto. > > And that there's now *three* eb freeing paths in this block should be a > hint that it should be refactored into a function that frees the eb > before returning. Thanks, missed that previously. > > - z -- Filipe David Manana, "Reasonable men adapt themselves to the world. Unreasonable men adapt the world to themselves. That's why all progress depends on unreasonable men." -- 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: add missing error handling to read_tree_block
Signed-off-by: Filipe David Borba Manana --- fs/btrfs/disk-io.c |4 1 file changed, 4 insertions(+) diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 254cdc8..b71e882 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -1148,6 +1148,10 @@ struct extent_buffer *read_tree_block(struct btrfs_root *root, u64 bytenr, return NULL; ret = btree_read_extent_buffer_pages(root, buf, 0, parent_transid); + if (ret) { + free_extent_buffer(buf); + return NULL; + } return buf; } -- 1.7.9.5 -- 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: optimize function btrfs_read_chunk_tree
On tue, 30 Jul 2013 12:03:04 +0100, Filipe David Borba Manana wrote: > After reading all device items from the chunk tree, don't > exit the loop and then navigate down the tree again to find > the chunk items. Instead just read all device items and > chunk items with a single tree search. This is possible > because all device items are found before any chunk item in > the chunks tree. > > Signed-off-by: Filipe David Borba Manana > --- > > V2: Simplified logic inside the loop > (suggested by Josef Bacik on irc). > V3: Updated comment to comply with kernel coding style. Reviewed-by: Miao Xie > > fs/btrfs/volumes.c | 30 +++--- > 1 file changed, 11 insertions(+), 19 deletions(-) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 090f57c..125a60e 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -5676,14 +5676,15 @@ int btrfs_read_chunk_tree(struct btrfs_root *root) > mutex_lock(&uuid_mutex); > lock_chunks(root); > > - /* first we search for all of the device items, and then we > - * read in all of the chunk items. This way we can create chunk > - * mappings that reference all of the devices that are afound > + /* > + * Read all device items, and then all the chunk items. All > + * device items are found before any chunk item (their object id > + * is smaller than the lowest possible object id for a chunk > + * item - BTRFS_FIRST_CHUNK_TREE_OBJECTID). >*/ > key.objectid = BTRFS_DEV_ITEMS_OBJECTID; > key.offset = 0; > key.type = 0; > -again: > ret = btrfs_search_slot(NULL, root, &key, path, 0, 0); > if (ret < 0) > goto error; > @@ -5699,17 +5700,13 @@ again: > break; > } > btrfs_item_key_to_cpu(leaf, &found_key, slot); > - if (key.objectid == BTRFS_DEV_ITEMS_OBJECTID) { > - if (found_key.objectid != BTRFS_DEV_ITEMS_OBJECTID) > - break; > - if (found_key.type == BTRFS_DEV_ITEM_KEY) { > - struct btrfs_dev_item *dev_item; > - dev_item = btrfs_item_ptr(leaf, slot, > + if (found_key.type == BTRFS_DEV_ITEM_KEY) { > + struct btrfs_dev_item *dev_item; > + dev_item = btrfs_item_ptr(leaf, slot, > struct btrfs_dev_item); > - ret = read_one_dev(root, leaf, dev_item); > - if (ret) > - goto error; > - } > + ret = read_one_dev(root, leaf, dev_item); > + if (ret) > + goto error; > } else if (found_key.type == BTRFS_CHUNK_ITEM_KEY) { > struct btrfs_chunk *chunk; > chunk = btrfs_item_ptr(leaf, slot, struct btrfs_chunk); > @@ -5719,11 +5716,6 @@ again: > } > path->slots[0]++; > } > - if (key.objectid == BTRFS_DEV_ITEMS_OBJECTID) { > - key.objectid = 0; > - btrfs_release_path(path); > - goto again; > - } > ret = 0; > error: > unlock_chunks(root); > -- 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: optimize function btrfs_read_chunk_tree
On tue, 30 Jul 2013 12:08:17 +0100, Filipe David Borba Manana wrote: > After reading all device items from the chunk tree, don't > exit the loop and then navigate down the tree again to find > the chunk items. Instead just read all device items and > chunk items with a single tree search. This is possible > because all device items are found before any chunk item in > the chunks tree. > > This is a port of the corresponding kernel patch to keep both > kernel and btrfs-progs identical: > > https://patchwork.kernel.org/patch/2835529/ > > Signed-off-by: Filipe David Borba Manana > --- > > V2: Simplified logic inside the loop > (suggested by Josef Bacik on irc). > V3: Updated comment to comply with kernel coding style. Reviewed-by: Miao Xie > > volumes.c | 28 ++-- > 1 file changed, 10 insertions(+), 18 deletions(-) > > diff --git a/volumes.c b/volumes.c > index 0ff2283..0cb3505 100644 > --- a/volumes.c > +++ b/volumes.c > @@ -1718,14 +1718,15 @@ int btrfs_read_chunk_tree(struct btrfs_root *root) > if (!path) > return -ENOMEM; > > - /* first we search for all of the device items, and then we > - * read in all of the chunk items. This way we can create chunk > - * mappings that reference all of the devices that are afound > + /* > + * Read all device items, and then all the chunk items. All > + * device items are found before any chunk item (their object id > + * is smaller than the lowest possible object id for a chunk > + * item - BTRFS_FIRST_CHUNK_TREE_OBJECTID). >*/ > key.objectid = BTRFS_DEV_ITEMS_OBJECTID; > key.offset = 0; > key.type = 0; > -again: > ret = btrfs_search_slot(NULL, root, &key, path, 0, 0); > while(1) { > leaf = path->nodes[0]; > @@ -1739,16 +1740,12 @@ again: > break; > } > btrfs_item_key_to_cpu(leaf, &found_key, slot); > - if (key.objectid == BTRFS_DEV_ITEMS_OBJECTID) { > - if (found_key.objectid != BTRFS_DEV_ITEMS_OBJECTID) > - break; > - if (found_key.type == BTRFS_DEV_ITEM_KEY) { > - struct btrfs_dev_item *dev_item; > - dev_item = btrfs_item_ptr(leaf, slot, > + if (found_key.type == BTRFS_DEV_ITEM_KEY) { > + struct btrfs_dev_item *dev_item; > + dev_item = btrfs_item_ptr(leaf, slot, > struct btrfs_dev_item); > - ret = read_one_dev(root, leaf, dev_item); > - BUG_ON(ret); > - } > + ret = read_one_dev(root, leaf, dev_item); > + BUG_ON(ret); > } else if (found_key.type == BTRFS_CHUNK_ITEM_KEY) { > struct btrfs_chunk *chunk; > chunk = btrfs_item_ptr(leaf, slot, struct btrfs_chunk); > @@ -1757,11 +1754,6 @@ again: > } > path->slots[0]++; > } > - if (key.objectid == BTRFS_DEV_ITEMS_OBJECTID) { > - key.objectid = 0; > - btrfs_release_path(root, path); > - goto again; > - } > > ret = 0; > error: > -- 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: add missing path alloc return value check
On tue, 30 Jul 2013 12:09:55 +0100, Filipe David Borba Manana wrote: > Also remove unused path in extent-tree.c:finish_current_insert(). > > Signed-off-by: Filipe David Borba Manana > --- > > V2: added 1 more path alloc check and removed unnecessary path > allocation in extent-tree.c:finish_current_insert(). > V3: added missing path alloc checks to dir-item.c, file-item.c > and btrfs-corrupt-block.c too. > V4: added missing patch alloc checks to cmd-checks.c and > root-tree.c. Reviewed-by: Miao Xie > > btrfs-corrupt-block.c |2 ++ > cmds-check.c |2 ++ > dir-item.c|2 ++ > extent-tree.c |8 > file-item.c |2 ++ > root-tree.c |2 ++ > 6 files changed, 14 insertions(+), 4 deletions(-) > > diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c > index 8176fad..22facd4 100644 > --- a/btrfs-corrupt-block.c > +++ b/btrfs-corrupt-block.c > @@ -159,6 +159,8 @@ static int corrupt_extent(struct btrfs_trans_handle > *trans, > int should_del = rand() % 3; > > path = btrfs_alloc_path(); > + if (!path) > + return -ENOMEM; > > key.objectid = bytenr; > key.type = (u8)-1; > diff --git a/cmds-check.c b/cmds-check.c > index 8015288..c1a0df9 100644 > --- a/cmds-check.c > +++ b/cmds-check.c > @@ -4561,6 +4561,8 @@ static int fixup_extent_refs(struct btrfs_trans_handle > *trans, > flags = BTRFS_BLOCK_FLAG_FULL_BACKREF; > > path = btrfs_alloc_path(); > + if (!path) > + return -ENOMEM; > > /* step one, make sure all of the backrefs agree */ > ret = verify_backrefs(trans, info, path, rec); > diff --git a/dir-item.c b/dir-item.c > index f00485a..0ab3c5e 100644 > --- a/dir-item.c > +++ b/dir-item.c > @@ -123,6 +123,8 @@ int btrfs_insert_dir_item(struct btrfs_trans_handle > *trans, struct btrfs_root > btrfs_set_key_type(&key, BTRFS_DIR_ITEM_KEY); > key.offset = btrfs_name_hash(name, name_len); > path = btrfs_alloc_path(); > + if (!path) > + return -ENOMEM; > data_size = sizeof(*dir_item) + name_len; > dir_item = insert_with_overflow(trans, root, path, &key, data_size, > name, name_len); > diff --git a/extent-tree.c b/extent-tree.c > index f597e16..e4adaa3 100644 > --- a/extent-tree.c > +++ b/extent-tree.c > @@ -1487,6 +1487,8 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle > *trans, > } > > path = btrfs_alloc_path(); > + if (!path) > + return -ENOMEM; > path->reada = 1; > > key.objectid = bytenr; > @@ -1577,6 +1579,8 @@ int btrfs_set_block_flags(struct btrfs_trans_handle > *trans, > BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA); > > path = btrfs_alloc_path(); > + if (!path) > + return -ENOMEM; > path->reada = 1; > > key.objectid = bytenr; > @@ -2078,7 +2082,6 @@ static int finish_current_insert(struct > btrfs_trans_handle *trans, > u64 end; > u64 priv; > struct btrfs_fs_info *info = extent_root->fs_info; > - struct btrfs_path *path; > struct pending_extent_op *extent_op; > struct btrfs_key key; > int ret; > @@ -2086,8 +2089,6 @@ static int finish_current_insert(struct > btrfs_trans_handle *trans, > btrfs_fs_incompat(extent_root->fs_info, > BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA); > > - path = btrfs_alloc_path(); > - > while(1) { > ret = find_first_extent_bit(&info->extent_ins, 0, &start, > &end, EXTENT_LOCKED); > @@ -2121,7 +2122,6 @@ static int finish_current_insert(struct > btrfs_trans_handle *trans, > GFP_NOFS); > kfree(extent_op); > } > - btrfs_free_path(path); > return 0; > } > > diff --git a/file-item.c b/file-item.c > index 9c787f0..82bf99e 100644 > --- a/file-item.c > +++ b/file-item.c > @@ -417,6 +417,8 @@ int btrfs_del_csums(struct btrfs_trans_handle *trans, > root = root->fs_info->csum_root; > > path = btrfs_alloc_path(); > + if (!path) > + return -ENOMEM; > > while (1) { > key.objectid = BTRFS_EXTENT_CSUM_OBJECTID; > diff --git a/root-tree.c b/root-tree.c > index ba380bd..efcdb7b 100644 > --- a/root-tree.c > +++ b/root-tree.c > @@ -264,6 +264,8 @@ int btrfs_add_root_ref(struct btrfs_trans_handle *trans, > > > path = btrfs_alloc_path(); > + if (!path) > + return -ENOMEM; > > key.objectid = root_id; > key.type = type; > -- 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 RESEND] Btrfs: fix crash regarding to ulist_add_merge
Several users reported this crash of NULL pointer or general protection, the story is that we add a rbtree for speedup ulist iteration, and we use krealloc() to address ulist growth, and krealloc() use memcpy to copy old data to new memory area, so it's OK for an array as it doesn't use pointers while it's not OK for a rbtree as it uses pointers. So krealloc() will mess up our rbtree and it ends up with crash. Cc: sta...@vger.kernel.org Reviewed-by: Wang Shilong Reviewed-by: Jan Schmidt Signed-off-by: Liu Bo Signed-off-by: Josef Bacik --- Cc -stable! fs/btrfs/ulist.c | 15 +++ 1 file changed, 15 insertions(+) diff --git a/fs/btrfs/ulist.c b/fs/btrfs/ulist.c index 7b417e2..b0a523b2 100644 --- a/fs/btrfs/ulist.c +++ b/fs/btrfs/ulist.c @@ -205,6 +205,10 @@ int ulist_add_merge(struct ulist *ulist, u64 val, u64 aux, u64 new_alloced = ulist->nodes_alloced + 128; struct ulist_node *new_nodes; void *old = NULL; + int i; + + for (i = 0; i < ulist->nnodes; i++) + rb_erase(&ulist->nodes[i].rb_node, &ulist->root); /* * if nodes_alloced == ULIST_SIZE no memory has been allocated @@ -224,6 +228,17 @@ int ulist_add_merge(struct ulist *ulist, u64 val, u64 aux, ulist->nodes = new_nodes; ulist->nodes_alloced = new_alloced; + + /* +* krealloc actually uses memcpy, which does not copy rb_node +* pointers, so we have to do it ourselves. Otherwise we may +* be bitten by crashes. +*/ + for (i = 0; i < ulist->nnodes; i++) { + ret = ulist_rbtree_insert(ulist, &ulist->nodes[i]); + if (ret < 0) + return ret; + } } ulist->nodes[ulist->nnodes].val = val; ulist->nodes[ulist->nnodes].aux = aux; -- 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 qgroup assign -> "ERROR: bad relation requested"
On Tue, 30 Jul 2013 21:06:32 +0800 Wang Shilong wrote: > I have implemented the above function, but they haven't gone into > upstream Btrfs-progs. > > You can try this: > > http://github.com/miaoxie/btrfs-progs.git qgroup > > Maybe i need rebase the patch-set later, But you can try it firstly, > if you want to see every qgroup's parent, you can add option '-p': > > btrfs qgroup show -p Hmm, is it correct git URL? # btrfs qgroup show -p /mnt/lxc2 btrfs qgroup show: too many arguments usage: btrfs qgroup show Show all subvolume quota groups. -- Tomasz Chmielewski http://wpkg.org -- 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 qgroup assign -> "ERROR: bad relation requested"
On 07/31/2013 11:46 AM, Tomasz Chmielewski wrote: > On Tue, 30 Jul 2013 21:06:32 +0800 > Wang Shilong wrote: > >> I have implemented the above function, but they haven't gone into >> upstream Btrfs-progs. >> >> You can try this: >> >> http://github.com/miaoxie/btrfs-progs.git qgroup >> >> Maybe i need rebase the patch-set later, But you can try it firstly, >> if you want to see every qgroup's parent, you can add option '-p': >> >> btrfs qgroup show -p > > Hmm, is it correct git URL? I have tried it and it works, you can do like the following to try it. #git clone http://github.com/miaoxie/btrfs-progs.git #cd btrfs-progs #git pull origin master #make && make install Thanks, Wang > > # btrfs qgroup show -p /mnt/lxc2 > btrfs qgroup show: too many arguments > usage: btrfs qgroup show > > Show all subvolume quota groups. > > > -- 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 qgroup assign -> "ERROR: bad relation requested"
On Wed, 31 Jul 2013 12:19:36 +0800 Wang Shilong wrote: > I have tried it and it works, you can do like the following to try it. > > #git clone http://github.com/miaoxie/btrfs-progs.git > > #cd btrfs-progs > > #git pull origin master > > #make && make install Hmm: # git clone http://github.com/miaoxie/btrfs-progs.git Cloning into 'btrfs-progs'... remote: Counting objects: 3917, done. remote: Compressing objects: 100% (1565/1565), done. remote: Total 3917 (delta 2681), reused 3582 (delta 2348) Receiving objects: 100% (3917/3917), 1.88 MiB | 634 KiB/s, done. Resolving deltas: 100% (2681/2681), done. # cd btrfs-progs/ # git pull origin master >From http://github.com/miaoxie/btrfs-progs * branchmaster -> FETCH_HEAD Already up-to-date. # make && make install (...) # btrfs qgroup show -p /mnt/lxc2 btrfs qgroup show: too many arguments usage: btrfs qgroup show Show all subvolume quota groups. This is the right binary (by looking at timestamps): # /usr/local/bin/btrfs qgroup show -p /mnt/lxc2 btrfs qgroup show: too many arguments usage: btrfs qgroup show Show all subvolume quota groups. # ls -l /usr/local/bin/btrfs -rwxr-xr-x 1 root staff 1.5M Jul 31 04:30 /usr/local/bin/btrfs # date Wed Jul 31 04:33:59 UTC 2013 "git log" is quite old, too (last change from Thu Nov 8 17:54:13 2012 +0800). -- Tomasz Chmielewski http://wpkg.org -- 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 qgroup assign -> "ERROR: bad relation requested"
On 07/31/2013 12:36 PM, Tomasz Chmielewski wrote: > On Wed, 31 Jul 2013 12:19:36 +0800 > Wang Shilong wrote: > >> I have tried it and it works, you can do like the following to try it. >> >> #git clone http://github.com/miaoxie/btrfs-progs.git >> >> #cd btrfs-progs >> >> #git pull origin master >> >> #make && make install > > Hmm: > > # git clone http://github.com/miaoxie/btrfs-progs.git > Cloning into 'btrfs-progs'... > remote: Counting objects: 3917, done. > remote: Compressing objects: 100% (1565/1565), done. > remote: Total 3917 (delta 2681), reused 3582 (delta 2348) > Receiving objects: 100% (3917/3917), 1.88 MiB | 634 KiB/s, done. > Resolving deltas: 100% (2681/2681), done. > > # cd btrfs-progs/ > > # git pull origin master Oops, i am sorry, here should: git pull origin qgroup would you please try it again >>From http://github.com/miaoxie/btrfs-progs > * branchmaster -> FETCH_HEAD > Already up-to-date. > > # make && make install > (...) > > # btrfs qgroup show -p /mnt/lxc2 > btrfs qgroup show: too many arguments > usage: btrfs qgroup show > > Show all subvolume quota groups. > > > > This is the right binary (by looking at timestamps): > > # /usr/local/bin/btrfs qgroup show -p /mnt/lxc2 > btrfs qgroup show: too many arguments > usage: btrfs qgroup show > > Show all subvolume quota groups. > > # ls -l /usr/local/bin/btrfs > -rwxr-xr-x 1 root staff 1.5M Jul 31 04:30 /usr/local/bin/btrfs > > # date > Wed Jul 31 04:33:59 UTC 2013 > > > "git log" is quite old, too (last change from Thu Nov 8 17:54:13 2012 +0800). Yeah, it is old, i need rebase my patch-set on the latest btrfs-progs. Thanks, Wang > > -- 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 qgroup assign -> "ERROR: bad relation requested"
On Wed, 31 Jul 2013 13:13:37 +0800 Wang Shilong wrote: > > # git pull origin master > > Oops, i am sorry, here should: > > git pull origin qgroup > > would you please try it again Excellent, it works: # btrfs qgroup show -p /mnt/lxc2 0/260 154273873920 154273873920 --- 0/261 181260288 181260288 --- 0/262 17888 983040 --- 0/377 180539392 26005504 --- 0/378 308607238144 308607238144 --- 0/535 572407808 417832960 --- 0/536 68085026816 68085026816 --- 0/642 247824384 93282304 --- 0/1276 636026880 4096 1/1 0/1277 38301687808 4096 1/1 0/1278 617164800 32768 1/1 0/1279 38297591808 4096 1/1 0/1284 38299160576 0 1/1 0/1285 38299160576 21364736 1/1 0/1286 620396544 0 1/1 0/1287 633090048 12722176 1/1 0/1293 38301687808 4096 1/1 0/1294 636026880 4096 1/1 1/1 38972137472 38972137472 --- Thanks a lot! Am I correct to think that I have to run quota rescan after assigning groups? # btrfs qgroup assign 378 1/1 /mnt/lxc2 <<- assigning volid 378, having 300+ GB # btrfs qgroup show -p /mnt/lxc2 | grep 1/1 0/378 308607238144 308607238144 1/1 0/1276 636026880 4096 1/1 0/1277 38301687808 4096 1/1 0/1278 617164800 32768 1/1 0/1279 38297591808 4096 1/1 0/1284 38299160576 0 1/1 0/1285 38299160576 21364736 1/1 0/1286 620396544 0 1/1 0/1287 633090048 12722176 1/1 0/1293 38301687808 4096 1/1 0/1294 636026880 4096 1/1 1/1 38972137472 38972137472 --- <<- although we've added 300+ GB volume, the total numbers are unchanged That can be quite a bit of IO? -- Tomasz Chmielewski http://wpkg.org -- 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 qgroup assign -> "ERROR: bad relation requested"
Hello, On 07/31/2013 01:39 PM, Tomasz Chmielewski wrote: > On Wed, 31 Jul 2013 13:13:37 +0800 > Wang Shilong wrote: > >>> # git pull origin master >> >> Oops, i am sorry, here should: >> >> git pull origin qgroup >> >> would you please try it again > > Excellent, it works: > > # btrfs qgroup show -p /mnt/lxc2 > 0/260 154273873920 154273873920 --- > 0/261 181260288 181260288 --- > 0/262 17888 983040 --- > 0/377 180539392 26005504 --- > 0/378 308607238144 308607238144 --- > 0/535 572407808 417832960 --- > 0/536 68085026816 68085026816 --- > 0/642 247824384 93282304 --- > 0/1276 636026880 4096 1/1 > 0/1277 38301687808 4096 1/1 > 0/1278 617164800 32768 1/1 > 0/1279 38297591808 4096 1/1 > 0/1284 38299160576 0 1/1 > 0/1285 38299160576 21364736 1/1 > 0/1286 620396544 0 1/1 > 0/1287 633090048 12722176 1/1 > 0/1293 38301687808 4096 1/1 > 0/1294 636026880 4096 1/1 > 1/1 38972137472 38972137472 --- > > > Thanks a lot! > > > Am I correct to think that I have to run quota rescan after > assigning groups? > > # btrfs qgroup assign 378 1/1 /mnt/lxc2 <<- assigning volid 378, having > 300+ GB Until now, Yes, we have to recan to correct qgroup accounting. > > # btrfs qgroup show -p /mnt/lxc2 | grep 1/1 > 0/378 308607238144 308607238144 1/1 > 0/1276 636026880 4096 1/1 > 0/1277 38301687808 4096 1/1 > 0/1278 617164800 32768 1/1 > 0/1279 38297591808 4096 1/1 > 0/1284 38299160576 0 1/1 > 0/1285 38299160576 21364736 1/1 > 0/1286 620396544 0 1/1 > 0/1287 633090048 12722176 1/1 > 0/1293 38301687808 4096 1/1 > 0/1294 636026880 4096 1/1 > 1/1 38972137472 38972137472 --- <<- although we've added 300+ GB volume, > the total numbers are unchanged > > > That can be quite a bit of IO? > > -- 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