* [PATCH 2/5] Btrfs: don't use global block reservation for inode cache truncation
2013-05-13 13:55 [PATCH 1/5] Btrfs: don't abort the current transaction if there is no enough space for inode cache Miao Xie
@ 2013-05-13 13:55 ` Miao Xie
2013-05-13 13:55 ` [PATCH 3/5] Btrfs: optimize the error handle of use_block_rsv() Miao Xie
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Miao Xie @ 2013-05-13 13:55 UTC (permalink / raw)
To: linux-btrfs; +Cc: Miao Xie, Tsutomu Itoh
It is very likely that there are lots of subvolumes/snapshots in the filesystem,
so if we use global block reservation to do inode cache truncation, we may hog
all the free space that is reserved in global rsv. So it is better that we do
the free space reservation for inode cache truncation by ourselves.
Cc: Tsutomu Itoh <t-itoh@jp.fujitsu.com>
Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
fs/btrfs/extent-tree.c | 5 +++++
fs/btrfs/free-space-cache.c | 39 +++++++++++++++++++--------------------
fs/btrfs/free-space-cache.h | 2 ++
fs/btrfs/inode-map.c | 5 +++--
fs/btrfs/relocation.c | 5 +++++
5 files changed, 34 insertions(+), 22 deletions(-)
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 2305b5c..43afa77 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -3109,6 +3109,11 @@ again:
WARN_ON(ret);
if (i_size_read(inode) > 0) {
+ ret = btrfs_check_trunc_cache_free_space(root,
+ &root->fs_info->global_block_rsv);
+ if (ret)
+ goto out_put;
+
ret = btrfs_truncate_free_space_cache(root, trans, path,
inode);
if (ret)
diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
index ecca6c7..a1948f4 100644
--- a/fs/btrfs/free-space-cache.c
+++ b/fs/btrfs/free-space-cache.c
@@ -197,30 +197,32 @@ int create_free_space_inode(struct btrfs_root *root,
block_group->key.objectid);
}
-int btrfs_truncate_free_space_cache(struct btrfs_root *root,
- struct btrfs_trans_handle *trans,
- struct btrfs_path *path,
- struct inode *inode)
+int btrfs_check_trunc_cache_free_space(struct btrfs_root *root,
+ struct btrfs_block_rsv *rsv)
{
- struct btrfs_block_rsv *rsv;
u64 needed_bytes;
- loff_t oldsize;
- int ret = 0;
-
- rsv = trans->block_rsv;
- trans->block_rsv = &root->fs_info->global_block_rsv;
+ int ret;
/* 1 for slack space, 1 for updating the inode */
needed_bytes = btrfs_calc_trunc_metadata_size(root, 1) +
btrfs_calc_trans_metadata_size(root, 1);
- spin_lock(&trans->block_rsv->lock);
- if (trans->block_rsv->reserved < needed_bytes) {
- spin_unlock(&trans->block_rsv->lock);
- trans->block_rsv = rsv;
- return -ENOSPC;
- }
- spin_unlock(&trans->block_rsv->lock);
+ spin_lock(&rsv->lock);
+ if (rsv->reserved < needed_bytes)
+ ret = -ENOSPC;
+ else
+ ret = 0;
+ spin_unlock(&rsv->lock);
+ return 0;
+}
+
+int btrfs_truncate_free_space_cache(struct btrfs_root *root,
+ struct btrfs_trans_handle *trans,
+ struct btrfs_path *path,
+ struct inode *inode)
+{
+ loff_t oldsize;
+ int ret = 0;
oldsize = i_size_read(inode);
btrfs_i_size_write(inode, 0);
@@ -232,9 +234,7 @@ int btrfs_truncate_free_space_cache(struct btrfs_root *root,
*/
ret = btrfs_truncate_inode_items(trans, root, inode,
0, BTRFS_EXTENT_DATA_KEY);
-
if (ret) {
- trans->block_rsv = rsv;
btrfs_abort_transaction(trans, root, ret);
return ret;
}
@@ -242,7 +242,6 @@ int btrfs_truncate_free_space_cache(struct btrfs_root *root,
ret = btrfs_update_inode(trans, root, inode);
if (ret)
btrfs_abort_transaction(trans, root, ret);
- trans->block_rsv = rsv;
return ret;
}
diff --git a/fs/btrfs/free-space-cache.h b/fs/btrfs/free-space-cache.h
index 4dc17d8..8b7f19f 100644
--- a/fs/btrfs/free-space-cache.h
+++ b/fs/btrfs/free-space-cache.h
@@ -54,6 +54,8 @@ int create_free_space_inode(struct btrfs_root *root,
struct btrfs_block_group_cache *block_group,
struct btrfs_path *path);
+int btrfs_check_trunc_cache_free_space(struct btrfs_root *root,
+ struct btrfs_block_rsv *rsv);
int btrfs_truncate_free_space_cache(struct btrfs_root *root,
struct btrfs_trans_handle *trans,
struct btrfs_path *path,
diff --git a/fs/btrfs/inode-map.c b/fs/btrfs/inode-map.c
index 9818d4a..2c66ddb 100644
--- a/fs/btrfs/inode-map.c
+++ b/fs/btrfs/inode-map.c
@@ -429,11 +429,12 @@ int btrfs_save_ino_cache(struct btrfs_root *root,
num_bytes = trans->bytes_reserved;
/*
* 1 item for inode item insertion if need
- * 3 items for inode item update (in the worst case)
+ * 4 items for inode item update (in the worst case)
+ * 1 items for slack space if we need do truncation
* 1 item for free space object
* 3 items for pre-allocation
*/
- trans->bytes_reserved = btrfs_calc_trans_metadata_size(root, 8);
+ trans->bytes_reserved = btrfs_calc_trans_metadata_size(root, 10);
ret = btrfs_block_rsv_add(root, trans->block_rsv,
trans->bytes_reserved,
BTRFS_RESERVE_NO_FLUSH);
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 704a1b8..f823711 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -3350,6 +3350,11 @@ static int delete_block_group_cache(struct btrfs_fs_info *fs_info,
}
truncate:
+ ret = btrfs_check_trunc_cache_free_space(root,
+ &fs_info->global_block_rsv);
+ if (ret)
+ goto out;
+
path = btrfs_alloc_path();
if (!path) {
ret = -ENOMEM;
--
1.8.0.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/5] Btrfs: optimize the error handle of use_block_rsv()
2013-05-13 13:55 [PATCH 1/5] Btrfs: don't abort the current transaction if there is no enough space for inode cache Miao Xie
2013-05-13 13:55 ` [PATCH 2/5] Btrfs: don't use global block reservation for inode cache truncation Miao Xie
@ 2013-05-13 13:55 ` Miao Xie
2013-05-13 13:55 ` [PATCH 4/5] Btrfs: don't steal the reserved space from the global reserve if their space type is different Miao Xie
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Miao Xie @ 2013-05-13 13:55 UTC (permalink / raw)
To: linux-btrfs; +Cc: Miao Xie, Tsutomu Itoh
cc: Tsutomu Itoh <t-itoh@jp.fujitsu.com>
Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
fs/btrfs/extent-tree.c | 65 ++++++++++++++++++++++----------------------------
1 file changed, 28 insertions(+), 37 deletions(-)
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 43afa77..e34e268 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -6659,48 +6659,39 @@ use_block_rsv(struct btrfs_trans_handle *trans,
block_rsv = get_block_rsv(trans, root);
- if (block_rsv->size == 0) {
- ret = reserve_metadata_bytes(root, block_rsv, blocksize,
- BTRFS_RESERVE_NO_FLUSH);
- /*
- * If we couldn't reserve metadata bytes try and use some from
- * the global reserve.
- */
- if (ret && block_rsv != global_rsv) {
- ret = block_rsv_use_bytes(global_rsv, blocksize);
- if (!ret)
- return global_rsv;
- return ERR_PTR(ret);
- } else if (ret) {
- return ERR_PTR(ret);
- }
- return block_rsv;
- }
+ if (unlikely(block_rsv->size == 0))
+ goto try_reserve;
ret = block_rsv_use_bytes(block_rsv, blocksize);
if (!ret)
return block_rsv;
- if (ret && !block_rsv->failfast) {
- if (btrfs_test_opt(root, ENOSPC_DEBUG)) {
- static DEFINE_RATELIMIT_STATE(_rs,
- DEFAULT_RATELIMIT_INTERVAL * 10,
- /*DEFAULT_RATELIMIT_BURST*/ 1);
- if (__ratelimit(&_rs))
- WARN(1, KERN_DEBUG
- "btrfs: block rsv returned %d\n", ret);
- }
- ret = reserve_metadata_bytes(root, block_rsv, blocksize,
- BTRFS_RESERVE_NO_FLUSH);
- if (!ret) {
- return block_rsv;
- } else if (ret && block_rsv != global_rsv) {
- ret = block_rsv_use_bytes(global_rsv, blocksize);
- if (!ret)
- return global_rsv;
- }
- }
- return ERR_PTR(-ENOSPC);
+ if (block_rsv->failfast)
+ return ERR_PTR(ret);
+
+ if (btrfs_test_opt(root, ENOSPC_DEBUG)) {
+ static DEFINE_RATELIMIT_STATE(_rs,
+ DEFAULT_RATELIMIT_INTERVAL * 10,
+ /*DEFAULT_RATELIMIT_BURST*/ 1);
+ if (__ratelimit(&_rs))
+ WARN(1, KERN_DEBUG
+ "btrfs: block rsv returned %d\n", ret);
+ }
+try_reserve:
+ ret = reserve_metadata_bytes(root, block_rsv, blocksize,
+ BTRFS_RESERVE_NO_FLUSH);
+ if (!ret)
+ return block_rsv;
+ /*
+ * If we couldn't reserve metadata bytes try and use some from
+ * the global reserve.
+ */
+ if (block_rsv->type != BTRFS_BLOCK_RSV_GLOBAL) {
+ ret = block_rsv_use_bytes(global_rsv, blocksize);
+ if (!ret)
+ return global_rsv;
+ }
+ return ERR_PTR(ret);
}
static void unuse_block_rsv(struct btrfs_fs_info *fs_info,
--
1.8.0.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 4/5] Btrfs: don't steal the reserved space from the global reserve if their space type is different
2013-05-13 13:55 [PATCH 1/5] Btrfs: don't abort the current transaction if there is no enough space for inode cache Miao Xie
2013-05-13 13:55 ` [PATCH 2/5] Btrfs: don't use global block reservation for inode cache truncation Miao Xie
2013-05-13 13:55 ` [PATCH 3/5] Btrfs: optimize the error handle of use_block_rsv() Miao Xie
@ 2013-05-13 13:55 ` Miao Xie
2013-05-13 13:55 ` [PATCH 5/5] Btrfs: update the global reserve if it is empty Miao Xie
2013-05-14 6:39 ` [PATCH 1/5] Btrfs: don't abort the current transaction if there is no enough space for inode cache Tsutomu Itoh
4 siblings, 0 replies; 6+ messages in thread
From: Miao Xie @ 2013-05-13 13:55 UTC (permalink / raw)
To: linux-btrfs; +Cc: Miao Xie, Tsutomu Itoh
If the type of the space we need is different with the global reserve, we
can not steal the space from the global reserve, because we can not allocate
the space from the free space cache that the global reserve points to.
Cc: Tsutomu Itoh <t-itoh@jp.fujitsu.com>
Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
fs/btrfs/extent-tree.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index e34e268..c48e1bd 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -6684,9 +6684,11 @@ try_reserve:
return block_rsv;
/*
* If we couldn't reserve metadata bytes try and use some from
- * the global reserve.
+ * the global reserve if its space type is the same as the global
+ * reservation.
*/
- if (block_rsv->type != BTRFS_BLOCK_RSV_GLOBAL) {
+ if (block_rsv->type != BTRFS_BLOCK_RSV_GLOBAL &&
+ block_rsv->space_info == global_rsv->space_info) {
ret = block_rsv_use_bytes(global_rsv, blocksize);
if (!ret)
return global_rsv;
--
1.8.0.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 5/5] Btrfs: update the global reserve if it is empty
2013-05-13 13:55 [PATCH 1/5] Btrfs: don't abort the current transaction if there is no enough space for inode cache Miao Xie
` (2 preceding siblings ...)
2013-05-13 13:55 ` [PATCH 4/5] Btrfs: don't steal the reserved space from the global reserve if their space type is different Miao Xie
@ 2013-05-13 13:55 ` Miao Xie
2013-05-14 6:39 ` [PATCH 1/5] Btrfs: don't abort the current transaction if there is no enough space for inode cache Tsutomu Itoh
4 siblings, 0 replies; 6+ messages in thread
From: Miao Xie @ 2013-05-13 13:55 UTC (permalink / raw)
To: linux-btrfs; +Cc: Miao Xie, Tsutomu Itoh
Before applying this patch, we reserved the space for the global reserve
by the minimum unit if we found it is empty, it was unreasonable and
inefficient, because if the global reserve space was depleted, it implied
that the size of the global reserve was too small. In this case, we shoud
update the global reserve and fill it.
Cc: Tsutomu Itoh <t-itoh@jp.fujitsu.com>
Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
fs/btrfs/extent-tree.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index c48e1bd..c75fe11 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -6656,12 +6656,13 @@ use_block_rsv(struct btrfs_trans_handle *trans,
struct btrfs_block_rsv *block_rsv;
struct btrfs_block_rsv *global_rsv = &root->fs_info->global_block_rsv;
int ret;
+ bool global_updated = false;
block_rsv = get_block_rsv(trans, root);
if (unlikely(block_rsv->size == 0))
goto try_reserve;
-
+again:
ret = block_rsv_use_bytes(block_rsv, blocksize);
if (!ret)
return block_rsv;
@@ -6669,6 +6670,12 @@ use_block_rsv(struct btrfs_trans_handle *trans,
if (block_rsv->failfast)
return ERR_PTR(ret);
+ if (block_rsv->type == BTRFS_BLOCK_RSV_GLOBAL && !global_updated) {
+ global_updated = true;
+ update_global_block_rsv(root->fs_info);
+ goto again;
+ }
+
if (btrfs_test_opt(root, ENOSPC_DEBUG)) {
static DEFINE_RATELIMIT_STATE(_rs,
DEFAULT_RATELIMIT_INTERVAL * 10,
--
1.8.0.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/5] Btrfs: don't abort the current transaction if there is no enough space for inode cache
2013-05-13 13:55 [PATCH 1/5] Btrfs: don't abort the current transaction if there is no enough space for inode cache Miao Xie
` (3 preceding siblings ...)
2013-05-13 13:55 ` [PATCH 5/5] Btrfs: update the global reserve if it is empty Miao Xie
@ 2013-05-14 6:39 ` Tsutomu Itoh
4 siblings, 0 replies; 6+ messages in thread
From: Tsutomu Itoh @ 2013-05-14 6:39 UTC (permalink / raw)
To: Miao Xie; +Cc: linux-btrfs
Hi, Miao,
On 2013/05/13 22:55, Miao Xie wrote:
> The filesystem with inode cache was forced to be read-only when we umounted it.
>
> Steps to reproduce:
> # mkfs.btrfs -f ${DEV}
> # mount -o inode_cache ${DEV} ${MNT}
> # dd if=/dev/zero of=${MNT}/file1 bs=1M count=8192
> # btrfs fi syn ${MNT}
> # dd if=${MNT}/file1 of=/dev/null bs=1M
> # rm -f ${MNT}/file1
> # btrfs fi syn ${MNT}
> # umount ${MNT}
>
> It is because there was no enough space to do inode cache truncation, and then
> we aborted the current transaction.
The problem has not occurred in my testset after applying your patchset.
Your patchset looks good to me. Thanks.
Tested-by: Tsutomu Itoh <t-itoh@jp.fujitsu.com>
>
> But no space error is not a serious problem when we write out the inode cache,
> and it is safe that we just skip this step if we meet this problem. So we need
> not abort the current transaction.
>
> Reported-by: Tsutomu Itoh <t-itoh@jp.fujitsu.com>
> Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
> ---
> fs/btrfs/inode-map.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/inode-map.c b/fs/btrfs/inode-map.c
> index d26f67a..9818d4a 100644
> --- a/fs/btrfs/inode-map.c
> +++ b/fs/btrfs/inode-map.c
> @@ -468,7 +468,8 @@ again:
> if (i_size_read(inode) > 0) {
> ret = btrfs_truncate_free_space_cache(root, trans, path, inode);
> if (ret) {
> - btrfs_abort_transaction(trans, root, ret);
> + if (ret != -ENOSPC)
> + btrfs_abort_transaction(trans, root, ret);
> goto out_put;
> }
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread