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: > > 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@xxxxxxxxxxxxx
