On 22/05/20 10:32AM, Zhang Yi wrote:
> We have already check the io_error and uptodate flag before submitting
> the superblock buffer, and re-set the uptodate flag if it has been
> failed to write out. But it was lockless and could be raced by another
> ext4_commit_super(), and finally trigger '!uptodate' WARNING when
> marking buffer dirty. Fix it by submit buffer directly.
>
> Reported-by: Hulk Robot <hulkci@xxxxxxxxxx>
> Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx>
> ---
> v2->v1:
> - Add clear_buffer_dirty() before submit_bh().
Thanks for the patch.
Reviewed lock_buffer()/unlock_buffer(), get_bh/put_bh() and other possible error
paths, looks good to me.
Feel free to add -
Reviewed-by: Ritesh Harjani <ritesh.list@xxxxxxxxx>
-ritesh
>
> fs/ext4/super.c | 22 ++++++++++++++++------
> 1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 1466fbdbc8e3..9f6892665be4 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -6002,7 +6002,6 @@ static void ext4_update_super(struct super_block *sb)
> static int ext4_commit_super(struct super_block *sb)
> {
> struct buffer_head *sbh = EXT4_SB(sb)->s_sbh;
> - int error = 0;
>
> if (!sbh)
> return -EINVAL;
> @@ -6011,6 +6010,13 @@ static int ext4_commit_super(struct super_block *sb)
>
> ext4_update_super(sb);
>
> + lock_buffer(sbh);
> + /* Buffer got discarded which means block device got invalidated */
> + if (!buffer_mapped(sbh)) {
> + unlock_buffer(sbh);
> + return -EIO;
> + }
> +
> if (buffer_write_io_error(sbh) || !buffer_uptodate(sbh)) {
> /*
> * Oh, dear. A previous attempt to write the
> @@ -6025,17 +6031,21 @@ static int ext4_commit_super(struct super_block *sb)
> clear_buffer_write_io_error(sbh);
> set_buffer_uptodate(sbh);
> }
> - BUFFER_TRACE(sbh, "marking dirty");
> - mark_buffer_dirty(sbh);
> - error = __sync_dirty_buffer(sbh,
> - REQ_SYNC | (test_opt(sb, BARRIER) ? REQ_FUA : 0));
> + get_bh(sbh);
> + /* Clear potential dirty bit if it was journalled update */
> + clear_buffer_dirty(sbh);
> + sbh->b_end_io = end_buffer_write_sync;
> + submit_bh(REQ_OP_WRITE,
> + REQ_SYNC | (test_opt(sb, BARRIER) ? REQ_FUA : 0), sbh);
> + wait_on_buffer(sbh);
> if (buffer_write_io_error(sbh)) {
> ext4_msg(sb, KERN_ERR, "I/O error while writing "
> "superblock");
> clear_buffer_write_io_error(sbh);
> set_buffer_uptodate(sbh);
> + return -EIO;
> }
> - return error;
> + return 0;
> }
>
> /*
> --
> 2.31.1
>
