Before EXT4_IO_END_FAILED was added, the writeback thread set EXT4_IO_END_UNWRITTEN and final completion cleared it after the last io_end reference was dropped. Adding EXT4_IO_END_FAILED introduced a concurrent writer in failed-bio completion. Successful bio completions do not update the flags. An io_end can cover already-mapped folios submitted before a later unwritten extent is mapped. An earlier bio can fail while writeback first sets UNWRITTEN. The non-atomic updates can then lose a bit: Bio completion Writeback thread -------------- ---------------- read flag = 0 attach reserved handle to io_end flag |= UNWRITTEN write flag = 0 | FAILED /* UNWRITTEN is lost */ With data_err=ignore, losing UNWRITTEN makes ext4_io_end_defer_completion() return false. When the io_end reference count reaches zero, ext4_end_io_end() and its reserved-handle cleanup are therefore skipped. ext4_release_io_end() only warns about the remaining handle before freeing io_end. The leaked reservation keeps j_reserved_credits nonzero, so jbd2_journal_lock_updates() can wait indefinitely. The reverse interleaving can lose FAILED and skip a requested journal abort. Use atomic bit operations to fix the race. Fixes: e856f93e0fb2 ("ext4: do not convert the unwritten extents if data writeback fails") Cc: stable@vger.kernel.org Signed-off-by: Julian Sun --- fs/ext4/ext4.h | 16 ++++++---------- fs/ext4/page-io.c | 20 +++++++++++--------- 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index 8d3af062a830..f6b2901fc8ba 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -271,12 +271,10 @@ struct ext4_system_blocks { }; /* - * Flags for ext4_io_end->flags + * Bit numbers for ext4_io_end->flag */ -#define EXT4_IO_END_UNWRITTEN 0x0001 -#define EXT4_IO_END_FAILED 0x0002 - -#define EXT4_IO_END_DEFER_COMPLETION (EXT4_IO_END_UNWRITTEN | EXT4_IO_END_FAILED) +#define EXT4_IO_END_UNWRITTEN 0 +#define EXT4_IO_END_FAILED 1 struct ext4_io_end_vec { struct list_head list; /* list of io_end_vec */ @@ -295,7 +293,7 @@ typedef struct ext4_io_end { struct inode *inode; /* file being written to */ struct bio *bio; /* Linked list of completed * bios covering the extent */ - unsigned int flag; /* unwritten or not */ + unsigned long flag; /* EXT4_IO_END_* bits */ refcount_t count; /* reference counter */ struct list_head list_vec; /* list of ext4_io_end_vec */ } ext4_io_end_t; @@ -4093,14 +4091,12 @@ 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)) - io_end->flag |= EXT4_IO_END_UNWRITTEN; + set_bit(EXT4_IO_END_UNWRITTEN, &io_end->flag); } 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; + clear_bit(EXT4_IO_END_UNWRITTEN, &io_end->flag); } extern const struct iomap_ops ext4_iomap_ops; diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c index d409821fefa2..3604292e8a5e 100644 --- a/fs/ext4/page-io.c +++ b/fs/ext4/page-io.c @@ -146,7 +146,7 @@ static void ext4_release_io_end(ext4_io_end_t *io_end) struct bio *bio, *next_bio; BUG_ON(!list_empty(&io_end->list)); - BUG_ON(io_end->flag & EXT4_IO_END_UNWRITTEN); + BUG_ON(test_bit(EXT4_IO_END_UNWRITTEN, &io_end->flag)); WARN_ON(io_end->handle); for (bio = io_end->bio; bio; bio = next_bio) { @@ -183,7 +183,7 @@ static int ext4_end_io_end(ext4_io_end_t *io_end) * or stale data may be exposed. */ io_end->handle = NULL; /* Following call will use up the handle */ - if (unlikely(io_end->flag & EXT4_IO_END_FAILED)) { + if (unlikely(test_bit(EXT4_IO_END_FAILED, &io_end->flag))) { ret = -EIO; if (handle) jbd2_journal_free_reserved(handle); @@ -198,7 +198,7 @@ static int ext4_end_io_end(ext4_io_end_t *io_end) mapping_set_error(inode->i_mapping, ret); if (ret < 0 && !ext4_emergency_state(sb) && - io_end->flag & EXT4_IO_END_UNWRITTEN) { + test_bit(EXT4_IO_END_UNWRITTEN, &io_end->flag)) { ext4_msg(sb, KERN_EMERG, "failed to convert unwritten extents to written " "extents -- potential data loss! " @@ -235,11 +235,11 @@ static void dump_completed_IO(struct inode *inode, struct list_head *head) static bool ext4_io_end_defer_completion(ext4_io_end_t *io_end) { - if (io_end->flag & EXT4_IO_END_UNWRITTEN && + if (test_bit(EXT4_IO_END_UNWRITTEN, &io_end->flag) && !list_empty(&io_end->list_vec)) return true; if (test_opt(io_end->inode->i_sb, DATA_ERR_ABORT) && - io_end->flag & EXT4_IO_END_FAILED && + test_bit(EXT4_IO_END_FAILED, &io_end->flag) && !ext4_emergency_state(io_end->inode->i_sb)) return true; return false; @@ -254,8 +254,9 @@ static void ext4_add_complete_io(ext4_io_end_t *io_end) unsigned long flags; /* Only reserved conversions or pending IO errors will enter here. */ - WARN_ON(!(io_end->flag & EXT4_IO_END_DEFER_COMPLETION)); - WARN_ON(io_end->flag & EXT4_IO_END_UNWRITTEN && + WARN_ON(!test_bit(EXT4_IO_END_UNWRITTEN, &io_end->flag) && + !test_bit(EXT4_IO_END_FAILED, &io_end->flag)); + WARN_ON(test_bit(EXT4_IO_END_UNWRITTEN, &io_end->flag) && !io_end->handle && sbi->s_journal); WARN_ON(!io_end->bio); @@ -283,7 +284,8 @@ static int ext4_do_flush_completed_IO(struct inode *inode, while (!list_empty(&unwritten)) { io_end = list_entry(unwritten.next, ext4_io_end_t, list); - BUG_ON(!(io_end->flag & EXT4_IO_END_DEFER_COMPLETION)); + BUG_ON(!test_bit(EXT4_IO_END_UNWRITTEN, &io_end->flag) && + !test_bit(EXT4_IO_END_FAILED, &io_end->flag)); list_del_init(&io_end->list); err = ext4_end_io_end(io_end); @@ -369,7 +371,7 @@ static void ext4_end_bio(struct bio *bio) bio->bi_status, inode->i_ino, (unsigned long long) bi_sector >> (inode->i_blkbits - 9)); - io_end->flag |= EXT4_IO_END_FAILED; + set_bit(EXT4_IO_END_FAILED, &io_end->flag); mapping_set_error(inode->i_mapping, blk_status_to_errno(bio->bi_status)); } -- 2.39.5