All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] btrfs-progs: fix cross stripe boundary check
@ 2015-09-10 15:02 David Sterba
  2015-09-11  0:27 ` Qu Wenruo
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: David Sterba @ 2015-09-10 15:02 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba, Qu Wenruo

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(-)

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,
-- 
2.1.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] btrfs-progs: fix cross stripe boundary check
  2015-09-10 15:02 [PATCH] btrfs-progs: fix cross stripe boundary check David Sterba
@ 2015-09-11  0:27 ` Qu Wenruo
  2015-09-11 14:52   ` David Sterba
  2015-09-11 13:24 ` Holger Hoffstätte
       [not found] ` <CAHji1518hEv2_H08ucBYNmvGA2qA=fLs4J+FAFSXEHWv5G8KUA@mail.gmail.com>
  2 siblings, 1 reply; 5+ messages in thread
From: Qu Wenruo @ 2015-09-11  0:27 UTC (permalink / raw)
  To: David Sterba, linux-btrfs



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,
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] btrfs-progs: fix cross stripe boundary check
  2015-09-10 15:02 [PATCH] btrfs-progs: fix cross stripe boundary check David Sterba
  2015-09-11  0:27 ` Qu Wenruo
@ 2015-09-11 13:24 ` Holger Hoffstätte
       [not found] ` <CAHji1518hEv2_H08ucBYNmvGA2qA=fLs4J+FAFSXEHWv5G8KUA@mail.gmail.com>
  2 siblings, 0 replies; 5+ messages in thread
From: Holger Hoffstätte @ 2015-09-11 13:24 UTC (permalink / raw)
  To: linux-btrfs

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] btrfs-progs: fix cross stripe boundary check
       [not found] ` <CAHji1518hEv2_H08ucBYNmvGA2qA=fLs4J+FAFSXEHWv5G8KUA@mail.gmail.com>
@ 2015-09-11 14:45   ` David Sterba
  0 siblings, 0 replies; 5+ messages in thread
From: David Sterba @ 2015-09-11 14:45 UTC (permalink / raw)
  To: Holger Hoffstätte; +Cc: linux-btrfs

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] btrfs-progs: fix cross stripe boundary check
  2015-09-11  0:27 ` Qu Wenruo
@ 2015-09-11 14:52   ` David Sterba
  0 siblings, 0 replies; 5+ messages in thread
From: David Sterba @ 2015-09-11 14:52 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: David Sterba, linux-btrfs

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).

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-09-11 14:53 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-10 15:02 [PATCH] btrfs-progs: fix cross stripe boundary check David Sterba
2015-09-11  0:27 ` Qu Wenruo
2015-09-11 14:52   ` David Sterba
2015-09-11 13:24 ` Holger Hoffstätte
     [not found] ` <CAHji1518hEv2_H08ucBYNmvGA2qA=fLs4J+FAFSXEHWv5G8KUA@mail.gmail.com>
2015-09-11 14:45   ` David Sterba

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.