[BUG] during balance operation, WARNING: at fs/btrfs/relocation.c:1624 replace_file_extents+0x74b/0x7e0 [btrfs]()

Commit Message

Chris Mason March 5, 2013, 4:40 p.m. UTC

On Tue, Mar 05, 2013 at 08:11:21AM -0700, Chris Mason wrote:
> inux 3.9 RC1.
> > >>
> > >> #!/bin/sh
> > >> mkfs.btrfs -f /dev/sdl /dev/sdk -m raid1 -d raid1 -l 16384
> > >> mount /dev/sdl /mnt
> > >> dd if=/dev/urandom of=/mnt/urandom.1GB bsM count0 &
> > >> dd if=/dev/zero of=/mnt/zero.4GB bsM count@0 &
> > >> (cd ~/kernel-src; tar cf - fs) | (cd /mnt && tar xf -)
> > >> wait
> > >>
> > >> ((cd ~/kernel-src; tar cf - drivers) | (cd /mnt && tar xf -)) &
> > >> sleep 5
> > >> btrfs fi balance start /mnt
> > >
> > > This doesn't look new, are you able to trigger it with an older kernel?
> > >
> >
> > git bisect identifies the following post v3.8 commit to be the one:
> 
> Is your dd running fallocate?  Trying to figure out how this is related.

So the preallocation came from balance, which is preallocating because
it requires us to make an extent exactly the same size as the one we are
replacing.

Zach's commit broke that rule, which means I finally get to send him a
tshirt to celebrate his first btrfs bug.

Looking through all other callers, min_bytes is always either the sector
size or the total allocation requested, so I've done this and pushed it
to for-linus.

Stefan, many thanks for bisecting and testing the patch.

commit 154ea2893002618bc3f9a1e2d8186c65490968b1
Author: Chris Mason <[e-mail]>
Date:   Tue Mar 5 11:11:26 2013 -0500

    Btrfs: enforce min_bytes parameter during extent allocation
    
    Commit 24542bf7ea5e4fdfdb5157ff544c093fa4dcb536 changed preallocation of
    extents to cap the max size we try to allocate.  It's a valid change,
    but the extent reservation code is also used by balance, and that
    can't tolerate a smaller extent being allocated.
    
    __btrfs_prealloc_file_range already has a min_size parameter, which is
    used by relocation to request a specific extent size.  This commit
    adds an extra check to enforce that minimum extent size.
    
    Signed-off-by: Chris Mason <[e-mail]>
    Reported-by: Stefan Behrens <[e-mail]>

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Zach Brown March 5, 2013, 6:43 p.m. UTC | #1

> Zach's commit broke that rule, which means I finally get to send him a
> tshirt to celebrate his first btrfs bug.

Hooray!  XL.

- z
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index ecd9c4c..13ab4de 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8502,6 +8502,7 @@  static int __btrfs_prealloc_file_range(struct inode *inode, int mode,
 	struct btrfs_key ins;
 	u64 cur_offset = start;
 	u64 i_size;
+	u64 cur_bytes;
 	int ret = 0;
 	bool own_trans = true;
 
@@ -8516,8 +8517,9 @@  static int __btrfs_prealloc_file_range(struct inode *inode, int mode,
 			}
 		}
 
-		ret = btrfs_reserve_extent(trans, root,
-					   min(num_bytes, 256ULL * 1024 * 1024),
+		cur_bytes = min(num_bytes, 256ULL * 1024 * 1024);
+		cur_bytes = max(cur_bytes, min_size);
+		ret = btrfs_reserve_extent(trans, root, cur_bytes,
 					   min_size, 0, *alloc_hint, &ins, 1);
 		if (ret) {
 			if (own_trans)