[PATCH 2/2] btrfs: fix false ENOSPC for btrfs_fallocate()

[Wang Xiaoguang <wangxg.fnst@cn.fujitsu.com>]

Below test scripts can reproduce this false ENOSPC:
	#!/bin/bash
	dd if=/dev/zero of=fs.img bs=$((1024*1024)) count=128
	dev=$(losetup --show -f fs.img)
	mkfs.btrfs -f -M $dev
	mkdir /tmp/mntpoint
	mount /dev/loop0 /tmp/mntpoint
	cd mntpoint
	xfs_io -f -c "falloc 0 $((40*1024*1024))" testfile

Above fallocate(2) operation will fail for ENOSPC reason, but indeed
fs still has free space to satisfy this request. The reason is
btrfs_fallocate() dose not decrease btrfs_space_info's bytes_may_use
just in time, and it calls btrfs_free_reserved_data_space_noquota() in
the end of btrfs_fallocate(), which is too late and have already added
false unnecessary pressure to enospc system. See call graph:
btrfs_fallocate()
|-> btrfs_alloc_data_chunk_ondemand()
    It will add btrfs_space_info's bytes_may_use accordingly.
|-> btrfs_prealloc_file_range()
    It will call btrfs_reserve_extent(), but note that alloc type is
    RESERVE_ALLOC_NO_ACCOUNT, so btrfs_update_reserved_bytes() will
    only increase btrfs_space_info's bytes_reserved accordingly, but
    will not decrease btrfs_space_info's bytes_may_use, then obviously
    we have overestimated real needed disk space, and it'll impact
    other processes who do write(2) or fallocate(2) operations, also
    can impact metadata reservation in mixed mode, and bytes_max_use
    will only be decreased in the end of btrfs_fallocate(). To fix
    this false ENOSPC, we need to decrease btrfs_space_info's
    bytes_may_use in btrfs_prealloc_file_range() in time, as what we
    do in cow_file_range(),
    See call graph in :
    cow_file_range()
    |-> extent_clear_unlock_delalloc()
        |-> clear_extent_bit()
            |-> btrfs_clear_bit_hook()
                |-> btrfs_free_reserved_data_space_noquota()
                    This function will decrease bytes_may_use accordingly.

So this patch choose to call btrfs_free_reserved_data_space() in
__btrfs_prealloc_file_range() for both successful and failed path.

Also this patch removes some old and useless comments.

Signed-off-by: Wang Xiaoguang <wangxg.fnst@cn.fujitsu.com>
---
 fs/btrfs/extent-tree.c |  1 -
 fs/btrfs/file.c        | 23 ++++++++++++-----------
 fs/btrfs/inode-map.c   |  3 +--
 fs/btrfs/inode.c       | 12 ++++++++++++
 fs/btrfs/relocation.c  | 10 +++++++++-
 5 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 82b912a..b0c86d2 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -3490,7 +3490,6 @@ again:
 		dcs = BTRFS_DC_SETUP;
 	else if (ret == -ENOSPC)
 		set_bit(BTRFS_TRANS_CACHE_ENOSPC, &trans->transaction->flags);
-	btrfs_free_reserved_data_space(inode, 0, num_pages);
 
 out_put:
 	iput(inode);
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 2234e88..f872113 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -2669,6 +2669,7 @@ static long btrfs_fallocate(struct file *file, int mode,
 
 	alloc_start = round_down(offset, blocksize);
 	alloc_end = round_up(offset + len, blocksize);
+	cur_offset = alloc_start;
 
 	/* Make sure we aren't being give some crap mode */
 	if (mode & ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE))
@@ -2761,7 +2762,6 @@ static long btrfs_fallocate(struct file *file, int mode,
 
 	/* First, check if we exceed the qgroup limit */
 	INIT_LIST_HEAD(&reserve_list);
-	cur_offset = alloc_start;
 	while (1) {
 		em = btrfs_get_extent(inode, NULL, 0, cur_offset,
 				      alloc_end - cur_offset, 0);
@@ -2788,6 +2788,14 @@ static long btrfs_fallocate(struct file *file, int mode,
 					last_byte - cur_offset);
 			if (ret < 0)
 				break;
+		} else {
+			/*
+			 * Do not need to reserve unwritten extent for this
+			 * range, free reserved data space first, otherwise
+			 * it'll result false ENOSPC error.
+			 */
+			btrfs_free_reserved_data_space(inode, cur_offset,
+				last_byte - cur_offset);
 		}
 		free_extent_map(em);
 		cur_offset = last_byte;
@@ -2839,18 +2847,11 @@ out_unlock:
 	unlock_extent_cached(&BTRFS_I(inode)->io_tree, alloc_start, locked_end,
 			     &cached_state, GFP_KERNEL);
 out:
-	/*
-	 * As we waited the extent range, the data_rsv_map must be empty
-	 * in the range, as written data range will be released from it.
-	 * And for prealloacted extent, it will also be released when
-	 * its metadata is written.
-	 * So this is completely used as cleanup.
-	 */
-	btrfs_qgroup_free_data(inode, alloc_start, alloc_end - alloc_start);
 	inode_unlock(inode);
 	/* Let go of our reservation. */
-	btrfs_free_reserved_data_space(inode, alloc_start,
-				       alloc_end - alloc_start);
+	if (ret != 0)
+		btrfs_free_reserved_data_space(inode, cur_offset,
+					       alloc_end - cur_offset);
 	return ret;
 }
 
diff --git a/fs/btrfs/inode-map.c b/fs/btrfs/inode-map.c
index 70107f7..e59e7d6 100644
--- a/fs/btrfs/inode-map.c
+++ b/fs/btrfs/inode-map.c
@@ -495,10 +495,9 @@ again:
 	ret = btrfs_prealloc_file_range_trans(inode, trans, 0, 0, prealloc,
 					      prealloc, prealloc, &alloc_hint);
 	if (ret) {
-		btrfs_delalloc_release_space(inode, 0, prealloc);
+		btrfs_delalloc_release_metadata(inode, prealloc);
 		goto out_put;
 	}
-	btrfs_free_reserved_data_space(inode, 0, prealloc);
 
 	ret = btrfs_write_out_ino_cache(root, trans, path, inode);
 out_put:
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 4421954..4dc7c838 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -10269,6 +10269,7 @@ static int __btrfs_prealloc_file_range(struct inode *inode, int mode,
 	u64 last_alloc = (u64)-1;
 	int ret = 0;
 	bool own_trans = true;
+	u64 end = start + num_bytes - 1;
 
 	if (trans)
 		own_trans = false;
@@ -10347,6 +10348,7 @@ static int __btrfs_prealloc_file_range(struct inode *inode, int mode,
 		}
 		free_extent_map(em);
 next:
+		btrfs_free_reserved_data_space(inode, cur_offset, ins.offset);
 		num_bytes -= ins.offset;
 		cur_offset += ins.offset;
 		*alloc_hint = ins.objectid + ins.offset;
@@ -10377,9 +10379,18 @@ next:
 		if (own_trans)
 			btrfs_end_transaction(trans, root);
 	}
+
+	if (cur_offset < end)
+		btrfs_free_reserved_data_space(inode, cur_offset,
+			end - cur_offset + 1);
 	return ret;
 }
 
+/*
+ * __btrfs_prealloc_file_range() will call btrfs_free_reserved_data_space()
+ * internally for both sucessful and failed path, btrfs_prealloc_file_range()'s
+ * callers does not need to call btrfs_free_reserved_data_space() any more.
+ */
 int btrfs_prealloc_file_range(struct inode *inode, int mode,
 			      u64 start, u64 num_bytes, u64 min_size,
 			      loff_t actual_len, u64 *alloc_hint)
@@ -10389,6 +10400,7 @@ int btrfs_prealloc_file_range(struct inode *inode, int mode,
 					   NULL);
 }
 
+/* Please see same comments in btrfs_prealloc_file_range() */
 int btrfs_prealloc_file_range_trans(struct inode *inode,
 				    struct btrfs_trans_handle *trans, int mode,
 				    u64 start, u64 num_bytes, u64 min_size,
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index abc2f69..70756fd 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -3031,6 +3031,7 @@ int prealloc_file_extent_cluster(struct inode *inode,
 	int nr = 0;
 	int ret = 0;
 	u64 prealloc_start, prealloc_end;
+	u64 cur_offset;
 
 	BUG_ON(cluster->start != cluster->boundary[0]);
 	inode_lock(inode);
@@ -3041,6 +3042,7 @@ int prealloc_file_extent_cluster(struct inode *inode,
 	if (ret)
 		goto out;
 
+	cur_offset = start;
 	while (nr < cluster->nr) {
 		prealloc_start = cluster->boundary[nr] - offset;
 		if (nr + 1 < cluster->nr)
@@ -3051,16 +3053,22 @@ int prealloc_file_extent_cluster(struct inode *inode,
 		lock_extent(&BTRFS_I(inode)->io_tree, prealloc_start,
 			    prealloc_end);
 		num_bytes = prealloc_end + 1 - prealloc_start;
+		if (cur_offset < start)
+			btrfs_free_reserved_data_space(inode, cur_offset,
+				start - cur_offset);
 		ret = btrfs_prealloc_file_range(inode, 0, prealloc_start,
 						num_bytes, num_bytes,
 						prealloc_end + 1, &alloc_hint);
 		unlock_extent(&BTRFS_I(inode)->io_tree, prealloc_start,
 			      prealloc_end);
+		cur_offset = prealloc_end + 1;
 		if (ret)
 			break;
 		nr++;
 	}
-	btrfs_free_reserved_data_space(inode, start, end + 1 - start);
+	if (cur_offset < end)
+		btrfs_free_reserved_data_space(inode, cur_offset,
+				end + 1 - cur_offset);
 out:
 	inode_unlock(inode);
 	return ret;
-- 
2.9.0