mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: Andrea Parri <parri.andrea@gmail.com>
Cc: Christian Brauner <brauner@kernel.org>,
	Carlos Maiolino <cem@kernel.org>,
	Joanne Koong <joannelkoong@gmail.com>,
	Brian Foster <bfoster@redhat.com>,
	Christoph Hellwig <hch@infradead.org>,
	Damien Le Moal <dlemoal@kernel.org>,
	Hannes Reinecke <hare@suse.de>,
	Daniel Gomez <da.gomez@samsung.com>,
	Pankaj Raghav <p.raghav@samsung.com>,
	Dave Chinner <dchinner@redhat.com>,
	linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH v2 4/4] iomap: don't lose a failed direct I/O bio's error when zeroing the tail
Date: Thu, 24 Sep 2026 15:36:20 -0700	[thread overview]
Message-ID: <20260924223620.GM2705364@frogsfrogsfrogs> (raw)
In-Reply-To: <20260924203013.9967-2-parri.andrea@gmail.com>

On Thu, Sep 24, 2026 at 10:30:08PM +0200, Andrea Parri wrote:
> On Thu, Sep 24, 2026 at 11:53:17AM -0700, Darrick J. Wong wrote:
> > > Cc: stable@vger.kernel.org
> > 
> > Cc: <stable@vger.kernel.org> # v6.12
> 
> Fixed for v3.
> 
> > > @@ -582,8 +578,7 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
> > >  		/* zero out from the end of the write to the end of the block */
> > >  		pad = pos & (fs_block_size - 1);
> > >  		if (pad)
> > > -			ret = iomap_dio_zero(iter, dio, pos,
> > > -					     fs_block_size - pad);
> > > +			iomap_dio_zero(iter, dio, pos, fs_block_size - pad);
> > 
> > I think this is the original cause of the bug, right?  We might have
> > already had a nonzero ret, and the assignment here blows that away.
> > Right?
> 
> Right.  When iomap_dio_bio_iter_one() fails we break out of the loop
> with ret < 0 and fall through to the tail zeroing, and this assignment
> replaced that error with iomap_dio_zero()'s 0.
> 
> > If the answer to that is yes, then
> > Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
> 
> Thanks, picked up for v3.
> 
> > (I should probably whine about how changing the function signature of
> > iomap_dio_zero should be a separate patch to make it more obvious what's
> > the actual fix...)
> 
> Fair point.  FWIW, before 10553a91652d iomap_dio_zero() returned void
> and neither call site touched ret; that commit made it return int and
> added both "ret = iomap_dio_zero(...)" assignments.  So the signature
> change here is really undoing the part of 10553a91652d that introduced
> the bug, and keeping it in one patch makes the stable backport
> self-contained.  If you'd still prefer a one-line fix followed by the
> void conversion, I'm happy to split it.

I'd prefer that, though I don't have a strong opinion.  Someone in
another multiverse could have come along and decided "URrgghg,
iomap_dio_zero doesn't need a farking return value!" and accidentally
fixed this bug.

Eh, whatever, just leave it as it is. :)

--D

> Thanks!
>   Andrea
> 

  reply	other threads:[~2026-09-24 22:36 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-24  9:11 [PATCH v2 0/4] iomap: fix error handling regressions Andrea Parri
2026-09-24  9:11 ` [PATCH v2 1/4] iomap: don't resubmit an ioend after ->writeback_submit() failed Andrea Parri
2026-09-24 18:49   ` Darrick J. Wong
2026-09-24 20:30     ` Andrea Parri
2026-09-24 22:33       ` Darrick J. Wong
2026-09-25  4:46   ` Christoph Hellwig
2026-09-24  9:11 ` [PATCH v2 2/4] xfs: add an error tag to inject a ->writeback_submit() failure Andrea Parri
2026-09-24 18:49   ` Darrick J. Wong
2026-09-25  4:46   ` Christoph Hellwig
2026-09-24  9:11 ` [PATCH v2 3/4] iomap: don't lose a fiemap iteration error when emitting the last extent Andrea Parri
2026-09-24  9:11 ` [PATCH v2 4/4] iomap: don't lose a failed direct I/O bio's error when zeroing the tail Andrea Parri
2026-09-24 18:53   ` Darrick J. Wong
2026-09-24 20:30     ` Andrea Parri
2026-09-24 22:36       ` Darrick J. Wong [this message]
2026-09-25  4:48   ` Christoph Hellwig
2026-09-25  6:43   ` Pankaj Raghav (Samsung)

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=20260924223620.GM2705364@frogsfrogsfrogs \
    --to=djwong@kernel.org \
    --cc=bfoster@redhat.com \
    --cc=brauner@kernel.org \
    --cc=cem@kernel.org \
    --cc=da.gomez@samsung.com \
    --cc=dchinner@redhat.com \
    --cc=dlemoal@kernel.org \
    --cc=hare@suse.de \
    --cc=hch@infradead.org \
    --cc=joannelkoong@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=p.raghav@samsung.com \
    --cc=parri.andrea@gmail.com \
    --cc=stable@vger.kernel.org \
    /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

all inboxes | Powered by JetHome®