On Tue, Mar 09, 2021 at 09:10:47PM -0500, Brian Foster wrote:
> On Mon, Mar 08, 2021 at 08:35:59PM -0800, Darrick J. Wong wrote:
> > On Tue, Mar 09, 2021 at 11:44:10AM +1100, Dave Chinner wrote:
> > > On Fri, Mar 05, 2021 at 09:58:26AM -0500, Brian Foster wrote:
> > > I'm not going to play that "now jump through this hoop" game. We
> > > add flags for on-off behaviours in internal functions -all the
> > > time-. If this makes the interface so complex and confusing that you
> > > don't understand it, then the interface was already too complex and
> > > confusing. And fixing that is *not in the scope of this patchset*.
> > >
> > > Demanding that code be made perfect before it can be merged is
> > > really not very helpful. Especially when there are already plans to
> > > rework the API but that rework is dependent on a bunch of other
> > > changes than need to be done first.
> > >
> > > iclogs are something that need to be moved behind the CIL, not sit
> > > in front of CIL. The CIL abstracts the journal and writing to the
> > > journal completely away from the transaction subsystem, yet the log
> > > force code punches straight through that abstraction and walks
> > > iclogs directly. The whole log force implementation needs to change,
> > > and I plan for the API that wraps the log forces to get reworked at
> > > that time.
> >
> > So here's what I want to know: Do Dave's changes to the log force APIs
> > introduce broken behavior? If the interfaces are so confusing that
> > /none/ of us understand it, can we introduce the appropriate wrappers
> > and documentation so that the rest of us plugging away at the rest of
> > the system can only call it the supported ways to achieve any of the
> > supported outcomes?
> >
>
> It looks to me that the changes to xlog_cil_force_seq() could
> essentially be replaced with something like:
>
> /*
> * Behold the magical async CIL force
> * ... <explain what this does> ...
> */
> static inline void
> xfs_cil_force_async(
> struct xlog *log)
> {
> struct xfs_cil *cil = log->l_cilp;
>
> /* true for magic async CIL force */
> xlog_cil_push_now(log, cil->xc_current_sequence, true);
> }
Oh, is that what you are asking for? You were talking about changing
the API for all the callers that didn't use XFS_LOG_SYNC to make
them all async, not about a one-off, single use wrapper for the AIL.
If all you want is a single line wrapper function, then don't
suggest that the whole API should be reworked and callers changed to
use a new API. *Ask for a one-line wrapper to be added*.
This wrapper will still need to go away in the near future, but at
least it doesn't involve changing lots of unrelated code...
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
