* [PATCH 1/3] ext4: fix incorrect calculate 'reserved' in '__es_remove_extent' when enable bigalloc feature
2022-11-17 1:42 [PATCH 0/3] Fix two issues about bigalloc feature Ye Bin
@ 2022-11-17 1:42 ` Ye Bin
2022-11-17 1:42 ` [PATCH 2/3] ext4: WANR_ON when detect abnormal 'i_reserved_data_blocks' Ye Bin
2022-11-17 1:42 ` [PATCH 3/3] ext4: add check pending tree when evict inode Ye Bin
2 siblings, 0 replies; 6+ messages in thread
From: Ye Bin @ 2022-11-17 1:42 UTC (permalink / raw)
To: tytso, adilger.kernel, linux-ext4
Cc: linux-kernel, jack, Ye Bin, syzbot+05a0f0ccab4a25626e38
From: Ye Bin <yebin10@huawei.com>
Syzbot report issue as follows:
EXT4-fs error (device loop0): ext4_validate_block_bitmap:398: comm rep: bg 0: block 5: invalid block bitmap
EXT4-fs (loop0): Delayed block allocation failed for inode 18 at logical offset 0 with max blocks 32 with error 28
EXT4-fs (loop0): This should not happen!! Data will be lost
EXT4-fs (loop0): Total free blocks count 0
EXT4-fs (loop0): Free/Dirty block details
EXT4-fs (loop0): free_blocks=0
EXT4-fs (loop0): dirty_blocks=32
EXT4-fs (loop0): Block reservation details
EXT4-fs (loop0): i_reserved_data_blocks=2
EXT4-fs (loop0): Inode 18 (00000000845cd634): i_reserved_data_blocks (1) not cleared!
Above issue happens as follows:
Assume:
sbi->s_cluster_ratio = 16
Step1: Insert delay block [0, 31] -> ei->i_reserved_data_blocks=2
Step2:
ext4_writepages
mpage_map_and_submit_extent -> return failed
mpage_release_unused_pages -> to release [0, 30]
ext4_es_remove_extent -> remove lblk=0 end=30
__es_remove_extent -> len1=0 len2=31-30=1
__es_remove_extent:
...
if (len2 > 0) {
...
if (len1 > 0) {
...
} else {
es->es_lblk = end + 1;
es->es_len = len2;
...
}
if (count_reserved)
count_rsvd(inode, lblk, orig_es.es_len - len1 - len2, &orig_es, &rc);
goto out; -> will return but didn't calculate 'reserved'
...
Step3: ext4_destroy_inode -> trigger "i_reserved_data_blocks (1) not cleared!"
To solve above issue if 'len2>0' call 'get_rsvd()' before goto out.
Reported-by: syzbot+05a0f0ccab4a25626e38@syzkaller.appspotmail.com
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
fs/ext4/extents_status.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
index cd0a861853e3..4684eaea9471 100644
--- a/fs/ext4/extents_status.c
+++ b/fs/ext4/extents_status.c
@@ -1371,7 +1371,7 @@ static int __es_remove_extent(struct inode *inode, ext4_lblk_t lblk,
if (count_reserved)
count_rsvd(inode, lblk, orig_es.es_len - len1 - len2,
&orig_es, &rc);
- goto out;
+ goto count;
}
if (len1 > 0) {
@@ -1413,6 +1413,7 @@ static int __es_remove_extent(struct inode *inode, ext4_lblk_t lblk,
}
}
+count:
if (count_reserved)
*reserved = get_rsvd(inode, end, es, &rc);
out:
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/3] ext4: WANR_ON when detect abnormal 'i_reserved_data_blocks'
2022-11-17 1:42 [PATCH 0/3] Fix two issues about bigalloc feature Ye Bin
2022-11-17 1:42 ` [PATCH 1/3] ext4: fix incorrect calculate 'reserved' in '__es_remove_extent' when enable " Ye Bin
@ 2022-11-17 1:42 ` Ye Bin
2022-11-21 9:47 ` Jan Kara
2022-11-17 1:42 ` [PATCH 3/3] ext4: add check pending tree when evict inode Ye Bin
2 siblings, 1 reply; 6+ messages in thread
From: Ye Bin @ 2022-11-17 1:42 UTC (permalink / raw)
To: tytso, adilger.kernel, linux-ext4; +Cc: linux-kernel, jack, Ye Bin
From: Ye Bin <yebin10@huawei.com>
If 'i_reserved_data_blocks' is not cleared which mean something wrong
with code, so emit WARN_ON to capture this abnormal closer to the first
scene.
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
fs/ext4/super.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 63ef74eb8091..30885a6fe18b 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1385,11 +1385,14 @@ static void ext4_destroy_inode(struct inode *inode)
dump_stack();
}
- if (EXT4_I(inode)->i_reserved_data_blocks)
- ext4_msg(inode->i_sb, KERN_ERR,
- "Inode %lu (%p): i_reserved_data_blocks (%u) not cleared!",
- inode->i_ino, EXT4_I(inode),
- EXT4_I(inode)->i_reserved_data_blocks);
+ if (EXT4_I(inode)->i_reserved_data_blocks) {
+ ext4_warning(inode->i_sb, "Inode %lu (%p): "
+ "i_reserved_data_blocks (%u) not cleared!",
+ inode->i_ino, EXT4_I(inode),
+ EXT4_I(inode)->i_reserved_data_blocks);
+
+ WARN_ON(1);
+ }
}
static void init_once(void *foo)
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/3] ext4: WANR_ON when detect abnormal 'i_reserved_data_blocks'
2022-11-17 1:42 ` [PATCH 2/3] ext4: WANR_ON when detect abnormal 'i_reserved_data_blocks' Ye Bin
@ 2022-11-21 9:47 ` Jan Kara
2022-11-21 11:14 ` yebin (H)
0 siblings, 1 reply; 6+ messages in thread
From: Jan Kara @ 2022-11-21 9:47 UTC (permalink / raw)
To: Ye Bin; +Cc: tytso, adilger.kernel, linux-ext4, linux-kernel, jack, Ye Bin
On Thu 17-11-22 09:42:45, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
>
> If 'i_reserved_data_blocks' is not cleared which mean something wrong
> with code, so emit WARN_ON to capture this abnormal closer to the first
> scene.
>
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
> fs/ext4/super.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 63ef74eb8091..30885a6fe18b 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1385,11 +1385,14 @@ static void ext4_destroy_inode(struct inode *inode)
> dump_stack();
> }
>
> - if (EXT4_I(inode)->i_reserved_data_blocks)
> - ext4_msg(inode->i_sb, KERN_ERR,
> - "Inode %lu (%p): i_reserved_data_blocks (%u) not cleared!",
> - inode->i_ino, EXT4_I(inode),
> - EXT4_I(inode)->i_reserved_data_blocks);
> + if (EXT4_I(inode)->i_reserved_data_blocks) {
> + ext4_warning(inode->i_sb, "Inode %lu (%p): "
> + "i_reserved_data_blocks (%u) not cleared!",
> + inode->i_ino, EXT4_I(inode),
> + EXT4_I(inode)->i_reserved_data_blocks);
> +
> + WARN_ON(1);
> + }
Hum, so I'd think that if this happens, the free space accounting is likely
wrong so we might as well just force the filesystem to error mode with
ext4_error() to force fsck? I also gives a good chance to various test
systems to detect there is some problem so we don't need the WARN_ON then?
What do others think?
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/3] ext4: WANR_ON when detect abnormal 'i_reserved_data_blocks'
2022-11-21 9:47 ` Jan Kara
@ 2022-11-21 11:14 ` yebin (H)
0 siblings, 0 replies; 6+ messages in thread
From: yebin (H) @ 2022-11-21 11:14 UTC (permalink / raw)
To: Jan Kara, Ye Bin; +Cc: tytso, adilger.kernel, linux-ext4, linux-kernel
On 2022/11/21 17:47, Jan Kara wrote:
> On Thu 17-11-22 09:42:45, Ye Bin wrote:
>> From: Ye Bin <yebin10@huawei.com>
>>
>> If 'i_reserved_data_blocks' is not cleared which mean something wrong
>> with code, so emit WARN_ON to capture this abnormal closer to the first
>> scene.
>>
>> Signed-off-by: Ye Bin <yebin10@huawei.com>
>> ---
>> fs/ext4/super.c | 13 ++++++++-----
>> 1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
>> index 63ef74eb8091..30885a6fe18b 100644
>> --- a/fs/ext4/super.c
>> +++ b/fs/ext4/super.c
>> @@ -1385,11 +1385,14 @@ static void ext4_destroy_inode(struct inode *inode)
>> dump_stack();
>> }
>>
>> - if (EXT4_I(inode)->i_reserved_data_blocks)
>> - ext4_msg(inode->i_sb, KERN_ERR,
>> - "Inode %lu (%p): i_reserved_data_blocks (%u) not cleared!",
>> - inode->i_ino, EXT4_I(inode),
>> - EXT4_I(inode)->i_reserved_data_blocks);
>> + if (EXT4_I(inode)->i_reserved_data_blocks) {
>> + ext4_warning(inode->i_sb, "Inode %lu (%p): "
>> + "i_reserved_data_blocks (%u) not cleared!",
>> + inode->i_ino, EXT4_I(inode),
>> + EXT4_I(inode)->i_reserved_data_blocks);
>> +
>> + WARN_ON(1);
>> + }
> Hum, so I'd think that if this happens, the free space accounting is likely
> wrong so we might as well just force the filesystem to error mode with
> ext4_error() to force fsck? I also gives a good chance to various test
> systems to detect there is some problem so we don't need the WARN_ON then?
> What do others think?
>
> Honza
Thanks for your advice, use ext4_error() maybe is suitable and also
testable.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/3] ext4: add check pending tree when evict inode
2022-11-17 1:42 [PATCH 0/3] Fix two issues about bigalloc feature Ye Bin
2022-11-17 1:42 ` [PATCH 1/3] ext4: fix incorrect calculate 'reserved' in '__es_remove_extent' when enable " Ye Bin
2022-11-17 1:42 ` [PATCH 2/3] ext4: WANR_ON when detect abnormal 'i_reserved_data_blocks' Ye Bin
@ 2022-11-17 1:42 ` Ye Bin
2 siblings, 0 replies; 6+ messages in thread
From: Ye Bin @ 2022-11-17 1:42 UTC (permalink / raw)
To: tytso, adilger.kernel, linux-ext4
Cc: linux-kernel, jack, Ye Bin, syzbot+05a0f0ccab4a25626e38
From: Ye Bin <yebin10@huawei.com>
Syzbot found the following issue:
BUG: memory leak
unreferenced object 0xffff8881bde17420 (size 32):
comm "rep", pid 2327, jiffies 4295381963 (age 32.265s)
hex dump (first 32 bytes):
01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<00000000ac6d38f8>] __insert_pending+0x13c/0x2d0
[<00000000d717de3b>] ext4_es_insert_delayed_block+0x399/0x4e0
[<000000004be03913>] ext4_da_map_blocks.constprop.0+0x739/0xfa0
[<00000000885a832a>] ext4_da_get_block_prep+0x10c/0x440
[<0000000029b7f8ef>] __block_write_begin_int+0x28d/0x860
[<00000000e182ebc3>] ext4_da_write_inline_data_begin+0x2d1/0xf30
[<00000000ced0c8a2>] ext4_da_write_begin+0x612/0x860
[<000000008d5f27fa>] generic_perform_write+0x215/0x4d0
[<00000000552c1cde>] ext4_buffered_write_iter+0x101/0x3b0
[<0000000052177ae8>] do_iter_readv_writev+0x19f/0x340
[<000000004b9de834>] do_iter_write+0x13b/0x650
[<00000000e2401b9b>] iter_file_splice_write+0x5a5/0xab0
[<0000000023aa5d90>] direct_splice_actor+0x103/0x1e0
[<0000000089e00fc1>] splice_direct_to_actor+0x2c9/0x7b0
[<000000004386851e>] do_splice_direct+0x159/0x280
[<00000000b567e609>] do_sendfile+0x932/0x1200
Above issue fixed by 1b8f787ef547 "ext4: fix warning in 'ext4_da_release_space'"
in this scene. To make things better add check pending tree when evit inode.
To avoid possible memleak free pending tree when check tree isn't cleared.
Reported-by: syzbot+05a0f0ccab4a25626e38@syzkaller.appspotmail.com
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
fs/ext4/extents_status.c | 30 ++++++++++++++++++++++++++++++
fs/ext4/extents_status.h | 1 +
fs/ext4/super.c | 3 +++
3 files changed, 34 insertions(+)
diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
index 4684eaea9471..786f7e39ee09 100644
--- a/fs/ext4/extents_status.c
+++ b/fs/ext4/extents_status.c
@@ -1948,6 +1948,36 @@ void ext4_remove_pending(struct inode *inode, ext4_lblk_t lblk)
write_unlock(&ei->i_es_lock);
}
+void ext4_check_inode_pending(struct inode *inode)
+{
+ struct ext4_inode_info *ei = EXT4_I(inode);
+ struct pending_reservation *pr;
+ struct ext4_pending_tree *tree;
+ struct rb_node *node;
+ int count = 0;
+
+ write_lock(&ei->i_es_lock);
+ tree = &EXT4_I(inode)->i_pending_tree;
+ node = rb_first(&tree->root);
+ while (node) {
+ pr = rb_entry(node, struct pending_reservation, rb_node);
+ node = rb_next(node);
+ rb_erase(&pr->rb_node, &tree->root);
+ kmem_cache_free(ext4_pending_cachep, pr);
+ count++;
+ }
+ write_unlock(&ei->i_es_lock);
+
+ if (!count)
+ return;
+
+ ext4_warning(inode->i_sb, "Inode %lu: pending tree has %d not cleared!",
+ inode->i_ino, count);
+ WARN_ON(1);
+
+ return;
+}
+
/*
* ext4_is_pending - determine whether a cluster has a pending reservation
* on it
diff --git a/fs/ext4/extents_status.h b/fs/ext4/extents_status.h
index 4ec30a798260..631267d45ab2 100644
--- a/fs/ext4/extents_status.h
+++ b/fs/ext4/extents_status.h
@@ -248,6 +248,7 @@ extern int __init ext4_init_pending(void);
extern void ext4_exit_pending(void);
extern void ext4_init_pending_tree(struct ext4_pending_tree *tree);
extern void ext4_remove_pending(struct inode *inode, ext4_lblk_t lblk);
+extern void ext4_check_inode_pending(struct inode *inode);
extern bool ext4_is_pending(struct inode *inode, ext4_lblk_t lblk);
extern int ext4_es_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk,
bool allocated);
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 30885a6fe18b..ae433e1337ed 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1393,6 +1393,9 @@ static void ext4_destroy_inode(struct inode *inode)
WARN_ON(1);
}
+
+ if (EXT4_SB(inode->i_sb)->s_cluster_ratio != 1)
+ ext4_check_inode_pending(inode);
}
static void init_once(void *foo)
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread