* [PATCH v2 1/7] ext4: clear buffer verified flag if read meta block from disk
2020-09-24 7:33 [PATCH v2 0/7] ext4: fix a memory corrupt problem zhangyi (F)
@ 2020-09-24 7:33 ` zhangyi (F)
2020-10-09 1:41 ` Theodore Y. Ts'o
2020-09-24 7:33 ` [PATCH v2 2/7] ext4: introduce new metadata buffer read helpers zhangyi (F)
` (5 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: zhangyi (F) @ 2020-09-24 7:33 UTC (permalink / raw)
To: linux-ext4; +Cc: jack, tytso, adilger.kernel, yi.zhang
The metadata buffer is no longer trusted after we read it from disk
again because it is not uptodate for some reasons (e.g. failed to write
back). Otherwise we may get below memory corruption problem in
ext4_ext_split()->memset() if we read stale data from the newly
allocated extent block on disk which has been failed to async write
out but miss verify again since the verified bit has already been set
on the buffer.
[ 29.774674] BUG: unable to handle kernel paging request at ffff88841949d000
...
[ 29.783317] Oops: 0002 [#2] SMP
[ 29.784219] R10: 00000000000f4240 R11: 0000000000002e28 R12: ffff88842fa1c800
[ 29.784627] CPU: 1 PID: 126 Comm: kworker/u4:3 Tainted: G D W
[ 29.785546] R13: ffffffff9cddcc20 R14: ffffffff9cddd420 R15: ffff88842fa1c2f8
[ 29.786679] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),BIOS ?-20190727_0738364
[ 29.787588] FS: 0000000000000000(0000) GS:ffff88842fa00000(0000) knlGS:0000000000000000
[ 29.789288] Workqueue: writeback wb_workfn
[ 29.790319] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 29.790321] (flush-8:0)
[ 29.790844] CR2: 0000000000000008 CR3: 00000004234f2000 CR4: 00000000000006f0
[ 29.791924] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 29.792839] RIP: 0010:__memset+0x24/0x30
[ 29.793739] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 29.794256] Code: 90 90 90 90 90 90 0f 1f 44 00 00 49 89 f9 48 89 d1 83 e2 07 48 c1 e9 033
[ 29.795161] Kernel panic - not syncing: Fatal exception in interrupt
...
[ 29.808149] Call Trace:
[ 29.808475] ext4_ext_insert_extent+0x102e/0x1be0
[ 29.809085] ext4_ext_map_blocks+0xa89/0x1bb0
[ 29.809652] ext4_map_blocks+0x290/0x8a0
[ 29.809085] ext4_ext_map_blocks+0xa89/0x1bb0
[ 29.809652] ext4_map_blocks+0x290/0x8a0
[ 29.810161] ext4_writepages+0xc85/0x17c0
...
Fix this by clearing buffer's verified bit if we read meta block from
disk again.
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
Cc: stable@vger.kernel.org
---
fs/ext4/balloc.c | 1 +
fs/ext4/extents.c | 1 +
fs/ext4/ialloc.c | 1 +
fs/ext4/inode.c | 5 ++++-
fs/ext4/super.c | 1 +
5 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index 48c3df47748d..8e7e9715cde9 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -494,6 +494,7 @@ ext4_read_block_bitmap_nowait(struct super_block *sb, ext4_group_t block_group,
* submit the buffer_head for reading
*/
set_buffer_new(bh);
+ clear_buffer_verified(bh);
trace_ext4_read_block_bitmap_load(sb, block_group, ignore_locked);
bh->b_end_io = ext4_end_bitmap_read;
get_bh(bh);
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index a0481582187a..0a5205edc00a 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -501,6 +501,7 @@ __read_extent_tree_block(const char *function, unsigned int line,
if (!bh_uptodate_or_lock(bh)) {
trace_ext4_ext_load_extent(inode, pblk, _RET_IP_);
+ clear_buffer_verified(bh);
err = bh_submit_read(bh);
if (err < 0)
goto errout;
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index df25d38d6539..20cda952c621 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -188,6 +188,7 @@ ext4_read_inode_bitmap(struct super_block *sb, ext4_group_t block_group)
/*
* submit the buffer_head for reading
*/
+ clear_buffer_verified(bh);
trace_ext4_load_inode_bitmap(sb, block_group);
bh->b_end_io = ext4_end_bitmap_read;
get_bh(bh);
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index bf596467c234..7eaa55651d29 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -884,6 +884,7 @@ struct buffer_head *ext4_bread(handle_t *handle, struct inode *inode,
return bh;
if (!bh || ext4_buffer_uptodate(bh))
return bh;
+ clear_buffer_verified(bh);
ll_rw_block(REQ_OP_READ, REQ_META | REQ_PRIO, 1, &bh);
wait_on_buffer(bh);
if (buffer_uptodate(bh))
@@ -909,9 +910,11 @@ int ext4_bread_batch(struct inode *inode, ext4_lblk_t block, int bh_count,
for (i = 0; i < bh_count; i++)
/* Note that NULL bhs[i] is valid because of holes. */
- if (bhs[i] && !ext4_buffer_uptodate(bhs[i]))
+ if (bhs[i] && !ext4_buffer_uptodate(bhs[i])) {
+ clear_buffer_verified(bhs[i]);
ll_rw_block(REQ_OP_READ, REQ_META | REQ_PRIO, 1,
&bhs[i]);
+ }
if (!wait)
return 0;
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index ea425b49b345..9e760bf9e8b1 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -156,6 +156,7 @@ ext4_sb_bread(struct super_block *sb, sector_t block, int op_flags)
return ERR_PTR(-ENOMEM);
if (ext4_buffer_uptodate(bh))
return bh;
+ clear_buffer_verified(bh);
ll_rw_block(REQ_OP_READ, REQ_META | op_flags, 1, &bh);
wait_on_buffer(bh);
if (buffer_uptodate(bh))
--
2.25.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v2 1/7] ext4: clear buffer verified flag if read meta block from disk
2020-09-24 7:33 ` [PATCH v2 1/7] ext4: clear buffer verified flag if read meta block from disk zhangyi (F)
@ 2020-10-09 1:41 ` Theodore Y. Ts'o
0 siblings, 0 replies; 15+ messages in thread
From: Theodore Y. Ts'o @ 2020-10-09 1:41 UTC (permalink / raw)
To: zhangyi (F); +Cc: linux-ext4, jack, adilger.kernel
On Thu, Sep 24, 2020 at 03:33:31PM +0800, zhangyi (F) wrote:
> The metadata buffer is no longer trusted after we read it from disk
> again because it is not uptodate for some reasons (e.g. failed to write
> back). Otherwise we may get below memory corruption problem in
> ext4_ext_split()->memset() if we read stale data from the newly
> allocated extent block on disk which has been failed to async write
> out but miss verify again since the verified bit has already been set
> on the buffer.
>
> [ 29.774674] BUG: unable to handle kernel paging request at ffff88841949d000
> ...
> [ 29.783317] Oops: 0002 [#2] SMP
> [ 29.784219] R10: 00000000000f4240 R11: 0000000000002e28 R12: ffff88842fa1c800
> [ 29.784627] CPU: 1 PID: 126 Comm: kworker/u4:3 Tainted: G D W
> [ 29.785546] R13: ffffffff9cddcc20 R14: ffffffff9cddd420 R15: ffff88842fa1c2f8
> [ 29.786679] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),BIOS ?-20190727_0738364
> [ 29.787588] FS: 0000000000000000(0000) GS:ffff88842fa00000(0000) knlGS:0000000000000000
> [ 29.789288] Workqueue: writeback wb_workfn
> [ 29.790319] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 29.790321] (flush-8:0)
> [ 29.790844] CR2: 0000000000000008 CR3: 00000004234f2000 CR4: 00000000000006f0
> [ 29.791924] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 29.792839] RIP: 0010:__memset+0x24/0x30
> [ 29.793739] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [ 29.794256] Code: 90 90 90 90 90 90 0f 1f 44 00 00 49 89 f9 48 89 d1 83 e2 07 48 c1 e9 033
> [ 29.795161] Kernel panic - not syncing: Fatal exception in interrupt
> ...
> [ 29.808149] Call Trace:
> [ 29.808475] ext4_ext_insert_extent+0x102e/0x1be0
> [ 29.809085] ext4_ext_map_blocks+0xa89/0x1bb0
> [ 29.809652] ext4_map_blocks+0x290/0x8a0
> [ 29.809085] ext4_ext_map_blocks+0xa89/0x1bb0
> [ 29.809652] ext4_map_blocks+0x290/0x8a0
> [ 29.810161] ext4_writepages+0xc85/0x17c0
> ...
>
> Fix this by clearing buffer's verified bit if we read meta block from
> disk again.
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> Cc: stable@vger.kernel.org
Thanks, applied.
- Ted
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 2/7] ext4: introduce new metadata buffer read helpers
2020-09-24 7:33 [PATCH v2 0/7] ext4: fix a memory corrupt problem zhangyi (F)
2020-09-24 7:33 ` [PATCH v2 1/7] ext4: clear buffer verified flag if read meta block from disk zhangyi (F)
@ 2020-09-24 7:33 ` zhangyi (F)
2020-10-09 1:43 ` Theodore Y. Ts'o
2020-09-24 7:33 ` [PATCH v2 3/7] ext4: use common helpers in all places reading metadata buffers zhangyi (F)
` (4 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: zhangyi (F) @ 2020-09-24 7:33 UTC (permalink / raw)
To: linux-ext4; +Cc: jack, tytso, adilger.kernel, yi.zhang
The previous patch add clear_buffer_verified() before we read metadata
block from disk again, but it's rather easy to miss clearing of this bit
because currently we read metadata buffer through different open codes
(e.g. ll_rw_block(), bh_submit_read() and invoke submit_bh() directly).
So, it's time to add common helpers to unify in all the places reading
metadata buffers instead. This patch add 3 helpers:
- ext4_read_bh_nowait(): async read metadata buffer if it's actually
not uptodate, clear buffer_verified bit before read from disk.
- ext4_read_bh(): sync version of read metadata buffer, it will wait
until the read operation return and check the return status.
- ext4_read_bh_lock(): try to lock the buffer before read buffer, it
will skip reading if the buffer is already locked.
After this patch, we need to use these helpers in all the places reading
metadata buffer instead of different open codes.
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
Suggested-by: Jan Kara <jack@suse.cz>
---
fs/ext4/ext4.h | 5 ++++
fs/ext4/super.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 67 insertions(+)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 523e00d7b392..75b46300a65c 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2824,6 +2824,11 @@ extern int ext4_resize_fs(struct super_block *sb, ext4_fsblk_t n_blocks_count);
/* super.c */
extern struct buffer_head *ext4_sb_bread(struct super_block *sb,
sector_t block, int op_flags);
+extern void ext4_read_bh_nowait(struct buffer_head *bh, int op_flags,
+ bh_end_io_t *end_io);
+extern int ext4_read_bh(struct buffer_head *bh, int op_flags,
+ bh_end_io_t *end_io);
+extern int ext4_read_bh_lock(struct buffer_head *bh, int op_flags, bool wait);
extern int ext4_seq_options_show(struct seq_file *seq, void *offset);
extern int ext4_calculate_overhead(struct super_block *sb);
extern void ext4_superblock_csum_set(struct super_block *sb);
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 9e760bf9e8b1..1b1a4ca00957 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -141,6 +141,68 @@ MODULE_ALIAS_FS("ext3");
MODULE_ALIAS("ext3");
#define IS_EXT3_SB(sb) ((sb)->s_bdev->bd_holder == &ext3_fs_type)
+
+static inline void __ext4_read_bh(struct buffer_head *bh, int op_flags,
+ bh_end_io_t *end_io)
+{
+ /*
+ * buffer's verified bit is no longer valid after reading from
+ * disk again due to write out error, clear it to make sure we
+ * recheck the buffer contents.
+ */
+ clear_buffer_verified(bh);
+
+ bh->b_end_io = end_io ? end_io : end_buffer_read_sync;
+ get_bh(bh);
+ submit_bh(REQ_OP_READ, op_flags, bh);
+}
+
+void ext4_read_bh_nowait(struct buffer_head *bh, int op_flags,
+ bh_end_io_t *end_io)
+{
+ BUG_ON(!buffer_locked(bh));
+
+ if (ext4_buffer_uptodate(bh)) {
+ unlock_buffer(bh);
+ return;
+ }
+ __ext4_read_bh(bh, op_flags, end_io);
+}
+
+int ext4_read_bh(struct buffer_head *bh, int op_flags, bh_end_io_t *end_io)
+{
+ BUG_ON(!buffer_locked(bh));
+
+ if (ext4_buffer_uptodate(bh)) {
+ unlock_buffer(bh);
+ return 0;
+ }
+
+ __ext4_read_bh(bh, op_flags, end_io);
+
+ wait_on_buffer(bh);
+ if (buffer_uptodate(bh))
+ return 0;
+ return -EIO;
+}
+
+int ext4_read_bh_lock(struct buffer_head *bh, int op_flags, bool wait)
+{
+ if (trylock_buffer(bh)) {
+ if (wait)
+ return ext4_read_bh(bh, op_flags, NULL);
+ ext4_read_bh_nowait(bh, op_flags, NULL);
+ return 0;
+ }
+ if (wait) {
+ wait_on_buffer(bh);
+ if (buffer_uptodate(bh))
+ return 0;
+ return -EIO;
+ }
+ return 0;
+}
+
/*
* This works like sb_bread() except it uses ERR_PTR for error
* returns. Currently with sb_bread it's impossible to distinguish
--
2.25.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v2 2/7] ext4: introduce new metadata buffer read helpers
2020-09-24 7:33 ` [PATCH v2 2/7] ext4: introduce new metadata buffer read helpers zhangyi (F)
@ 2020-10-09 1:43 ` Theodore Y. Ts'o
0 siblings, 0 replies; 15+ messages in thread
From: Theodore Y. Ts'o @ 2020-10-09 1:43 UTC (permalink / raw)
To: zhangyi (F); +Cc: linux-ext4, jack, adilger.kernel
On Thu, Sep 24, 2020 at 03:33:32PM +0800, zhangyi (F) wrote:
> The previous patch add clear_buffer_verified() before we read metadata
> block from disk again, but it's rather easy to miss clearing of this bit
> because currently we read metadata buffer through different open codes
> (e.g. ll_rw_block(), bh_submit_read() and invoke submit_bh() directly).
> So, it's time to add common helpers to unify in all the places reading
> metadata buffers instead. This patch add 3 helpers:
>
> - ext4_read_bh_nowait(): async read metadata buffer if it's actually
> not uptodate, clear buffer_verified bit before read from disk.
> - ext4_read_bh(): sync version of read metadata buffer, it will wait
> until the read operation return and check the return status.
> - ext4_read_bh_lock(): try to lock the buffer before read buffer, it
> will skip reading if the buffer is already locked.
>
> After this patch, we need to use these helpers in all the places reading
> metadata buffer instead of different open codes.
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> Suggested-by: Jan Kara <jack@suse.cz>
Thanks, applied.
- Ted
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 3/7] ext4: use common helpers in all places reading metadata buffers
2020-09-24 7:33 [PATCH v2 0/7] ext4: fix a memory corrupt problem zhangyi (F)
2020-09-24 7:33 ` [PATCH v2 1/7] ext4: clear buffer verified flag if read meta block from disk zhangyi (F)
2020-09-24 7:33 ` [PATCH v2 2/7] ext4: introduce new metadata buffer read helpers zhangyi (F)
@ 2020-09-24 7:33 ` zhangyi (F)
2020-10-09 1:48 ` Theodore Y. Ts'o
2020-09-24 7:33 ` [PATCH v2 4/7] ext4: use ext4_buffer_uptodate() in __ext4_get_inode_loc() zhangyi (F)
` (3 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: zhangyi (F) @ 2020-09-24 7:33 UTC (permalink / raw)
To: linux-ext4; +Cc: jack, tytso, adilger.kernel, yi.zhang
Revome all open codes that read metadata buffers, switch to use
ext4_read_bh_*() common helpers.
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
Suggested-by: Jan Kara <jack@suse.cz>
---
fs/ext4/balloc.c | 8 +++-----
fs/ext4/extents.c | 3 +--
fs/ext4/ialloc.c | 6 +-----
fs/ext4/indirect.c | 2 +-
fs/ext4/inode.c | 35 ++++++++++++++---------------------
fs/ext4/mmp.c | 10 +++-------
fs/ext4/move_extent.c | 2 +-
fs/ext4/resize.c | 2 +-
fs/ext4/super.c | 22 +++++++++++-----------
9 files changed, 36 insertions(+), 54 deletions(-)
diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index 8e7e9715cde9..dea738ba2acd 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -494,12 +494,10 @@ ext4_read_block_bitmap_nowait(struct super_block *sb, ext4_group_t block_group,
* submit the buffer_head for reading
*/
set_buffer_new(bh);
- clear_buffer_verified(bh);
trace_ext4_read_block_bitmap_load(sb, block_group, ignore_locked);
- bh->b_end_io = ext4_end_bitmap_read;
- get_bh(bh);
- submit_bh(REQ_OP_READ, REQ_META | REQ_PRIO |
- (ignore_locked ? REQ_RAHEAD : 0), bh);
+ ext4_read_bh_nowait(bh, REQ_META | REQ_PRIO |
+ (ignore_locked ? REQ_RAHEAD : 0),
+ ext4_end_bitmap_read);
return bh;
verify:
err = ext4_validate_block_bitmap(sb, desc, block_group, bh);
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 0a5205edc00a..7cc202dcf083 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -501,8 +501,7 @@ __read_extent_tree_block(const char *function, unsigned int line,
if (!bh_uptodate_or_lock(bh)) {
trace_ext4_ext_load_extent(inode, pblk, _RET_IP_);
- clear_buffer_verified(bh);
- err = bh_submit_read(bh);
+ err = ext4_read_bh(bh, 0, NULL);
if (err < 0)
goto errout;
}
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 20cda952c621..33c0fc0197ce 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -188,12 +188,8 @@ ext4_read_inode_bitmap(struct super_block *sb, ext4_group_t block_group)
/*
* submit the buffer_head for reading
*/
- clear_buffer_verified(bh);
trace_ext4_load_inode_bitmap(sb, block_group);
- bh->b_end_io = ext4_end_bitmap_read;
- get_bh(bh);
- submit_bh(REQ_OP_READ, REQ_META | REQ_PRIO, bh);
- wait_on_buffer(bh);
+ ext4_read_bh(bh, REQ_META | REQ_PRIO, ext4_end_bitmap_read);
ext4_simulate_fail_bh(sb, bh, EXT4_SIM_IBITMAP_EIO);
if (!buffer_uptodate(bh)) {
put_bh(bh);
diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
index 80c9f33800be..4d7656f4ebc3 100644
--- a/fs/ext4/indirect.c
+++ b/fs/ext4/indirect.c
@@ -163,7 +163,7 @@ static Indirect *ext4_get_branch(struct inode *inode, int depth,
}
if (!bh_uptodate_or_lock(bh)) {
- if (bh_submit_read(bh) < 0) {
+ if (ext4_read_bh(bh, 0, NULL) < 0) {
put_bh(bh);
goto failure;
}
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 7eaa55651d29..3661ed126c5f 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -878,19 +878,20 @@ struct buffer_head *ext4_bread(handle_t *handle, struct inode *inode,
ext4_lblk_t block, int map_flags)
{
struct buffer_head *bh;
+ int ret;
bh = ext4_getblk(handle, inode, block, map_flags);
if (IS_ERR(bh))
return bh;
if (!bh || ext4_buffer_uptodate(bh))
return bh;
- clear_buffer_verified(bh);
- ll_rw_block(REQ_OP_READ, REQ_META | REQ_PRIO, 1, &bh);
- wait_on_buffer(bh);
- if (buffer_uptodate(bh))
- return bh;
- put_bh(bh);
- return ERR_PTR(-EIO);
+
+ ret = ext4_read_bh_lock(bh, REQ_META | REQ_PRIO, true);
+ if (ret) {
+ put_bh(bh);
+ return ERR_PTR(ret);
+ }
+ return bh;
}
/* Read a contiguous batch of blocks. */
@@ -910,11 +911,8 @@ int ext4_bread_batch(struct inode *inode, ext4_lblk_t block, int bh_count,
for (i = 0; i < bh_count; i++)
/* Note that NULL bhs[i] is valid because of holes. */
- if (bhs[i] && !ext4_buffer_uptodate(bhs[i])) {
- clear_buffer_verified(bhs[i]);
- ll_rw_block(REQ_OP_READ, REQ_META | REQ_PRIO, 1,
- &bhs[i]);
- }
+ if (bhs[i] && !ext4_buffer_uptodate(bhs[i]))
+ ext4_read_bh_lock(bhs[i], REQ_META | REQ_PRIO, false);
if (!wait)
return 0;
@@ -1084,7 +1082,7 @@ static int ext4_block_write_begin(struct page *page, loff_t pos, unsigned len,
if (!buffer_uptodate(bh) && !buffer_delay(bh) &&
!buffer_unwritten(bh) &&
(block_start < from || block_end > to)) {
- ll_rw_block(REQ_OP_READ, 0, 1, &bh);
+ ext4_read_bh_lock(bh, 0, false);
wait[nr_wait++] = bh;
}
}
@@ -3733,11 +3731,8 @@ static int __ext4_block_zero_page_range(handle_t *handle,
set_buffer_uptodate(bh);
if (!buffer_uptodate(bh)) {
- err = -EIO;
- ll_rw_block(REQ_OP_READ, 0, 1, &bh);
- wait_on_buffer(bh);
- /* Uhhuh. Read error. Complain and punt. */
- if (!buffer_uptodate(bh))
+ err = ext4_read_bh_lock(bh, 0, true);
+ if (err)
goto unlock;
if (fscrypt_inode_uses_fs_layer_crypto(inode)) {
/* We expect the key to be set. */
@@ -4381,9 +4376,7 @@ static int __ext4_get_inode_loc(struct inode *inode,
* Read the block from disk.
*/
trace_ext4_load_inode(inode);
- get_bh(bh);
- bh->b_end_io = end_buffer_read_sync;
- submit_bh(REQ_OP_READ, REQ_META | REQ_PRIO, bh);
+ ext4_read_bh_nowait(bh, REQ_META | REQ_PRIO, NULL);
blk_finish_plug(&plug);
wait_on_buffer(bh);
if (!buffer_uptodate(bh)) {
diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
index d34cb8c46655..795c3ff2907c 100644
--- a/fs/ext4/mmp.c
+++ b/fs/ext4/mmp.c
@@ -85,15 +85,11 @@ static int read_mmp_block(struct super_block *sb, struct buffer_head **bh,
}
}
- get_bh(*bh);
lock_buffer(*bh);
- (*bh)->b_end_io = end_buffer_read_sync;
- submit_bh(REQ_OP_READ, REQ_META | REQ_PRIO, *bh);
- wait_on_buffer(*bh);
- if (!buffer_uptodate(*bh)) {
- ret = -EIO;
+ ret = ext4_read_bh(*bh, REQ_META | REQ_PRIO, NULL);
+ if (ret)
goto warn_exit;
- }
+
mmp = (struct mmp_struct *)((*bh)->b_data);
if (le32_to_cpu(mmp->mmp_magic) != EXT4_MMP_MAGIC) {
ret = -EFSCORRUPTED;
diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
index 0d601b822875..64a579734f93 100644
--- a/fs/ext4/move_extent.c
+++ b/fs/ext4/move_extent.c
@@ -215,7 +215,7 @@ mext_page_mkuptodate(struct page *page, unsigned from, unsigned to)
for (i = 0; i < nr; i++) {
bh = arr[i];
if (!bh_uptodate_or_lock(bh)) {
- err = bh_submit_read(bh);
+ err = ext4_read_bh(bh, 0, NULL);
if (err)
return err;
}
diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index a50b51270ea9..8596bdda304c 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -1243,7 +1243,7 @@ static struct buffer_head *ext4_get_bitmap(struct super_block *sb, __u64 block)
if (unlikely(!bh))
return NULL;
if (!bh_uptodate_or_lock(bh)) {
- if (bh_submit_read(bh) < 0) {
+ if (ext4_read_bh(bh, 0, NULL) < 0) {
brelse(bh);
return NULL;
}
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 1b1a4ca00957..0adba4871f57 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -212,19 +212,21 @@ int ext4_read_bh_lock(struct buffer_head *bh, int op_flags, bool wait)
struct buffer_head *
ext4_sb_bread(struct super_block *sb, sector_t block, int op_flags)
{
- struct buffer_head *bh = sb_getblk(sb, block);
+ struct buffer_head *bh;
+ int ret;
+ bh = sb_getblk(sb, block);
if (bh == NULL)
return ERR_PTR(-ENOMEM);
if (ext4_buffer_uptodate(bh))
return bh;
- clear_buffer_verified(bh);
- ll_rw_block(REQ_OP_READ, REQ_META | op_flags, 1, &bh);
- wait_on_buffer(bh);
- if (buffer_uptodate(bh))
- return bh;
- put_bh(bh);
- return ERR_PTR(-EIO);
+
+ ret = ext4_read_bh_lock(bh, REQ_META | op_flags, true);
+ if (ret) {
+ put_bh(bh);
+ return ERR_PTR(ret);
+ }
+ return bh;
}
static int ext4_verify_csum_type(struct super_block *sb,
@@ -5165,9 +5167,7 @@ static journal_t *ext4_get_dev_journal(struct super_block *sb,
goto out_bdev;
}
journal->j_private = sb;
- ll_rw_block(REQ_OP_READ, REQ_META | REQ_PRIO, 1, &journal->j_sb_buffer);
- wait_on_buffer(journal->j_sb_buffer);
- if (!buffer_uptodate(journal->j_sb_buffer)) {
+ if (ext4_read_bh_lock(journal->j_sb_buffer, REQ_META | REQ_PRIO, true)) {
ext4_msg(sb, KERN_ERR, "I/O error on journal device");
goto out_journal;
}
--
2.25.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v2 4/7] ext4: use ext4_buffer_uptodate() in __ext4_get_inode_loc()
2020-09-24 7:33 [PATCH v2 0/7] ext4: fix a memory corrupt problem zhangyi (F)
` (2 preceding siblings ...)
2020-09-24 7:33 ` [PATCH v2 3/7] ext4: use common helpers in all places reading metadata buffers zhangyi (F)
@ 2020-09-24 7:33 ` zhangyi (F)
2020-10-09 1:49 ` Theodore Y. Ts'o
2020-09-24 7:33 ` [PATCH v2 5/7] ext4: introduce ext4_sb_breadahead_unmovable() to replace sb_breadahead_unmovable() zhangyi (F)
` (2 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: zhangyi (F) @ 2020-09-24 7:33 UTC (permalink / raw)
To: linux-ext4; +Cc: jack, tytso, adilger.kernel, yi.zhang
We have already introduced ext4_buffer_uptodate() to re-set the uptodate
bit on buffer which has been failed to write out to disk. Just remove
the redundant codes and switch to use ext4_buffer_uptodate() in
__ext4_get_inode_loc().
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
fs/ext4/inode.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 3661ed126c5f..171df289ef7e 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4287,16 +4287,7 @@ static int __ext4_get_inode_loc(struct inode *inode,
if (!buffer_uptodate(bh)) {
lock_buffer(bh);
- /*
- * If the buffer has the write error flag, we have failed
- * to write out another inode in the same block. In this
- * case, we don't have to read the block because we may
- * read the old inode data successfully.
- */
- if (buffer_write_io_error(bh) && !buffer_uptodate(bh))
- set_buffer_uptodate(bh);
-
- if (buffer_uptodate(bh)) {
+ if (ext4_buffer_uptodate(bh)) {
/* someone brought it uptodate while we waited */
unlock_buffer(bh);
goto has_buffer;
--
2.25.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v2 5/7] ext4: introduce ext4_sb_breadahead_unmovable() to replace sb_breadahead_unmovable()
2020-09-24 7:33 [PATCH v2 0/7] ext4: fix a memory corrupt problem zhangyi (F)
` (3 preceding siblings ...)
2020-09-24 7:33 ` [PATCH v2 4/7] ext4: use ext4_buffer_uptodate() in __ext4_get_inode_loc() zhangyi (F)
@ 2020-09-24 7:33 ` zhangyi (F)
2020-10-09 1:55 ` Theodore Y. Ts'o
2020-09-24 7:33 ` [PATCH v2 6/7] ext4: use ext4_sb_bread() instead of sb_bread() zhangyi (F)
2020-09-24 7:33 ` [PATCH v2 7/7] ext4: introduce ext4_sb_bread_unmovable() to replace sb_bread_unmovable() zhangyi (F)
6 siblings, 1 reply; 15+ messages in thread
From: zhangyi (F) @ 2020-09-24 7:33 UTC (permalink / raw)
To: linux-ext4; +Cc: jack, tytso, adilger.kernel, yi.zhang
If we readahead inode tables in __ext4_get_inode_loc(), it may bypass
buffer_write_io_error() check, so introduce ext4_sb_breadahead_unmovable()
to handle this special case.
This patch also replace sb_breadahead_unmovable() in ext4_fill_super()
for the sake of unification.
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
fs/ext4/ext4.h | 1 +
fs/ext4/inode.c | 2 +-
fs/ext4/super.c | 12 +++++++++++-
3 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 75b46300a65c..6da1419f6ee7 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2829,6 +2829,7 @@ extern void ext4_read_bh_nowait(struct buffer_head *bh, int op_flags,
extern int ext4_read_bh(struct buffer_head *bh, int op_flags,
bh_end_io_t *end_io);
extern int ext4_read_bh_lock(struct buffer_head *bh, int op_flags, bool wait);
+extern void ext4_sb_breadahead_unmovable(struct super_block *sb, sector_t block);
extern int ext4_seq_options_show(struct seq_file *seq, void *offset);
extern int ext4_calculate_overhead(struct super_block *sb);
extern void ext4_superblock_csum_set(struct super_block *sb);
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 171df289ef7e..e106d03e4b77 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4358,7 +4358,7 @@ static int __ext4_get_inode_loc(struct inode *inode,
if (end > table)
end = table;
while (b <= end)
- sb_breadahead_unmovable(sb, b++);
+ ext4_sb_breadahead_unmovable(sb, b++);
}
/*
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 0adba4871f57..b24e68eff48d 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -229,6 +229,16 @@ ext4_sb_bread(struct super_block *sb, sector_t block, int op_flags)
return bh;
}
+void ext4_sb_breadahead_unmovable(struct super_block *sb, sector_t block)
+{
+ struct buffer_head *bh = sb_getblk_gfp(sb, block, 0);
+
+ if (likely(bh)) {
+ ext4_read_bh_lock(bh, REQ_RAHEAD, false);
+ brelse(bh);
+ }
+}
+
static int ext4_verify_csum_type(struct super_block *sb,
struct ext4_super_block *es)
{
@@ -4545,7 +4555,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
/* Pre-read the descriptors into the buffer cache */
for (i = 0; i < db_count; i++) {
block = descriptor_loc(sb, logical_sb_block, i);
- sb_breadahead_unmovable(sb, block);
+ ext4_sb_breadahead_unmovable(sb, block);
}
for (i = 0; i < db_count; i++) {
--
2.25.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v2 6/7] ext4: use ext4_sb_bread() instead of sb_bread()
2020-09-24 7:33 [PATCH v2 0/7] ext4: fix a memory corrupt problem zhangyi (F)
` (4 preceding siblings ...)
2020-09-24 7:33 ` [PATCH v2 5/7] ext4: introduce ext4_sb_breadahead_unmovable() to replace sb_breadahead_unmovable() zhangyi (F)
@ 2020-09-24 7:33 ` zhangyi (F)
2020-10-09 1:57 ` Theodore Y. Ts'o
2020-09-24 7:33 ` [PATCH v2 7/7] ext4: introduce ext4_sb_bread_unmovable() to replace sb_bread_unmovable() zhangyi (F)
6 siblings, 1 reply; 15+ messages in thread
From: zhangyi (F) @ 2020-09-24 7:33 UTC (permalink / raw)
To: linux-ext4; +Cc: jack, tytso, adilger.kernel, yi.zhang
We have already remove open codes that invoke helpers provide by
fs/buffer.c in all places reading metadata buffers. This patch switch to
use ext4_sb_bread() to replace all sb_bread() helpers, which is
ext4_read_bh() helper back end.
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
fs/ext4/indirect.c | 6 +++---
fs/ext4/resize.c | 8 ++++----
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
index 4d7656f4ebc3..ba944d67c1c0 100644
--- a/fs/ext4/indirect.c
+++ b/fs/ext4/indirect.c
@@ -1012,14 +1012,14 @@ static void ext4_free_branches(handle_t *handle, struct inode *inode,
}
/* Go read the buffer for the next level down */
- bh = sb_bread(inode->i_sb, nr);
+ bh = ext4_sb_bread(inode->i_sb, nr, 0);
/*
* A read failure? Report error and clear slot
* (should be rare).
*/
- if (!bh) {
- ext4_error_inode_block(inode, nr, EIO,
+ if (IS_ERR(bh)) {
+ ext4_error_inode_block(inode, nr, -PTR_ERR(bh),
"Read failure");
continue;
}
diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index 8596bdda304c..5e72849f1df9 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -1806,8 +1806,8 @@ int ext4_group_extend(struct super_block *sb, struct ext4_super_block *es,
o_blocks_count + add, add);
/* See if the device is actually as big as what was requested */
- bh = sb_bread(sb, o_blocks_count + add - 1);
- if (!bh) {
+ bh = ext4_sb_bread(sb, o_blocks_count + add - 1, 0);
+ if (IS_ERR(bh)) {
ext4_warning(sb, "can't read last block, resize aborted");
return -ENOSPC;
}
@@ -1932,8 +1932,8 @@ int ext4_resize_fs(struct super_block *sb, ext4_fsblk_t n_blocks_count)
int meta_bg;
/* See if the device is actually as big as what was requested */
- bh = sb_bread(sb, n_blocks_count - 1);
- if (!bh) {
+ bh = ext4_sb_bread(sb, n_blocks_count - 1, 0);
+ if (IS_ERR(bh)) {
ext4_warning(sb, "can't read last block, resize aborted");
return -ENOSPC;
}
--
2.25.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v2 7/7] ext4: introduce ext4_sb_bread_unmovable() to replace sb_bread_unmovable()
2020-09-24 7:33 [PATCH v2 0/7] ext4: fix a memory corrupt problem zhangyi (F)
` (5 preceding siblings ...)
2020-09-24 7:33 ` [PATCH v2 6/7] ext4: use ext4_sb_bread() instead of sb_bread() zhangyi (F)
@ 2020-09-24 7:33 ` zhangyi (F)
2020-10-09 2:00 ` Theodore Y. Ts'o
6 siblings, 1 reply; 15+ messages in thread
From: zhangyi (F) @ 2020-09-24 7:33 UTC (permalink / raw)
To: linux-ext4; +Cc: jack, tytso, adilger.kernel, yi.zhang
Now we only use sb_bread_unmovable() to read superblock and descriptor
block at mount time, so there is no opportunity that we need to clear
buffer verified bit and also handle buffer write_io error bit. But for
the sake of unification, let's introduce ext4_sb_bread_unmovable() to
replace all sb_bread_unmovable(). After this patch, we stop using read
helpers in fs/buffer.c.
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
fs/ext4/ext4.h | 2 ++
fs/ext4/super.c | 38 +++++++++++++++++++++++++++++---------
2 files changed, 31 insertions(+), 9 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 6da1419f6ee7..28b135a536b5 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2824,6 +2824,8 @@ extern int ext4_resize_fs(struct super_block *sb, ext4_fsblk_t n_blocks_count);
/* super.c */
extern struct buffer_head *ext4_sb_bread(struct super_block *sb,
sector_t block, int op_flags);
+extern struct buffer_head *ext4_sb_bread_unmovable(struct super_block *sb,
+ sector_t block);
extern void ext4_read_bh_nowait(struct buffer_head *bh, int op_flags,
bh_end_io_t *end_io);
extern int ext4_read_bh(struct buffer_head *bh, int op_flags,
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index b24e68eff48d..2b5b6033b8e6 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -204,18 +204,19 @@ int ext4_read_bh_lock(struct buffer_head *bh, int op_flags, bool wait)
}
/*
- * This works like sb_bread() except it uses ERR_PTR for error
+ * This works like __bread_gfp() except it uses ERR_PTR for error
* returns. Currently with sb_bread it's impossible to distinguish
* between ENOMEM and EIO situations (since both result in a NULL
* return.
*/
-struct buffer_head *
-ext4_sb_bread(struct super_block *sb, sector_t block, int op_flags)
+static struct buffer_head *__ext4_sb_bread_gfp(struct super_block *sb,
+ sector_t block, int op_flags,
+ gfp_t gfp)
{
struct buffer_head *bh;
int ret;
- bh = sb_getblk(sb, block);
+ bh = sb_getblk_gfp(sb, block, gfp);
if (bh == NULL)
return ERR_PTR(-ENOMEM);
if (ext4_buffer_uptodate(bh))
@@ -229,6 +230,18 @@ ext4_sb_bread(struct super_block *sb, sector_t block, int op_flags)
return bh;
}
+struct buffer_head *ext4_sb_bread(struct super_block *sb, sector_t block,
+ int op_flags)
+{
+ return __ext4_sb_bread_gfp(sb, block, op_flags, __GFP_MOVABLE);
+}
+
+struct buffer_head *ext4_sb_bread_unmovable(struct super_block *sb,
+ sector_t block)
+{
+ return __ext4_sb_bread_gfp(sb, block, 0, 0);
+}
+
void ext4_sb_breadahead_unmovable(struct super_block *sb, sector_t block)
{
struct buffer_head *bh = sb_getblk_gfp(sb, block, 0);
@@ -3943,8 +3956,11 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
logical_sb_block = sb_block;
}
- if (!(bh = sb_bread_unmovable(sb, logical_sb_block))) {
+ bh = ext4_sb_bread_unmovable(sb, logical_sb_block);
+ if (IS_ERR(bh)) {
ext4_msg(sb, KERN_ERR, "unable to read superblock");
+ ret = PTR_ERR(bh);
+ bh = NULL;
goto out_fail;
}
/*
@@ -4340,10 +4356,12 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
brelse(bh);
logical_sb_block = sb_block * EXT4_MIN_BLOCK_SIZE;
offset = do_div(logical_sb_block, blocksize);
- bh = sb_bread_unmovable(sb, logical_sb_block);
- if (!bh) {
+ bh = ext4_sb_bread_unmovable(sb, logical_sb_block);
+ if (IS_ERR(bh)) {
ext4_msg(sb, KERN_ERR,
"Can't read superblock on 2nd try");
+ ret = PTR_ERR(bh);
+ bh = NULL;
goto failed_mount;
}
es = (struct ext4_super_block *)(bh->b_data + offset);
@@ -4562,11 +4580,13 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
struct buffer_head *bh;
block = descriptor_loc(sb, logical_sb_block, i);
- bh = sb_bread_unmovable(sb, block);
- if (!bh) {
+ bh = ext4_sb_bread_unmovable(sb, block);
+ if (IS_ERR(bh)) {
ext4_msg(sb, KERN_ERR,
"can't read group descriptor %d", i);
db_count = i;
+ ret = PTR_ERR(bh);
+ bh = NULL;
goto failed_mount2;
}
rcu_read_lock();
--
2.25.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v2 7/7] ext4: introduce ext4_sb_bread_unmovable() to replace sb_bread_unmovable()
2020-09-24 7:33 ` [PATCH v2 7/7] ext4: introduce ext4_sb_bread_unmovable() to replace sb_bread_unmovable() zhangyi (F)
@ 2020-10-09 2:00 ` Theodore Y. Ts'o
0 siblings, 0 replies; 15+ messages in thread
From: Theodore Y. Ts'o @ 2020-10-09 2:00 UTC (permalink / raw)
To: zhangyi (F); +Cc: linux-ext4, jack, adilger.kernel
On Thu, Sep 24, 2020 at 03:33:37PM +0800, zhangyi (F) wrote:
> Now we only use sb_bread_unmovable() to read superblock and descriptor
> block at mount time, so there is no opportunity that we need to clear
> buffer verified bit and also handle buffer write_io error bit. But for
> the sake of unification, let's introduce ext4_sb_bread_unmovable() to
> replace all sb_bread_unmovable(). After this patch, we stop using read
> helpers in fs/buffer.c.
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
Thanks, applied.
- Ted
^ permalink raw reply [flat|nested] 15+ messages in thread