Linux EXT4 FS development
 help / color / mirror / Atom feed
From: Dan Carpenter <error27@gmail.com>
To: Zhang Yi <yi.zhang@huaweicloud.com>
Cc: oe-kbuild@lists.linux.dev, lkp@intel.com,
	oe-kbuild-all@lists.linux.dev, linux-ext4@vger.kernel.org,
	Theodore Ts'o <tytso@mit.edu>
Subject: Re: [tytso-ext4:dev 40/41] fs/ext4/inode.c:4518 ext4_iomap_writepages() warn: missing error code? 'ret'
Date: Fri, 16 Oct 2026 12:05:50 +0300	[thread overview]
Message-ID: <atHo7i5Uq9cdeVFt@stanley.mountain> (raw)
In-Reply-To: <1165ce1d-6485-4b70-9949-47ee071a0a22@huaweicloud.com>

On Fri, Oct 16, 2026 at 04:49:10PM +0800, Zhang Yi wrote:
> On 10/16/2026 3:58 PM, Dan Carpenter wrote:
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
> > head:   8c4f6e3376348fc3162eed86a05b933921c8f9ed
> > commit: bdcbb0f19e921c7ea3241f55922032dff23d1d26 [40/41] ext4: partially enable iomap for the buffered I/O path of regular files
> > config: x86_64-randconfig-2200-20261016 (https://download.01.org/0day-ci/archive/20261016/202610160314.PLMEXaOJ-lkp@intel.com/config)
> > compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
> > smatch: v0.5.0-9187-g5189e3fb
> > 
> > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Reported-by: Dan Carpenter <error27@gmail.com>
> > | Closes: https://lore.kernel.org/r/202610160314.PLMEXaOJ-lkp@intel.com/
> > 
> > New smatch warnings:
> > fs/ext4/inode.c:4518 ext4_iomap_writepages() warn: missing error code? 'ret'
> > 
> > vim +/ret +4518 fs/ext4/inode.c
> > 
> > a304299a40d92c Zhang Yi 2026-10-09  4488  static int ext4_iomap_writepages(struct address_space *mapping,
> > a304299a40d92c Zhang Yi 2026-10-09  4489  				 struct writeback_control *wbc)
> > a304299a40d92c Zhang Yi 2026-10-09  4490  {
> > a2e096bf59634f Zhang Yi 2026-10-09  4491  	struct inode *inode = mapping->host;
> > a2e096bf59634f Zhang Yi 2026-10-09  4492  	struct super_block *sb = inode->i_sb;
> > a2e096bf59634f Zhang Yi 2026-10-09  4493  	long nr = wbc->nr_to_write;
> > a2e096bf59634f Zhang Yi 2026-10-09  4494  	int alloc_ctx, ret;
> > a2e096bf59634f Zhang Yi 2026-10-09  4495  	struct iomap_writepage_ctx wpc = {
> > a2e096bf59634f Zhang Yi 2026-10-09  4496  		.inode = inode,
> > a2e096bf59634f Zhang Yi 2026-10-09  4497  		.wbc = wbc,
> > a2e096bf59634f Zhang Yi 2026-10-09  4498  		.ops = &ext4_writeback_ops,
> > a2e096bf59634f Zhang Yi 2026-10-09  4499  	};
> > a2e096bf59634f Zhang Yi 2026-10-09  4500  
> > a2e096bf59634f Zhang Yi 2026-10-09  4501  	ret = ext4_emergency_state(sb);
> > a2e096bf59634f Zhang Yi 2026-10-09  4502  	if (unlikely(ret))
> > a2e096bf59634f Zhang Yi 2026-10-09  4503  		return ret;
> > a2e096bf59634f Zhang Yi 2026-10-09  4504  
> > 53fe71a6bcad0f Zhang Yi 2026-10-09  4505  	/*
> > 53fe71a6bcad0f Zhang Yi 2026-10-09  4506  	 * Submit the pending zeroed EOF block range if the entire
> > 53fe71a6bcad0f Zhang Yi 2026-10-09  4507  	 * writeback range lies beyond it.
> > 53fe71a6bcad0f Zhang Yi 2026-10-09  4508  	 */
> > 53fe71a6bcad0f Zhang Yi 2026-10-09  4509  	ext4_iomap_wb_submit_zeroed_eof(inode, wbc);
> > 53fe71a6bcad0f Zhang Yi 2026-10-09  4510  
> > a2e096bf59634f Zhang Yi 2026-10-09  4511  	alloc_ctx = ext4_writepages_down_read(sb);
> > bdcbb0f19e921c Zhang Yi 2026-10-09  4512  	/*
> > bdcbb0f19e921c Zhang Yi 2026-10-09  4513  	 * Does ext4_change_inode_journal_flag() change the inode's
> > bdcbb0f19e921c Zhang Yi 2026-10-09  4514  	 * buffered I/O path?
> > bdcbb0f19e921c Zhang Yi 2026-10-09  4515  	 */
> > bdcbb0f19e921c Zhang Yi 2026-10-09  4516  	if (!ext4_inode_buffered_iomap(inode)) {
> > bdcbb0f19e921c Zhang Yi 2026-10-09  4517  		ext4_writepages_up_read(sb, alloc_ctx);
> > bdcbb0f19e921c Zhang Yi 2026-10-09 @4518  		return ret;
> > 
> > Either return 0 or an error code here, but not ret.
> 
> Thanks for pointing that out, but this is a false positive. No error
> code needs to be set here. This path is only taken when the writeback
> races with a concurrent journal flag switch, and since the switch will
> write back all dirty data, there is nothing to be done for the leftover
> writeback here and we can just return directly. So the 'ret' will simply
> be 0 here.
> 

Yes, please change it to "return 0;" so it's more readable.

regards,
dan carpenter


  reply	other threads:[~2026-10-16  9:05 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-16  7:58 [tytso-ext4:dev 40/41] fs/ext4/inode.c:4518 ext4_iomap_writepages() warn: missing error code? 'ret' Dan Carpenter
2026-10-16  8:49 ` Zhang Yi
2026-10-16  9:05   ` Dan Carpenter [this message]
2026-10-16  9:20     ` Zhang Yi
2026-10-16  9:59 ` [PATCH] ext4: return 0 instead of stale ret in ext4_iomap_writepages() Zhang Yi
2026-10-16 10:09   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=atHo7i5Uq9cdeVFt@stanley.mountain \
    --to=error27@gmail.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=lkp@intel.com \
    --cc=oe-kbuild-all@lists.linux.dev \
    --cc=oe-kbuild@lists.linux.dev \
    --cc=tytso@mit.edu \
    --cc=yi.zhang@huaweicloud.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox