From: Zhang Yi <yi.zhang@huawei.com>
To: <linux-ext4@vger.kernel.org>
Cc: <tytso@mit.edu>, <adilger.kernel@dilger.ca>, <jack@suse.cz>,
<yi.zhang@huawei.com>, <yukuai3@huawei.com>
Subject: [PATCH 3/3] ext4: prevent getting empty inode buffer
Date: Tue, 10 Aug 2021 22:27:22 +0800 [thread overview]
Message-ID: <20210810142722.923175-4-yi.zhang@huawei.com> (raw)
In-Reply-To: <20210810142722.923175-1-yi.zhang@huawei.com>
In ext4_get_inode_loc(), we may skip IO and get an zero && uptodate
inode buffer when the inode monopolize an inode block for performance
reason. For most cases, ext4_mark_iloc_dirty() will fill the inode
buffer to make it fine, but we could miss this call if something bad
happened. Finally, __ext4_get_inode_loc_noinmem() may probably get an
empty inode buffer and trigger ext4 error.
For example, if we remove a nonexistent xattr on inode A,
ext4_xattr_set_handle() will return ENODATA before invoking
ext4_mark_iloc_dirty(), it will left an uptodate but zero buffer. We
will get checksum error message in ext4_iget() when getting inode again.
EXT4-fs error (device sda): ext4_lookup:1784: inode #131074: comm cat: iget: checksum invalid
Even worse, if we allocate another inode B at the same inode block, it
will corrupt the inode A on disk when write back inode B.
So this patch clear uptodate flag and mark buffer new if we get an empty
buffer, clear it after we fill inode data or making read IO.
Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
---
fs/ext4/inode.c | 27 +++++++++++++++++++++++----
1 file changed, 23 insertions(+), 4 deletions(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index eae1b2d0b550..1f36289e9ef6 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4292,6 +4292,18 @@ int ext4_truncate(struct inode *inode)
return err;
}
+static void ext4_end_inode_loc_read(struct buffer_head *bh, int uptodate)
+{
+ if (buffer_new(bh))
+ clear_buffer_new(bh);
+ if (uptodate)
+ set_buffer_uptodate(bh);
+ else
+ clear_buffer_uptodate(bh);
+ unlock_buffer(bh);
+ put_bh(bh);
+}
+
/*
* ext4_get_inode_loc returns with an extra refcount against the inode's
* underlying buffer_head on success. If 'in_mem' is true, we have all
@@ -4367,9 +4379,11 @@ static int __ext4_get_inode_loc(struct super_block *sb, unsigned long ino,
}
brelse(bitmap_bh);
if (i == start + inodes_per_block) {
- /* all other inodes are free, so skip I/O */
- memset(bh->b_data, 0, bh->b_size);
- set_buffer_uptodate(bh);
+ if (!buffer_new(bh)) {
+ /* all other inodes are free, so skip I/O */
+ memset(bh->b_data, 0, bh->b_size);
+ set_buffer_new(bh);
+ }
unlock_buffer(bh);
goto has_buffer;
}
@@ -4408,7 +4422,7 @@ static int __ext4_get_inode_loc(struct super_block *sb, unsigned long ino,
* Read the block from disk.
*/
trace_ext4_load_inode(sb, ino);
- ext4_read_bh_nowait(bh, REQ_META | REQ_PRIO, NULL);
+ ext4_read_bh_nowait(bh, REQ_META | REQ_PRIO, ext4_end_inode_loc_read);
blk_finish_plug(&plug);
wait_on_buffer(bh);
ext4_simulate_fail_bh(sb, bh, EXT4_SIM_INODE_EIO);
@@ -5132,6 +5146,11 @@ static int ext4_do_update_inode(handle_t *handle,
if (err)
goto out_brelse;
ext4_clear_inode_state(inode, EXT4_STATE_NEW);
+ if (buffer_new(bh)) {
+ clear_buffer_new(bh);
+ set_buffer_uptodate(bh);
+ }
+
if (set_large_file) {
BUFFER_TRACE(EXT4_SB(sb)->s_sbh, "get write access");
err = ext4_journal_get_write_access(handle, EXT4_SB(sb)->s_sbh);
--
2.31.1
next prev parent reply other threads:[~2021-08-10 14:18 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-10 14:27 [PATCH 0/3] ext4: fix a inode checksum error Zhang Yi
2021-08-10 14:27 ` [PATCH 1/3] ext4: move inode eio simulation behind io completeion Zhang Yi
2021-08-13 12:55 ` Jan Kara
2021-08-10 14:27 ` [PATCH 2/3] ext4: remove an unnecessary if statement in __ext4_get_inode_loc() Zhang Yi
2021-08-13 13:00 ` Jan Kara
2021-08-10 14:27 ` Zhang Yi [this message]
2021-08-13 13:44 ` [PATCH 3/3] ext4: prevent getting empty inode buffer Jan Kara
2021-08-16 14:29 ` Zhang Yi
2021-08-16 17:14 ` Jan Kara
2021-08-18 12:15 ` Zhang Yi
2021-08-18 13:11 ` Jan Kara
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=20210810142722.923175-4-yi.zhang@huawei.com \
--to=yi.zhang@huawei.com \
--cc=adilger.kernel@dilger.ca \
--cc=jack@suse.cz \
--cc=linux-ext4@vger.kernel.org \
--cc=tytso@mit.edu \
--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;
as well as URLs for NNTP newsgroup(s).