Re: [PATCH v2 1/4] ext4: return 0 instead of stale ret in ext4_iomap_writepages()

[Jan Kara]

On Fri 30-10-26 10:13:35, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@xxxxxxxxxx>
> 
> smatch reports a "missing error code" warning:
> 
>   fs/ext4/inode.c:4518 ext4_iomap_writepages() warn: missing error code? 'ret'
> 
> The 'ret' variable has already been consumed by the
> ext4_emergency_state() check at the beginning of the function, so its
> value is always zero at this point. Returning it is equivalent to
> returning 0, but it is misleading because it looks like an error code is
> being propagated.
> 
> This branch handles the race with ext4_change_inode_journal_flag(),
> which flushes the dirty data and drops the page cache before switching
> the inode to the buffer_head path, so there is genuinely nothing to
> write back here. Return 0 explicitly to make the intent clear.
> 
> Fixes: bdcbb0f19e92 ("ext4: partially enable iomap for the buffered I/O path of regular files")
> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> Reported-by: Dan Carpenter <error27@xxxxxxxxx>
> Closes: https://lore.kernel.org/r/202610160314.PLMEXaOJ-lkp@xxxxxxxxx/
> Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx>

Sure. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> ---
>  fs/ext4/inode.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index b4d581aa5ee1a..6b2f1b07f41e6 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4515,7 +4515,7 @@ static int ext4_iomap_writepages(struct address_space *mapping,
>  	 */
>  	if (!ext4_inode_buffered_iomap(inode)) {
>  		ext4_writepages_up_read(sb, alloc_ctx);
> -		return ret;
> +		return 0;
>  	}
>  
>  	trace_ext4_writepages(inode, wbc);
> -- 
> 2.52.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR