| Message ID | 1441897376-5572-1-git-send-email-dsterba@suse.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
David Sterba wrote on 2015/09/10 17:02 +0200: > Commit 854437ca3c228d8ab3eb24d2efc1c21b5d56a635 ("btrfs-progs: > extent-tree: avoid allocating tree block that crosses stripe boundary") > does not work for 64k nodesize. Due to an off-by-one error, all queries > to check_crossing_stripes will return that all extents cross a stripe > and this will lead to a false ENOSPC. This crashes later > > $ ./mkfs.btrfs -n 64k image > > ./mkfs.btrfs(btrfs_reserve_extent+0xb77)[0x417f38] > ./mkfs.btrfs(btrfs_alloc_free_block+0x57)[0x417fe0] > ./mkfs.btrfs(__btrfs_cow_block+0x163)[0x408eb7] > ./mkfs.btrfs(btrfs_cow_block+0xd0)[0x4097c4] > ./mkfs.btrfs(btrfs_search_slot+0x16f)[0x40be4d] > ./mkfs.btrfs(btrfs_insert_empty_items+0xc0)[0x40d5f9] > ./mkfs.btrfs(btrfs_insert_item+0x99)[0x40da5f] > ./mkfs.btrfs(btrfs_make_block_group+0x4d)[0x41705c] > ./mkfs.btrfs(main+0xeef)[0x434b56] > > CC: Qu Wenruo <quwenruo@cn.fujitsu.com> > Signed-off-by: David Sterba <dsterba@suse.com> Nice one, yep, my fault. BTW, any idea to add mkfs test? Reviewed-by: Qu Wenruo <quwenruo@cn.fujitsu.com> Thanks, Qu > --- > volumes.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/volumes.h b/volumes.h > index f7761311e8ce..4ecb99314a0c 100644 > --- a/volumes.h > +++ b/volumes.h > @@ -156,7 +156,7 @@ struct map_lookup { > static inline int check_crossing_stripes(u64 start, u64 len) > { > return (start / BTRFS_STRIPE_LEN) != > - ((start + len) / BTRFS_STRIPE_LEN); > + ((start + len - 1) / BTRFS_STRIPE_LEN); > } > > int __btrfs_map_block(struct btrfs_mapping_tree *map_tree, int rw, > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Sep 10, 2015 at 5:02 PM, David Sterba <dsterba@suse.com> wrote: > Commit 854437ca3c228d8ab3eb24d2efc1c21b5d56a635 ("btrfs-progs: > extent-tree: avoid allocating tree block that crosses stripe boundary") > does not work for 64k nodesize. Due to an off-by-one error, all queries > to check_crossing_stripes will return that all extents cross a stripe > and this will lead to a false ENOSPC. This crashes later > > $ ./mkfs.btrfs -n 64k image > > ./mkfs.btrfs(btrfs_reserve_extent+0xb77)[0x417f38] > ./mkfs.btrfs(btrfs_alloc_free_block+0x57)[0x417fe0] > ./mkfs.btrfs(__btrfs_cow_block+0x163)[0x408eb7] > ./mkfs.btrfs(btrfs_cow_block+0xd0)[0x4097c4] > ./mkfs.btrfs(btrfs_search_slot+0x16f)[0x40be4d] > ./mkfs.btrfs(btrfs_insert_empty_items+0xc0)[0x40d5f9] > ./mkfs.btrfs(btrfs_insert_item+0x99)[0x40da5f] > ./mkfs.btrfs(btrfs_make_block_group+0x4d)[0x41705c] > ./mkfs.btrfs(main+0xeef)[0x434b56] Am I correct that this also causes false positives with btrfs check? I just ran a sanity check on an fs that had no problems whatsoever and was definitely not converted (so 16k nodesize) and got thousands of cross-stripe complaints; repair didn't help. Applying the patch seems to have fixed those; it completes without problems now. Holger -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Sep 11, 2015 at 03:16:02PM +0200, Holger Hoffstätte wrote: > Am I correct that this also causes false positives with btrfs check? I just > ran a sanity check on an fs that had no problems whatsoever and was > definitely not converted (so 16k nodesize) and got thousands of > cross-stripe complaints; repair didn't help. Applying the patch seems to > have fixed those; it completes without problems now. Yes you are. If the node blocks end at the stripe boundary, they're incorrectly marked as stripe crossing. The 64k nodesize case was quick to detect that because this holds for all nodes. Thanks for your report, this means the bug is not that rare. I'll do a release within a few days including this patch. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Sep 11, 2015 at 08:27:17AM +0800, Qu Wenruo wrote:
> BTW, any idea to add mkfs test?
Yes, I'll add a test that will cycle through combinations of various
options (nodesize, raid profiles).
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/volumes.h b/volumes.h index f7761311e8ce..4ecb99314a0c 100644 --- a/volumes.h +++ b/volumes.h @@ -156,7 +156,7 @@ struct map_lookup { static inline int check_crossing_stripes(u64 start, u64 len) { return (start / BTRFS_STRIPE_LEN) != - ((start + len) / BTRFS_STRIPE_LEN); + ((start + len - 1) / BTRFS_STRIPE_LEN); } int __btrfs_map_block(struct btrfs_mapping_tree *map_tree, int rw,
Commit 854437ca3c228d8ab3eb24d2efc1c21b5d56a635 ("btrfs-progs: extent-tree: avoid allocating tree block that crosses stripe boundary") does not work for 64k nodesize. Due to an off-by-one error, all queries to check_crossing_stripes will return that all extents cross a stripe and this will lead to a false ENOSPC. This crashes later $ ./mkfs.btrfs -n 64k image ./mkfs.btrfs(btrfs_reserve_extent+0xb77)[0x417f38] ./mkfs.btrfs(btrfs_alloc_free_block+0x57)[0x417fe0] ./mkfs.btrfs(__btrfs_cow_block+0x163)[0x408eb7] ./mkfs.btrfs(btrfs_cow_block+0xd0)[0x4097c4] ./mkfs.btrfs(btrfs_search_slot+0x16f)[0x40be4d] ./mkfs.btrfs(btrfs_insert_empty_items+0xc0)[0x40d5f9] ./mkfs.btrfs(btrfs_insert_item+0x99)[0x40da5f] ./mkfs.btrfs(btrfs_make_block_group+0x4d)[0x41705c] ./mkfs.btrfs(main+0xeef)[0x434b56] CC: Qu Wenruo <quwenruo@cn.fujitsu.com> Signed-off-by: David Sterba <dsterba@suse.com> --- volumes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)