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 v2 2/2] ext4: use __GFP_NOFAIL if allocating extents_status cannot fail
Date: Thu, 6 Apr 2023 21:28:34 +0800 [thread overview]
Message-ID: <20230406132834.1669710-3-libaokun1@huawei.com> (raw)
In-Reply-To: <20230406132834.1669710-1-libaokun1@huawei.com>
If extent status tree update fails, we have inconsistency between what is
stored in the extent status tree and what is stored on disk. And that can
cause even data corruption issues in some cases.
In the extent status tree, we have extents which we can just drop without
issues and extents we must not drop - this depends on the extent's status
- currently ext4_es_is_delayed() extents must stay, others may be dropped.
For extents that cannot be dropped we use __GFP_NOFAIL to allocate memory.
A helper function is also added to help determine if the current extent can
be dropped, although only ext4_es_is_delayed() extents cannot be dropped
currently. In addition, with the above logic, the undo operation in
__es_remove_extent that may cause inconsistency if the split extent fails
is unnecessary, so we remove it as well.
Suggested-by: Jan Kara <jack@suse.cz>
Signed-off-by: Baokun Li <libaokun1@huawei.com>
---
V1->V2:
Add the patch 2 as suggested by Jan Kara.
fs/ext4/extents_status.c | 36 +++++++++++++++++++++++++++++-------
1 file changed, 29 insertions(+), 7 deletions(-)
diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
index 7bc221038c6c..8eed17f35b11 100644
--- a/fs/ext4/extents_status.c
+++ b/fs/ext4/extents_status.c
@@ -448,12 +448,29 @@ static void ext4_es_list_del(struct inode *inode)
spin_unlock(&sbi->s_es_lock);
}
+/*
+ * Helper function to help determine if memory allocation for this
+ * extent_status is allowed to fail.
+ */
+static inline bool ext4_es_alloc_should_nofail(struct extent_status *es)
+{
+ if (ext4_es_is_delayed(es))
+ return true;
+
+ return false;
+}
+
static struct extent_status *
ext4_es_alloc_extent(struct inode *inode, ext4_lblk_t lblk, ext4_lblk_t len,
- ext4_fsblk_t pblk)
+ ext4_fsblk_t pblk, int nofail)
{
struct extent_status *es;
- es = kmem_cache_alloc(ext4_es_cachep, GFP_ATOMIC);
+ gfp_t gfp_flags = GFP_ATOMIC;
+
+ if (nofail)
+ gfp_flags |= __GFP_NOFAIL;
+
+ es = kmem_cache_alloc(ext4_es_cachep, gfp_flags);
if (es == NULL)
return NULL;
es->es_lblk = lblk;
@@ -792,9 +809,16 @@ static int __es_insert_extent(struct inode *inode, struct extent_status *newes)
}
es = ext4_es_alloc_extent(inode, newes->es_lblk, newes->es_len,
- newes->es_pblk);
- if (!es)
- return -ENOMEM;
+ newes->es_pblk, 0);
+ if (!es) {
+ /* Use GFP_NOFAIL if the allocation cannot fail. */
+ if (ext4_es_alloc_should_nofail(newes))
+ es = ext4_es_alloc_extent(inode, newes->es_lblk,
+ newes->es_len, newes->es_pblk, 1);
+ else
+ return -ENOMEM;
+ }
+
rb_link_node(&es->rb_node, parent, p);
rb_insert_color(&es->rb_node, &tree->root);
@@ -1349,8 +1373,6 @@ static int __es_remove_extent(struct inode *inode, ext4_lblk_t lblk,
ext4_es_status(&orig_es));
err = __es_insert_extent(inode, &newes);
if (err) {
- es->es_lblk = orig_es.es_lblk;
- es->es_len = orig_es.es_len;
if ((err == -ENOMEM) &&
__es_shrink(EXT4_SB(inode->i_sb),
128, EXT4_I(inode)))
--
2.31.1
next prev parent reply other threads:[~2023-04-06 13:29 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-06 13:28 [PATCH v2 0/2] ext4: fix WARNING in ext4_da_update_reserve_space Baokun Li
2023-04-06 13:28 ` [PATCH v2 1/2] ext4: only update i_reserved_data_blocks on successful block allocation Baokun Li
2023-04-11 9:08 ` Jan Kara
2023-04-11 14:10 ` Baokun Li
2023-04-06 13:28 ` Baokun Li [this message]
2023-04-11 9:19 ` [PATCH v2 2/2] ext4: use __GFP_NOFAIL if allocating extents_status cannot fail Jan Kara
2023-04-11 14:04 ` Baokun Li
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=20230406132834.1669710-3-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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®