From: Baokun Li <libaokun1@huawei.com>
To: <linux-ext4@vger.kernel.org>
Cc: <tytso@mit.edu>, <adilger.kernel@dilger.ca>, <jack@suse.cz>,
<ritesh.list@gmail.com>, <linux-kernel@vger.kernel.org>,
<yi.zhang@huawei.com>, <yangerkun@huawei.com>,
<yukuai3@huawei.com>, <libaokun1@huawei.com>
Subject: [PATCH v4 01/12] ext4: only update i_reserved_data_blocks on successful block allocation
Date: Mon, 24 Apr 2023 11:38:35 +0800 [thread overview]
Message-ID: <20230424033846.4732-2-libaokun1@huawei.com> (raw)
In-Reply-To: <20230424033846.4732-1-libaokun1@huawei.com>
In our fault injection test, we create an ext4 file, migrate it to
non-extent based file, then punch a hole and finally trigger a WARN_ON
in the ext4_da_update_reserve_space():
EXT4-fs warning (device sda): ext4_da_update_reserve_space:369:
ino 14, used 11 with only 10 reserved data blocks
When writing back a non-extent based file, if we enable delalloc, the
number of reserved blocks will be subtracted from the number of blocks
mapped by ext4_ind_map_blocks(), and the extent status tree will be
updated. We update the extent status tree by first removing the old
extent_status and then inserting the new extent_status. If the block range
we remove happens to be in an extent, then we need to allocate another
extent_status with ext4_es_alloc_extent().
use old to remove to add new
|----------|------------|------------|
old extent_status
The problem is that the allocation of a new extent_status failed due to a
fault injection, and __es_shrink() did not get free memory, resulting in
a return of -ENOMEM. Then do_writepages() retries after receiving -ENOMEM,
we map to the same extent again, and the number of reserved blocks is again
subtracted from the number of blocks in that extent. Since the blocks in
the same extent are subtracted twice, we end up triggering WARN_ON at
ext4_da_update_reserve_space() because used > ei->i_reserved_data_blocks.
For non-extent based file, we update the number of reserved blocks after
ext4_ind_map_blocks() is executed, which causes a problem that when we call
ext4_ind_map_blocks() to create a block, it doesn't always create a block,
but we always reduce the number of reserved blocks. So we move the logic
for updating reserved blocks to ext4_ind_map_blocks() to ensure that the
number of reserved blocks is updated only after we do succeed in allocating
some new blocks.
Fixes: 5f634d064c70 ("ext4: Fix quota accounting error with fallocate")
Signed-off-by: Baokun Li <libaokun1@huawei.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---
fs/ext4/indirect.c | 8 ++++++++
fs/ext4/inode.c | 10 ----------
2 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
index c68bebe7ff4b..a9f3716119d3 100644
--- a/fs/ext4/indirect.c
+++ b/fs/ext4/indirect.c
@@ -651,6 +651,14 @@ int ext4_ind_map_blocks(handle_t *handle, struct inode *inode,
ext4_update_inode_fsync_trans(handle, inode, 1);
count = ar.len;
+
+ /*
+ * Update reserved blocks/metadata blocks after successful block
+ * allocation which had been deferred till now.
+ */
+ if (flags & EXT4_GET_BLOCKS_DELALLOC_RESERVE)
+ ext4_da_update_reserve_space(inode, count, 1);
+
got_it:
map->m_flags |= EXT4_MAP_MAPPED;
map->m_pblk = le32_to_cpu(chain[depth-1].key);
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 974747a6eb99..20de04399c8b 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -659,16 +659,6 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
*/
ext4_clear_inode_state(inode, EXT4_STATE_EXT_MIGRATE);
}
-
- /*
- * Update reserved blocks/metadata blocks after successful
- * block allocation which had been deferred till now. We don't
- * support fallocate for non extent files. So we can update
- * reserve space here.
- */
- if ((retval > 0) &&
- (flags & EXT4_GET_BLOCKS_DELALLOC_RESERVE))
- ext4_da_update_reserve_space(inode, retval, 1);
}
if (retval > 0) {
--
2.31.1
next prev parent reply other threads:[~2023-04-24 3:39 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-24 3:38 [PATCH v4 00/12] ext4: fix WARNING in ext4_da_update_reserve_space Baokun Li
2023-04-24 3:38 ` Baokun Li [this message]
2023-04-24 3:38 ` [PATCH v4 02/12] ext4: add a new helper to check if es must be kept Baokun Li
2023-05-03 12:57 ` Jan Kara
2023-04-24 3:38 ` [PATCH v4 03/12] ext4: factor out __es_alloc_extent() and __es_free_extent() Baokun Li
2023-05-03 14:28 ` Jan Kara
2023-04-24 3:38 ` [PATCH v4 04/12] ext4: use pre-allocated es in __es_insert_extent() Baokun Li
2023-05-03 14:28 ` Jan Kara
2023-04-24 3:38 ` [PATCH v4 05/12] ext4: use pre-allocated es in __es_remove_extent() Baokun Li
2023-05-03 14:29 ` Jan Kara
2023-04-24 3:38 ` [PATCH v4 06/12] ext4: using nofail preallocation in ext4_es_remove_extent() Baokun Li
2023-05-03 14:30 ` Jan Kara
2023-04-24 3:38 ` [PATCH v4 07/12] ext4: using nofail preallocation in ext4_es_insert_delayed_block() Baokun Li
2023-05-03 14:31 ` Jan Kara
2023-04-24 3:38 ` [PATCH v4 08/12] ext4: using nofail preallocation in ext4_es_insert_extent() Baokun Li
2023-05-03 14:32 ` Jan Kara
2023-04-24 3:38 ` [PATCH v4 09/12] ext4: make ext4_es_remove_extent() return void Baokun Li
2023-05-03 14:32 ` Jan Kara
2023-04-24 3:38 ` [PATCH v4 10/12] ext4: make ext4_es_insert_delayed_block() " Baokun Li
2023-05-03 14:32 ` Jan Kara
2023-06-10 19:03 ` Theodore Ts'o
2023-06-12 3:04 ` Theodore Ts'o
2023-06-12 3:47 ` Baokun Li
2023-06-12 15:26 ` Theodore Ts'o
2023-06-13 1:36 ` Baokun Li
2023-04-24 3:38 ` [PATCH v4 11/12] ext4: make ext4_es_insert_extent() " Baokun Li
2023-05-03 14:32 ` Jan Kara
2023-04-24 3:38 ` [PATCH v4 12/12] ext4: make ext4_zeroout_es() " Baokun Li
2023-05-03 14:33 ` Jan Kara
2023-05-24 7:30 ` [PATCH v4 00/12] ext4: fix WARNING in ext4_da_update_reserve_space Baokun Li
2023-06-09 3:14 ` Theodore Ts'o
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230424033846.4732-2-libaokun1@huawei.com \
--to=libaokun1@huawei.com \
--cc=adilger.kernel@dilger.ca \
--cc=jack@suse.cz \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=ritesh.list@gmail.com \
--cc=tytso@mit.edu \
--cc=yangerkun@huawei.com \
--cc=yi.zhang@huawei.com \
--cc=yukuai3@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.