Re: [PATCH v2] ext4: fix warning when submitting superblock in ext4_commit_super()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
>



[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux