[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220520042405.xncu55ceekgrzhyu@riteshh-domain>
Date: Fri, 20 May 2022 09:54:05 +0530
From: Ritesh Harjani <ritesh.list@...il.com>
To: Zhang Yi <yi.zhang@...wei.com>
Cc: linux-ext4@...r.kernel.org, tytso@....edu,
adilger.kernel@...ger.ca, jack@...e.cz, yukuai3@...wei.com
Subject: Re: [PATCH v2] ext4: fix warning when submitting superblock in
ext4_commit_super()
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@...wei.com>
> Signed-off-by: Zhang Yi <yi.zhang@...wei.com>
> ---
> 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@...il.com>
-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
>
Powered by blists - more mailing lists