[PATCH 1/2] btrfs: improve inode's outstanding_extents computation
This issue was revealed by modifying BTRFS_MAX_EXTENT_SIZE(128MB) to 64KB,
When modifying BTRFS_MAX_EXTENT_SIZE(128MB) to 64KB, fsstress test often
gets these warnings from btrfs_destroy_inode():
WARN_ON(BTRFS_I(inode)->outstanding_extents);
WARN_ON(BTRFS_I(inode)->reserved_extents);
Simple test program below can reproduce this issue steadily.
Note: you need to modify BTRFS_MAX_EXTENT_SIZE to 64KB to have test,
otherwise there won't be such WARNING.
#include
#include
#include
#include
#include
int main(void)
{
int fd;
char buf[68 *1024];
memset(buf, 0, 68 * 1024);
fd = open("testfile", O_CREAT | O_EXCL | O_RDWR);
pwrite(fd, buf, 68 * 1024, 64 * 1024);
return;
}
When BTRFS_MAX_EXTENT_SIZE is 64KB, and buffered data range is:
64KB128K132KB
|---|---|
64 + 4KB
1) for above data range, btrfs_delalloc_reserve_metadata() will reserve
metadata and set BTRFS_I(inode)->outstanding_extents to 2.
(68KB + 64KB - 1) / 64KB == 2
Outstanding_extents: 2
2) then btrfs_dirty_page() will be called to dirty pages and set
EXTENT_DELALLOC flag. In this case, btrfs_set_bit_hook() will be called
twice.
The 1st set_bit_hook() call will set DEALLOC flag for the first 64K.
64KB128KB
|---|
64KB DELALLOC
Outstanding_extents: 2
Set_bit_hooks() uses FIRST_DELALLOC flag to avoid re-increase
outstanding_extents counter.
So for 1st set_bit_hooks() call, it won't modify outstanding_extents,
it's still 2.
Then FIRST_DELALLOC flag is *CLEARED*.
3) 2nd btrfs_set_bit_hook() call.
Because FIRST_DELALLOC have been cleared by previous set_bit_hook(),
btrfs_set_bit_hook() will increase BTRFS_I(inode)->outstanding_extents by
one, so now BTRFS_I(inode)->outstanding_extents is 3.
64KB128KB132KB
|---||
64K DELALLOC 4K DELALLOC
Outstanding_extents: 3
But the correct outstanding_extents number should be 2, not 3.
The 2nd btrfs_set_bit_hook() call just screwed up this, and leads to the
WARN_ON().
Normally, we can solve it by only increasing outstanding_extents in
set_bit_hook().
But the problem is for delalloc_reserve/release_metadata(), we only have
a 'length' parameter, and calculate in-accurate outstanding_extents.
If we only rely on set_bit_hook() release_metadata() will crew things up
as it will decrease inaccurate number.
So the fix we use is:
1) Increase *INACCURATE* outstanding_extents at delalloc_reserve_meta
Just as a place holder.
2) Increase *accurate* outstanding_extents at set_bit_hooks()
This is the real increaser.
3) Decrease *INACCURATE* outstanding_extents before returning
This makes outstanding_extents to correct value.
For 128M BTRFS_MAX_EXTENT_SIZE, due to limitation of
__btrfs_buffered_write(), each iteration will only handle about 2MB
data.
So btrfs_dirty_pages() won't need to handle cases cross 2 extents.
Signed-off-by: Wang Xiaoguang
Tested-by: Holger Hoffstätte
Tested-by: Stefan Priebe
---
fs/btrfs/ctree.h | 2 ++
fs/btrfs/inode.c | 65 ++--
fs/btrfs/ioctl.c | 6 ++
3 files changed, 62 insertions(+), 11 deletions(-)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 0b8ce2b..52a8535 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3139,6 +3139,8 @@ int btrfs_start_delalloc_roots(struct btrfs_fs_info
*fs_info, int delay_iput,
int nr);
int btrfs_set_extent_delalloc(struct inode *inode, u64 start, u64 end,
struct extent_state **cached_state, int dedupe);
+int btrfs_set_extent_defrag(struct inode *inode, u64 start, u64 end,
+ struct extent_state **cached_state);
int btrfs_create_subvol_root(struct btrfs_trans_handle *trans,
struct btrfs_root *new_root,
struct btrfs_root *parent_root,
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 2b790bd..e427b81 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -1601,6 +1601,9 @@ static void btrfs_split_extent_hook(struct inode *inode,
if (!(orig->state & EXTENT_DELALLOC))
return;
+ if (btrfs_is_free_space_inode(inode))
+ return;
+
size = orig->end - orig->start + 1;
if (size > BTRFS_MAX_EXTENT_SIZE) {
u64 num_extents;
@@ -1643,6 +1646,9 @@ static void btrfs_merge_extent_hook(struct inode *inode,
if (!(other->state &
Re: [PATCH 1/2] btrfs: improve inode's outstanding_extents computation
Hello list,
just want to report again that i've seen not a single ENOSPC msg with
this series applied.
Now working fine since 18 days.
Stefan
Am 14.10.2016 um 15:09 schrieb Stefan Priebe - Profihost AG:
>
> Am 06.10.2016 um 04:51 schrieb Wang Xiaoguang:
>> This issue was revealed by modifying BTRFS_MAX_EXTENT_SIZE(128MB) to 64KB,
>> When modifying BTRFS_MAX_EXTENT_SIZE(128MB) to 64KB, fsstress test often
>> gets these warnings from btrfs_destroy_inode():
>> WARN_ON(BTRFS_I(inode)->outstanding_extents);
>> WARN_ON(BTRFS_I(inode)->reserved_extents);
>>
>> Simple test program below can reproduce this issue steadily.
>> Note: you need to modify BTRFS_MAX_EXTENT_SIZE to 64KB to have test,
>> otherwise there won't be such WARNING.
>> #include
>> #include
>> #include
>> #include
>> #include
>>
>> int main(void)
>> {
>> int fd;
>> char buf[68 *1024];
>>
>> memset(buf, 0, 68 * 1024);
>> fd = open("testfile", O_CREAT | O_EXCL | O_RDWR);
>> pwrite(fd, buf, 68 * 1024, 64 * 1024);
>> return;
>> }
>>
>> When BTRFS_MAX_EXTENT_SIZE is 64KB, and buffered data range is:
>> 64KB 128K132KB
>> |---|---|
>> 64 + 4KB
>>
>> 1) for above data range, btrfs_delalloc_reserve_metadata() will reserve
>> metadata and set BTRFS_I(inode)->outstanding_extents to 2.
>> (68KB + 64KB - 1) / 64KB == 2
>>
>> Outstanding_extents: 2
>>
>> 2) then btrfs_dirty_page() will be called to dirty pages and set
>> EXTENT_DELALLOC flag. In this case, btrfs_set_bit_hook() will be called
>> twice.
>> The 1st set_bit_hook() call will set DEALLOC flag for the first 64K.
>> 64KB 128KB
>> |---|
>> 64KB DELALLOC
>> Outstanding_extents: 2
>>
>> Set_bit_hooks() uses FIRST_DELALLOC flag to avoid re-increase
>> outstanding_extents counter.
>> So for 1st set_bit_hooks() call, it won't modify outstanding_extents,
>> it's still 2.
>>
>> Then FIRST_DELALLOC flag is *CLEARED*.
>>
>> 3) 2nd btrfs_set_bit_hook() call.
>> Because FIRST_DELALLOC have been cleared by previous set_bit_hook(),
>> btrfs_set_bit_hook() will increase BTRFS_I(inode)->outstanding_extents by
>> one, so now BTRFS_I(inode)->outstanding_extents is 3.
>> 64KB128KB132KB
>> |---||
>> 64K DELALLOC 4K DELALLOC
>> Outstanding_extents: 3
>>
>> But the correct outstanding_extents number should be 2, not 3.
>> The 2nd btrfs_set_bit_hook() call just screwed up this, and leads to the
>> WARN_ON().
>>
>> Normally, we can solve it by only increasing outstanding_extents in
>> set_bit_hook().
>> But the problem is for delalloc_reserve/release_metadata(), we only have
>> a 'length' parameter, and calculate in-accurate outstanding_extents.
>> If we only rely on set_bit_hook() release_metadata() will crew things up
>> as it will decrease inaccurate number.
>>
>> So the fix we use is:
>> 1) Increase *INACCURATE* outstanding_extents at delalloc_reserve_meta
>>Just as a place holder.
>> 2) Increase *accurate* outstanding_extents at set_bit_hooks()
>>This is the real increaser.
>> 3) Decrease *INACCURATE* outstanding_extents before returning
>>This makes outstanding_extents to correct value.
>>
>> For 128M BTRFS_MAX_EXTENT_SIZE, due to limitation of
>> __btrfs_buffered_write(), each iteration will only handle about 2MB
>> data.
>> So btrfs_dirty_pages() won't need to handle cases cross 2 extents.
>>
>> Signed-off-by: Wang Xiaoguang
>
> Tested-by: Stefan Priebe
>
> Works fine since 8 days - no ENOSPC errors anymore.
>
> Greets,
> Stefan
>
>> ---
>> fs/btrfs/ctree.h | 2 ++
>> fs/btrfs/inode.c | 65
>> ++--
>> fs/btrfs/ioctl.c | 6 ++
>> 3 files changed, 62 insertions(+), 11 deletions(-)
>>
>> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
>> index 33fe035..16885f6 100644
>> --- a/fs/btrfs/ctree.h
>> +++ b/fs/btrfs/ctree.h
>> @@ -3119,6 +3119,8 @@ int btrfs_start_delalloc_roots(struct btrfs_fs_info
>> *fs_info, int delay_iput,
>> int nr);
>> int btrfs_set_extent_delalloc(struct inode *inode, u64 start, u64 end,
>>struct extent_state **cached_state);
>> +int btrfs_set_extent_defrag(struct inode *inode, u64 start, u64 end,
>> +struct extent_state **cached_state);
>> int btrfs_create_subvol_root(struct btrfs_trans_handle *trans,
>> struct btrfs_root *new_root,
>> struct btrfs_root *parent_root,
>> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
>> index
Re: [PATCH 1/2] btrfs: improve inode's outstanding_extents computation
Am 06.10.2016 um 04:51 schrieb Wang Xiaoguang:
> This issue was revealed by modifying BTRFS_MAX_EXTENT_SIZE(128MB) to 64KB,
> When modifying BTRFS_MAX_EXTENT_SIZE(128MB) to 64KB, fsstress test often
> gets these warnings from btrfs_destroy_inode():
> WARN_ON(BTRFS_I(inode)->outstanding_extents);
> WARN_ON(BTRFS_I(inode)->reserved_extents);
>
> Simple test program below can reproduce this issue steadily.
> Note: you need to modify BTRFS_MAX_EXTENT_SIZE to 64KB to have test,
> otherwise there won't be such WARNING.
> #include
> #include
> #include
> #include
> #include
>
> int main(void)
> {
> int fd;
> char buf[68 *1024];
>
> memset(buf, 0, 68 * 1024);
> fd = open("testfile", O_CREAT | O_EXCL | O_RDWR);
> pwrite(fd, buf, 68 * 1024, 64 * 1024);
> return;
> }
>
> When BTRFS_MAX_EXTENT_SIZE is 64KB, and buffered data range is:
> 64KB 128K132KB
> |---|---|
> 64 + 4KB
>
> 1) for above data range, btrfs_delalloc_reserve_metadata() will reserve
> metadata and set BTRFS_I(inode)->outstanding_extents to 2.
> (68KB + 64KB - 1) / 64KB == 2
>
> Outstanding_extents: 2
>
> 2) then btrfs_dirty_page() will be called to dirty pages and set
> EXTENT_DELALLOC flag. In this case, btrfs_set_bit_hook() will be called
> twice.
> The 1st set_bit_hook() call will set DEALLOC flag for the first 64K.
> 64KB 128KB
> |---|
> 64KB DELALLOC
> Outstanding_extents: 2
>
> Set_bit_hooks() uses FIRST_DELALLOC flag to avoid re-increase
> outstanding_extents counter.
> So for 1st set_bit_hooks() call, it won't modify outstanding_extents,
> it's still 2.
>
> Then FIRST_DELALLOC flag is *CLEARED*.
>
> 3) 2nd btrfs_set_bit_hook() call.
> Because FIRST_DELALLOC have been cleared by previous set_bit_hook(),
> btrfs_set_bit_hook() will increase BTRFS_I(inode)->outstanding_extents by
> one, so now BTRFS_I(inode)->outstanding_extents is 3.
> 64KB128KB132KB
> |---||
> 64K DELALLOC 4K DELALLOC
> Outstanding_extents: 3
>
> But the correct outstanding_extents number should be 2, not 3.
> The 2nd btrfs_set_bit_hook() call just screwed up this, and leads to the
> WARN_ON().
>
> Normally, we can solve it by only increasing outstanding_extents in
> set_bit_hook().
> But the problem is for delalloc_reserve/release_metadata(), we only have
> a 'length' parameter, and calculate in-accurate outstanding_extents.
> If we only rely on set_bit_hook() release_metadata() will crew things up
> as it will decrease inaccurate number.
>
> So the fix we use is:
> 1) Increase *INACCURATE* outstanding_extents at delalloc_reserve_meta
>Just as a place holder.
> 2) Increase *accurate* outstanding_extents at set_bit_hooks()
>This is the real increaser.
> 3) Decrease *INACCURATE* outstanding_extents before returning
>This makes outstanding_extents to correct value.
>
> For 128M BTRFS_MAX_EXTENT_SIZE, due to limitation of
> __btrfs_buffered_write(), each iteration will only handle about 2MB
> data.
> So btrfs_dirty_pages() won't need to handle cases cross 2 extents.
>
> Signed-off-by: Wang Xiaoguang
Tested-by: Stefan Priebe
Works fine since 8 days - no ENOSPC errors anymore.
Greets,
Stefan
> ---
> fs/btrfs/ctree.h | 2 ++
> fs/btrfs/inode.c | 65
> ++--
> fs/btrfs/ioctl.c | 6 ++
> 3 files changed, 62 insertions(+), 11 deletions(-)
>
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 33fe035..16885f6 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -3119,6 +3119,8 @@ int btrfs_start_delalloc_roots(struct btrfs_fs_info
> *fs_info, int delay_iput,
> int nr);
> int btrfs_set_extent_delalloc(struct inode *inode, u64 start, u64 end,
> struct extent_state **cached_state);
> +int btrfs_set_extent_defrag(struct inode *inode, u64 start, u64 end,
> + struct extent_state **cached_state);
> int btrfs_create_subvol_root(struct btrfs_trans_handle *trans,
>struct btrfs_root *new_root,
>struct btrfs_root *parent_root,
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index e6811c4..a7193b1 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -1590,6 +1590,9 @@ static void btrfs_split_extent_hook(struct inode *inode,
> if (!(orig->state & EXTENT_DELALLOC))
> return;
>
> + if (btrfs_is_free_space_inode(inode))
> + return;
> +
>
[PATCH 1/2] btrfs: improve inode's outstanding_extents computation
This issue was revealed by modifying BTRFS_MAX_EXTENT_SIZE(128MB) to 64KB,
When modifying BTRFS_MAX_EXTENT_SIZE(128MB) to 64KB, fsstress test often
gets these warnings from btrfs_destroy_inode():
WARN_ON(BTRFS_I(inode)->outstanding_extents);
WARN_ON(BTRFS_I(inode)->reserved_extents);
Simple test program below can reproduce this issue steadily.
Note: you need to modify BTRFS_MAX_EXTENT_SIZE to 64KB to have test,
otherwise there won't be such WARNING.
#include
#include
#include
#include
#include
int main(void)
{
int fd;
char buf[68 *1024];
memset(buf, 0, 68 * 1024);
fd = open("testfile", O_CREAT | O_EXCL | O_RDWR);
pwrite(fd, buf, 68 * 1024, 64 * 1024);
return;
}
When BTRFS_MAX_EXTENT_SIZE is 64KB, and buffered data range is:
64KB128K132KB
|---|---|
64 + 4KB
1) for above data range, btrfs_delalloc_reserve_metadata() will reserve
metadata and set BTRFS_I(inode)->outstanding_extents to 2.
(68KB + 64KB - 1) / 64KB == 2
Outstanding_extents: 2
2) then btrfs_dirty_page() will be called to dirty pages and set
EXTENT_DELALLOC flag. In this case, btrfs_set_bit_hook() will be called
twice.
The 1st set_bit_hook() call will set DEALLOC flag for the first 64K.
64KB128KB
|---|
64KB DELALLOC
Outstanding_extents: 2
Set_bit_hooks() uses FIRST_DELALLOC flag to avoid re-increase
outstanding_extents counter.
So for 1st set_bit_hooks() call, it won't modify outstanding_extents,
it's still 2.
Then FIRST_DELALLOC flag is *CLEARED*.
3) 2nd btrfs_set_bit_hook() call.
Because FIRST_DELALLOC have been cleared by previous set_bit_hook(),
btrfs_set_bit_hook() will increase BTRFS_I(inode)->outstanding_extents by
one, so now BTRFS_I(inode)->outstanding_extents is 3.
64KB128KB132KB
|---||
64K DELALLOC 4K DELALLOC
Outstanding_extents: 3
But the correct outstanding_extents number should be 2, not 3.
The 2nd btrfs_set_bit_hook() call just screwed up this, and leads to the
WARN_ON().
Normally, we can solve it by only increasing outstanding_extents in
set_bit_hook().
But the problem is for delalloc_reserve/release_metadata(), we only have
a 'length' parameter, and calculate in-accurate outstanding_extents.
If we only rely on set_bit_hook() release_metadata() will crew things up
as it will decrease inaccurate number.
So the fix we use is:
1) Increase *INACCURATE* outstanding_extents at delalloc_reserve_meta
Just as a place holder.
2) Increase *accurate* outstanding_extents at set_bit_hooks()
This is the real increaser.
3) Decrease *INACCURATE* outstanding_extents before returning
This makes outstanding_extents to correct value.
For 128M BTRFS_MAX_EXTENT_SIZE, due to limitation of
__btrfs_buffered_write(), each iteration will only handle about 2MB
data.
So btrfs_dirty_pages() won't need to handle cases cross 2 extents.
Signed-off-by: Wang Xiaoguang
---
fs/btrfs/ctree.h | 2 ++
fs/btrfs/inode.c | 65 ++--
fs/btrfs/ioctl.c | 6 ++
3 files changed, 62 insertions(+), 11 deletions(-)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 33fe035..16885f6 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3119,6 +3119,8 @@ int btrfs_start_delalloc_roots(struct btrfs_fs_info
*fs_info, int delay_iput,
int nr);
int btrfs_set_extent_delalloc(struct inode *inode, u64 start, u64 end,
struct extent_state **cached_state);
+int btrfs_set_extent_defrag(struct inode *inode, u64 start, u64 end,
+ struct extent_state **cached_state);
int btrfs_create_subvol_root(struct btrfs_trans_handle *trans,
struct btrfs_root *new_root,
struct btrfs_root *parent_root,
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index e6811c4..a7193b1 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -1590,6 +1590,9 @@ static void btrfs_split_extent_hook(struct inode *inode,
if (!(orig->state & EXTENT_DELALLOC))
return;
+ if (btrfs_is_free_space_inode(inode))
+ return;
+
size = orig->end - orig->start + 1;
if (size > BTRFS_MAX_EXTENT_SIZE) {
u64 num_extents;
@@ -1632,6 +1635,9 @@ static void btrfs_merge_extent_hook(struct inode *inode,
if (!(other->state & EXTENT_DELALLOC))
return;
+ if (btrfs_is_free_space_inode(inode))
+ return;
+
if
