When endio conversion of an unwritten extent fails (e.g. ENOSPC while trying to split the extent), ext4 falls back to zeroing out the unwritten portions of the extent and marking it as written. This is safe only when no other in-flight writes have valid data underneath other parts of the same unwritten extent, otherwise the zeroout would silently corrupt that data. Commit ea96cb5c4ae3 ("ext4: don't split extent before submitting I/O") changed the dioread_nolock writeback (non buffered iomap) and dio path to submit I/O without pre-splitting extents, deferring the split to endio. This means multiple concurrent writes can share a single unwritten extent, and a failed split on one of them could zeroout data belonging to another. Fix this for dioread_nolock non buffered iomap writeback by making sure the writeback path increments the i_conv_pending_ios counter when it encounters an unwritten io_end, and decrements it after all the io_end_vecs have been converted. The existing checks in zeroout paths then use this counter to suppress the zeroout fallback whenever there are other unwritten IOs pending conversion, preventing silent data corruption. Fixes: ea96cb5c4ae3 ("ext4: don't split extent before submitting I/O") Signed-off-by: Ojaswin Mujoo --- fs/ext4/ext4.h | 7 ++++--- fs/ext4/extents.c | 29 +++++++++++++++++++++++------ fs/ext4/page-io.c | 6 ++++++ 3 files changed, 33 insertions(+), 9 deletions(-) diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index 1a01b1f809f7..44cd9f12d350 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -4113,14 +4113,15 @@ extern int ext4_resize_end(struct super_block *sb, bool update_backups); static inline void ext4_set_io_unwritten_flag(struct ext4_io_end *io_end) { - if (!(io_end->flag & EXT4_IO_END_UNWRITTEN)) + if (!(io_end->flag & EXT4_IO_END_UNWRITTEN)) { io_end->flag |= EXT4_IO_END_UNWRITTEN; + ext4_conv_pending_inc(io_end->inode); + } } static inline void ext4_clear_io_unwritten_flag(ext4_io_end_t *io_end) { - if (io_end->flag & EXT4_IO_END_UNWRITTEN) - io_end->flag &= ~EXT4_IO_END_UNWRITTEN; + io_end->flag &= ~EXT4_IO_END_UNWRITTEN; } extern const struct iomap_ops ext4_iomap_ops; diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c index d255884cf075..96d8ad012b2a 100644 --- a/fs/ext4/extents.c +++ b/fs/ext4/extents.c @@ -5286,6 +5286,19 @@ int ext4_convert_unwritten_io_end_vec(handle_t *handle, ext4_io_end_t *io_end) { int ret = 0, err = 0; struct ext4_io_end_vec *io_end_vec; + int n = 0; + + /* + * We take a single i_conv_pending_ios count for every unwritten io_end + * however each io_vec also needs its count to prevent a neighboring + * io_end_vec zeroing its data. This can't be accurately done during + * submission so do it here. + */ + list_for_each_entry(io_end_vec, &io_end->list_vec, list) + n++; + + if (n > 1) + ext4_conv_pending_sum(io_end->inode, n - 1); /* * This is somewhat ugly but the idea is clear: When transaction is @@ -5295,16 +5308,20 @@ int ext4_convert_unwritten_io_end_vec(handle_t *handle, ext4_io_end_t *io_end) if (handle) { handle = ext4_journal_start_reserved(handle, EXT4_HT_EXT_CONVERT); - if (IS_ERR(handle)) + if (IS_ERR(handle)) { + ext4_conv_pending_sub(io_end->inode, n); return PTR_ERR(handle); + } } list_for_each_entry(io_end_vec, &io_end->list_vec, list) { - ret = ext4_convert_unwritten_extents(handle, io_end->inode, - io_end_vec->offset, - io_end_vec->size, NULL, 0); - if (ret) - break; + if (!ret) + ret = ext4_convert_unwritten_extents(handle, + io_end->inode, + io_end_vec->offset, + io_end_vec->size, + NULL, 0); + ext4_conv_pending_dec(io_end->inode); } if (handle) diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c index 4c5c77da2a5e..00abef904702 100644 --- a/fs/ext4/page-io.c +++ b/fs/ext4/page-io.c @@ -190,6 +190,12 @@ static int ext4_end_io_end(ext4_io_end_t *io_end) if (test_opt(sb, DATA_ERR_ABORT)) jbd2_journal_abort(EXT4_SB(sb)->s_journal, ret); + + /* + * Conversion was skipped so drop the count that submission took + */ + if (io_end->flag & EXT4_IO_END_UNWRITTEN) + ext4_conv_pending_dec(inode); } else { ret = ext4_convert_unwritten_io_end_vec(handle, io_end); } -- 2.55.0