Re: [RFC PATCH v4 05/12] iomap: Add initial support for buffered RWF_WRITETHROUGH

[Ojaswin Mujoo]

On Thu, Oct 15, 2026 at 04:09:50PM +0200, Pankaj Raghav (Samsung) wrote:
> Hi Ojaswin,
> 
> I was comparing normal buffered IO scenario and writethrough.
> 
> In a normal buffered IO scenario, for unwritten extents or CoW extents,
> we first convert them before ending the writeback on the folio.
> xfs_end_io calls xfs_end_ioend_write().
> 
> static void
> xfs_end_ioend_write(
>         struct iomap_ioend      *ioend)
> {
> <snip>
>        /*
>          * Success: commit the COW or unwritten blocks if needed.
>          */
>         if (is_zoned)
>                 error = xfs_zoned_end_io(ip, offset, size, ioend->io_sector,
>                                 ioend->io_private, NULLFSBLOCK);
>         else if (ioend->io_flags & IOMAP_IOEND_SHARED)
>                 error = xfs_reflink_end_cow(ip, offset, size);
>         else if (ioend->io_flags & IOMAP_IOEND_UNWRITTEN)
>                 error = xfs_iomap_write_unwritten(ip, offset, size, false);
> 
>         if (!error &&
>             !(ioend->io_flags & IOMAP_IOEND_DIRECT) &&
>             xfs_ioend_is_append(ioend))
>                 error = xfs_setfilesize(ip, offset, size);
> done:
>         if (is_zoned)
>                 xfs_ioend_put_open_zones(ioend);
>         iomap_finish_ioends(ioend, error); // #1
>         memalloc_nofs_restore(nofs_flag);
> }
> 
> #1
> iomap_finish_ioends()
>   iomap_finish_ioend_buffered_write()
>     iomap_finish_folio_write()
>       folio_end_writeback()
> 
> 
> On Mon, Sep 28, 2026 at 05:33:06PM +0530, Ojaswin Mujoo wrote:
> > @@ -1185,6 +1186,384 @@ static bool iomap_write_end(struct iomap_iter *iter, size_t len, size_t copied,
> >  	return __iomap_write_end(iter->inode, pos, len, copied, folio);
> >  }
> >  
> > +static ssize_t iomap_writethrough_complete(struct iomap_writethrough_ctx *wt_ctx)
> > +{
> > +	struct kiocb *iocb	= wt_ctx->iocb;
> > +	struct inode *inode	= wt_ctx->inode;
> > +	ssize_t ret		= wt_ctx->error;
> > +
> > +	if (wt_ctx->end_io) {
> > +		int err = wt_ctx->end_io(wt_ctx, wt_ctx->written,
> > +					 wt_ctx->error,
> > +					 wt_ctx->flags);
> > +		if (err)
> > +			ret = err;
> > +	}
> > +
> 
> <snip>
> 
> > +static void iomap_writethrough_bio_end_io(struct bio *bio)
> > +{
> > +	struct iomap_writethrough_ctx *wt_ctx = bio->bi_private;
> > +	struct folio_iter fi;
> > +
> > +	if (bio->bi_status)
> > +		cmpxchg(&wt_ctx->error, 0,
> > +			blk_status_to_errno(bio->bi_status));
> > +	bio_for_each_folio_all(fi, bio)
> > +		folio_end_writeback(fi.folio);
> > +
> > +	bio_put(bio);
> > +	if (atomic_dec_and_test(&wt_ctx->ref))
> > +		iomap_writethrough_done(wt_ctx);
> > +}
> > +
> 
> But in the writethrough patches, we first end the writeback on the folio
> and then call xfs_dio_write_end_io(), which converts unwritten or CoW
> extents.
> 
> Is there a race possible as we are doing the other way? What if folios
> are reclaimed between ending the writeback and conversion? A new read in
> that area will return zero filled folios as XFS still sees IOMAP_UNWRITTEN?
> 
> I don't have a reproducer, but my hunch says we can run into this if we
> have a lot of AIO as the end_io will be run via a workqueue?
> 
> Do we do something similar to buffered IO where we do
> folio_end_writeback after extents conversion?

Hey Pankaj, thanks for pointing this out. I agree that this can open up
some races like you mentioned. I believe the right things will be to
have an buffered endio like flow however we will need to store the
folios somewhere till the whole conversion is done, like buffered endio
does in struct ioend->io_bio. Hmm I think it would be ideal if we could
somehow reuse some of the logic there. I'll see if its possible.

Thanks,
ojaswin


> 
> --
> Pankaj