Btrfs devel - Apr 2012 - [PATCH] Revert "Btrfs: increase the global block reserve estimates"

This reverts commit 5500cdbe14d7435e04f66ff3cfb8ecd8b8e44ebf.

We had numerous reports of premature ENOSPC that were bisected to this
patch. Reverting will not break things but a warning in
''use_block_rsv''
may show up in the syslog.

There''s no alternative fix in sight and the ENOSPC problem affects all
3.3 btrfs users during normal filesystem use.

CC: stable@vger.kernel.org
CC: Liu Bo <liubo2009@cn.fujitsu.com>
CC: Chris Mason <chris.mason@oracle.com>
Signed-off-by: David Sterba <dsterba@suse.cz>
---
 fs/btrfs/extent-tree.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index a844204..ace5e8c 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -4205,7 +4205,7 @@ static u64 calc_global_metadata_size(struct btrfs_fs_info
*fs_info)
 	num_bytes += div64_u64(data_used + meta_used, 50);
 
 	if (num_bytes * 3 > meta_used)
-		num_bytes = div64_u64(meta_used, 3) * 2;
+		num_bytes = div64_u64(meta_used, 3);
 
 	return ALIGN(num_bytes, fs_info->extent_root->leafsize << 10);
 }
-- 
1.7.9

--
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 04/09/2012 07:40 AM, David Sterba wrote:
> This reverts commit 5500cdbe14d7435e04f66ff3cfb8ecd8b8e44ebf.
> 
> We had numerous reports of premature ENOSPC that were bisected to this
> patch. Reverting will not break things but a warning in
''use_block_rsv''
> may show up in the syslog.
> 
> There''s no alternative fix in sight and the ENOSPC problem affects
all
> 3.3 btrfs users during normal filesystem use.
> 
Hi David,

(add cc Josef)

The whole thing is about our overcommit stuff, that is,
since we are not able to get _precise_ number of reservation right now, we
usually
reserve more than what we need.
For this, we''ve done overcommit dance (thanks for Josef''s
work!) but it''s still not
enough for our reservation when we still have some disk space.

I''m ok with this revert, but since we don''t use up all the
reserved space in most time,
I assume the following can be an alternative, thanks,

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index a844204..1b692c2 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -3844,6 +3844,7 @@ again:
 
 		spin_lock(&root->fs_info->free_chunk_lock);
 		avail = root->fs_info->free_chunk_space;
+		spin_unlock(&root->fs_info->free_chunk_lock);
 
 		/*
 		 * If we have dup, raid1 or raid10 then only half of the free
@@ -3854,17 +3855,6 @@ again:
 			       BTRFS_BLOCK_GROUP_RAID10))
 			avail >>= 1;
 
-		/*
-		 * If we aren''t flushing don''t let us overcommit too much,
say
-		 * 1/8th of the space.  If we can flush, let it overcommit up to
-		 * 1/2 of the space.
-		 */
-		if (flush)
-			avail >>= 3;
-		else
-			avail >>= 1;
-		 spin_unlock(&root->fs_info->free_chunk_lock);
-
 		if (used + num_bytes < space_info->total_bytes + avail) {
 			space_info->bytes_may_use += orig_bytes;
 			trace_btrfs_space_reservation(root->fs_info,
--
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 Mon, Apr 09, 2012 at 09:37:08AM +0800, Liu Bo wrote:
> The whole thing is about our overcommit stuff, that is,
> since we are not able to get _precise_ number of reservation right now, we
usually
> reserve more than what we need.
> For this, we''ve done overcommit dance (thanks for Josef''s
work!) but it''s still not
> enough for our reservation when we still have some disk space.
>
> I''m ok with this revert, but since we don''t use up all
the reserved space in most time,
> I assume the following can be an alternative, thanks,
Unfortunatelly the situation looks not good for 3.3 btrfs users, I''m
looking for something we know that somehow (ie like in 3.2) works and
are able to submit to the stable tree very soon. I''ll definitelly test
your alternative and if Chris applies it during the -rc phase we''ll
have
more time to verify it or you/Josef fix it in another way.


thanks,
david
--
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 Tue, Apr 10, 2012 at 12:40:08AM +0200, David Sterba
wrote:
> On Mon, Apr 09, 2012 at 09:37:08AM +0800, Liu Bo wrote:
> > The whole thing is about our overcommit stuff, that is,
> > since we are not able to get _precise_ number of reservation right
now, we usually
> > reserve more than what we need.
> > For this, we''ve done overcommit dance (thanks for
Josef''s work!) but it''s still not
> > enough for our reservation when we still have some disk space.
> >
> > I''m ok with this revert, but since we don''t use up
all the reserved space in most time,
> > I assume the following can be an alternative, thanks,
> 
> Unfortunatelly the situation looks not good for 3.3 btrfs users,
I''m
> looking for something we know that somehow (ie like in 3.2) works and
> are able to submit to the stable tree very soon. I''ll definitelly
test
> your alternative and if Chris applies it during the -rc phase
we''ll have
> more time to verify it or you/Josef fix it in another way.
Lets do the alternative of reverting the patch and dropping the warning.
I''ve been trying to find a small and simple patch for that but I
don''t
think that''s going to happen for a stable commit.

-chris

--
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 Mon, Apr 09, 2012 at 09:37:08AM +0800, Liu Bo wrote:
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index a844204..1b692c2 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -3844,6 +3844,7 @@ again:
>  
>  		spin_lock(&root->fs_info->free_chunk_lock);
>  		avail = root->fs_info->free_chunk_space;
> +		spin_unlock(&root->fs_info->free_chunk_lock);
>  
>  		/*
>  		 * If we have dup, raid1 or raid10 then only half of the free
> @@ -3854,17 +3855,6 @@ again:
>  			       BTRFS_BLOCK_GROUP_RAID10))
>  			avail >>= 1;
>  
> -		/*
> -		 * If we aren''t flushing don''t let us overcommit too
much, say
> -		 * 1/8th of the space.  If we can flush, let it overcommit up to
> -		 * 1/2 of the space.
> -		 */
> -		if (flush)
> -			avail >>= 3;
> -		else
> -			avail >>= 1;
> -		 spin_unlock(&root->fs_info->free_chunk_lock);
> -
>  		if (used + num_bytes < space_info->total_bytes + avail) {
>  			space_info->bytes_may_use += orig_bytes;
>  			trace_btrfs_space_reservation(root->fs_info,
Hangs during xfstests/013 with freshly made fs with just -o space_cache
mount:

$ btrfs fi df /mnt/a1
Data, RAID0: total=4.00GB, used=652.00KB
Data: total=8.00MB, used=0.00
System, RAID1: total=8.00MB, used=4.00KB
System: total=4.00MB, used=0.00
Metadata, RAID1: total=1.00GB, used=52.00KB
Metadata: total=8.00MB, used=0.00

/root/xfstests/ltp/fsstress -r -v -m 8 -n 1000 -d /mnt/a1/fsstress.26492.1

[<ffffffffa004e845>] btrfs_start_ordered_extent+0x85/0x140 [btrfs]
[<ffffffffa004ea44>] btrfs_wait_ordered_extents+0x144/0x230 [btrfs]
[<ffffffffa0004962>] btrfs_sync_fs+0x52/0x100 [btrfs]
[<ffffffff81172983>] __sync_filesystem+0x63/0x90
[<ffffffff811729cf>] sync_one_sb+0x1f/0x30
[<ffffffff81147611>] iterate_supers+0xf1/0x100
[<ffffffff811728d0>] sync_filesystems+0x20/0x30
[<ffffffff81172a76>] sys_sync+0x26/0x40
[<ffffffff818a65a9>] system_call_fastpath+0x16/0x1b
[<ffffffffffffffff>] 0xffffffffffffffff
--
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