On Wed, Mar 03, 2021 at 11:41:19AM +1100, Dave Chinner wrote:

On Mon, Mar 01, 2021 at 10:19:36AM -0500, Brian Foster wrote:

quoted

On Tue, Feb 23, 2021 at 02:34:36PM +1100, Dave Chinner wrote:
That aside, this iteration logic all seems a bit overengineered to me.
We have the commit record iclog of the current checkpoint and thus the
immediately previous iclog in the ring. We know that previous record
isn't earlier than start_lsn because the caller confirmed that start_lsn
!= commit_lsn. We also know that iclog can't become dirty -> active
until it and all previous iclog writes have completed because the
callback ordering implemented by xlog_state_do_callback() won't clean
the iclog until that point. Given that, can't this whole thing be
replaced with a check of iclog->prev to either see if it's been cleaned
or to otherwise xlog_wait() for that condition and return?

Maybe. I was more concerned about ensuring that it did the right
thing so I checked all the things that came to mind. There was more
than enough compexity in other parts of this patchset to fill my
brain that minimal implementation were not a concern. I'll go take
another look at it.

Ok, so we can just use xlog_wait_on_iclog() here. I didn't look too
closely at the implementation of that function, just took the
comment above it at face value that it only waited for an iclog to
hit the disk.

We actually have two different iclog IO completion wait points - one
to wait for an iclog to hit the disk, and one to wait for hit the
disk and run completion callbacks. i.e. one is not ordered against
other iclogs and the other is strictly ordered.

The ordered version runs completion callbacks before waking waiters
thereby guaranteeing all previous iclog have been completed before
completing the current iclog and waking waiters.

The CIL code needs the latter, so yes, this can be simplified down
to a single xlog_wait_on_iclog(commit_iclog->ic_prev); call from the
CIL.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com