Re: [PATCH v2 1/2] xfs: number the cache flushes submitted to each buftarg

[Brian Foster <bfoster@redhat.com>]

On Wed, Nov 11, 2026 at 12:17:11PM +0900, Daejun Park via B4 Relay wrote:
> From: Daejun Park <daejun7.park@samsung.com>
> 
> fsync can only skip its own cache flush when its log force wrote out
> the iclog. It cannot tell whether a flush that another task submitted
> covers its data.
> 
> Number the cache flushes that the log code submits to each buftarg,
> and record the highest number that completed without error. Nothing
> reads the numbers yet.
> 
> Signed-off-by: Daejun Park <daejun7.park@samsung.com>
> ---
>  fs/xfs/xfs_buf.c      | 13 +++++++++++++
>  fs/xfs/xfs_buf.h      | 21 +++++++++++++++++++++
>  fs/xfs/xfs_log.c      | 28 +++++++++++++++++++++-------
>  fs/xfs/xfs_log_priv.h |  1 +
>  4 files changed, 56 insertions(+), 7 deletions(-)
> 
...
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 74b324f915a..d99a5b678cf 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -1211,6 +1211,8 @@ xlog_ioend_work(
>  		xlog_force_shutdown(log, SHUTDOWN_LOG_IO_ERROR);
>  	}
>  
> +	if (!error && iclog->ic_flush_seq)
> +		xfs_buftarg_flush_done(log->l_targ, iclog->ic_flush_seq);

Taking another look at this I started to also wonder whether to store
this flush seq in the xlog_flush_done struct. It looks like we reuse
that for multiple async flushes in flight, however, so probably not
worth the complication to try and track all that there.

Thanks for the tweaks and this all looks reasonable to me:

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  	xlog_state_done_syncing(iclog);
>  	bio_uninit(&iclog->ic_bio);
>  
> @@ -1570,19 +1572,21 @@ xlog_flush_end_io(
>  	bio_put(bio);
>  }
>  
> -static void
> +static s64
>  xlog_flush_async(
>  	struct xlog_flush_done	*done,
> -	struct block_device	*bdev)
> +	struct xfs_buftarg	*btp)
>  {
>  	struct bio		*bio;
> +	s64			seq = xfs_buftarg_flush_start(btp);
>  
> -	bio = bio_alloc(bdev, 0, REQ_OP_WRITE | REQ_PREFLUSH | REQ_SYNC,
> +	bio = bio_alloc(btp->bt_bdev, 0, REQ_OP_WRITE | REQ_PREFLUSH | REQ_SYNC,
>  			GFP_NOFS);
>  	bio->bi_private = done;
>  	bio->bi_end_io = xlog_flush_end_io;
>  	atomic_inc(&done->pending);
>  	submit_bio(bio);
> +	return seq;
>  }
>  
>  /*
> @@ -1605,16 +1609,17 @@ xlog_flush_data_caches(
>  		.pending	= ATOMIC_INIT(1),
>  		.done		= COMPLETION_INITIALIZER_ONSTACK(done.done),
>  	};
> +	s64			dseq = 0, rseq = 0, lseq;
>  
>  	*did_flush = false;
>  	if (mp->m_ddev_targp != mp->m_logdev_targp &&
>  	    bdev_write_cache(mp->m_ddev_targp->bt_bdev)) {
> -		xlog_flush_async(&done, mp->m_ddev_targp->bt_bdev);
> +		dseq = xlog_flush_async(&done, mp->m_ddev_targp);
>  		*did_flush = true;
>  	}
>  	if (mp->m_rtdev_targp && mp->m_rtdev_targp != mp->m_ddev_targp &&
>  	    bdev_write_cache(mp->m_rtdev_targp->bt_bdev)) {
> -		xlog_flush_async(&done, mp->m_rtdev_targp->bt_bdev);
> +		rseq = xlog_flush_async(&done, mp->m_rtdev_targp);
>  		*did_flush = true;
>  	}
>  
> @@ -1623,12 +1628,17 @@ xlog_flush_data_caches(
>  		 * If we flushed any other device, also use an async flush for
>  		 * the log device so that all flushes happen in parallel.
>  		 */
> -		xlog_flush_async(&done, mp->m_logdev_targp->bt_bdev);
> +		lseq = xlog_flush_async(&done, mp->m_logdev_targp);
>  
>  		xlog_flush_done(&done);
>  		wait_for_completion(&done.done);
>  		if (done.status)
>  			return -EIO;
> +		if (dseq)
> +			xfs_buftarg_flush_done(mp->m_ddev_targp, dseq);
> +		if (rseq)
> +			xfs_buftarg_flush_done(mp->m_rtdev_targp, rseq);
> +		xfs_buftarg_flush_done(mp->m_logdev_targp, lseq);
>  	}
>  
>  	return 0;
> @@ -1678,14 +1688,18 @@ xlog_write_iclog(
>  	iclog->ic_bio.bi_iter.bi_sector = log->l_logBBstart + bno;
>  	iclog->ic_bio.bi_end_io = xlog_bio_end_io;
>  	iclog->ic_bio.bi_private = iclog;
> +	iclog->ic_flush_seq = 0;
>  
>  	if (iclog->ic_flags & XLOG_ICL_NEED_FLUSH) {
>  		if (xlog_flush_data_caches(log->l_mp, &did_flush))
>  			goto shutdown;
>  	}
>  	if (!did_flush &&
> -	    (iclog->ic_flags & (XLOG_ICL_NEED_FLUSH | XLOG_ICL_NEED_FLUSH_LOG)))
> +	    (iclog->ic_flags &
> +	     (XLOG_ICL_NEED_FLUSH | XLOG_ICL_NEED_FLUSH_LOG))) {
>  		iclog->ic_bio.bi_opf |= REQ_PREFLUSH;
> +		iclog->ic_flush_seq = xfs_buftarg_flush_start(log->l_targ);
> +	}
>  	if (iclog->ic_flags & XLOG_ICL_NEED_FUA)
>  		iclog->ic_bio.bi_opf |= REQ_FUA;
>  
> diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
> index 529720a4984..df0f13c4e04 100644
> --- a/fs/xfs/xfs_log_priv.h
> +++ b/fs/xfs/xfs_log_priv.h
> @@ -222,6 +222,7 @@ struct xlog_in_core {
>  #endif
>  	struct semaphore	ic_sema;
>  	struct work_struct	ic_end_io_work;
> +	s64			ic_flush_seq;	/* buftarg flush number */
>  	struct bio		ic_bio;
>  	struct bio_vec		ic_bvec[];
>  };
> 
> -- 
> 2.43.0
> 
> 
>