Re: [PATCH 4/8] xfs: l_last_sync_lsn is really tracking AIL state

[Dave Chinner <david@fromorbit.com>]

On Sun, Jul 10, 2022 at 11:42:52PM -0700, Christoph Hellwig wrote:
> On Fri, Jul 08, 2022 at 11:55:54AM +1000, Dave Chinner wrote:
> > +		/*
> > +		 * If there are no callbacks on this iclog, we can mark it clean
> > +		 * immediately and return. Otherwise we need to run the
> > +		 * callbacks.
> > +		 */
> > +		if (list_empty(&iclog->ic_callbacks)) {
> > +			xlog_state_clean_iclog(log, iclog);
> > +			return false;
> > +		}
> > +		trace_xlog_iclog_callback(iclog, _RET_IP_);
> > +		iclog->ic_state = XLOG_STATE_CALLBACK;
> 
> Can you split the optimization of skipping the XLOG_STATE_CALLBACK
> state out?  It seems unrelated to the rest and really confused me
> when trying to understand this patch. 

OK.

> 
> > +static inline void
> > +xfs_ail_assign_tail_lsn(
> > +	struct xfs_ail		*ailp)
> > +{
> > +
> > +	spin_lock(&ailp->ail_lock);
> > +	xfs_ail_update_tail_lsn(ailp);
> > +	spin_unlock(&ailp->ail_lock);
> > +}
> 
> This naming scheme seems a lot more confusing than the old _locked
> suffix or the __ prefix.

Easy enough.

-Dave.

-- 
Dave Chinner
david@fromorbit.com