On Mon, Mar 01, 2021 at 04:44:53PM +1100, Dave Chinner wrote:
> On Thu, Feb 25, 2021 at 09:39:05AM +0530, Chandan Babu R wrote:
> > On 23 Feb 2021 at 13:35, Dave Chinner wrote:
> > > @@ -2009,13 +2010,14 @@ xlog_sync(
> > > * synchronously here; for an internal log we can simply use the block
> > > * layer state machine for preflushes.
> > > */
> > > - if (log->l_targ != log->l_mp->m_ddev_targp || split) {
> > > + if (log->l_targ != log->l_mp->m_ddev_targp ||
> > > + (split && (iclog->ic_flags & XLOG_ICL_NEED_FLUSH))) {
> > > xfs_flush_bdev(log->l_mp->m_ddev_targp->bt_bdev);
> > > - need_flush = false;
> > > + iclog->ic_flags &= ~XLOG_ICL_NEED_FLUSH;
> > > }
> >
> > If a checkpoint transaction spans across 2 or more iclogs and the log is
> > stored on an external device, then the above would remove XLOG_ICL_NEED_FLUSH
> > flag from iclog->ic_flags causing xlog_write_iclog() to include only REQ_FUA
> > flag in the corresponding bio.
>
> Yup, good catch, this is a subtle change of behaviour only for
> external logs and only for the commit iclog that needs to flush the
> previous log writes to stable storage. I'll rework the logic here.
And now that I think about it, we can simply remove this code if we
put an explicit data device cache flush in the unmount recrod write
code. The CIL has already guaranteed metadata vs journal ordering
before we start writing the checkpoint, meaning the
XLOG_ICL_NEED_FLUSH flag only has meaning for internal iclog write
ordering, not external metadata ordering. ANd for the split, we can
simply clear the REQ_PREFLUSH flag from the split bio before
submitting it....
Much simpler and faster for external logs, too.
CHeers,
Dav.e
--
Dave Chinner
david@xxxxxxxxxxxxx
