Re: [PATCH 2/7] xfs: use xfs_defer_pending objects to recover intent items

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Nov 29, 2023 at 11:53:31PM -0800, Christoph Hellwig wrote:
> > +/* Create a pending deferred work item for use with log recovery. */
> > +struct xfs_defer_pending *
> > +xfs_defer_start_recovery(
> > +	struct xfs_log_item		*lip,
> > +	enum xfs_defer_ops_type		dfp_type)
> > +{
> > +	struct xfs_defer_pending	*dfp;
> > +
> > +	dfp = kmem_cache_zalloc(xfs_defer_pending_cache,
> > +			GFP_NOFS | __GFP_NOFAIL);
> > +	dfp->dfp_type = dfp_type;
> > +	dfp->dfp_intent = lip;
> > +	INIT_LIST_HEAD(&dfp->dfp_work);
> > +	INIT_LIST_HEAD(&dfp->dfp_list);
> > +	return dfp;
> > +}
> 
> Initializing dfp_list here is a bit pointless as the caller instantly
> adds it to log->l_dfops.

I'd rather pass the list_head into xfs_defer_start_recovery so that it
can create a fully initialized dfp and add it to the tracker.

> > -#if defined(DEBUG) || defined(XFS_WARN)
> > +	spin_lock(&log->l_ailp->ail_lock);
> >  	last_lsn = xlog_assign_lsn(log->l_curr_cycle, log->l_curr_block);
> > +	spin_unlock(&log->l_ailp->ail_lock);
> 
> ail_lock shouldn't be needed here.

Oh, right, the AIL lock was to protect the cursor, not the debug
statements.

> Otherwise this looks reasonable to me.

<nod>

--D