Re: [PATCH 03/11] xfs: refactor and split xfs_log_done()

[Dave Chinner <david@fromorbit.com>]

On Wed, Mar 04, 2020 at 07:49:55AM -0800, Christoph Hellwig wrote:
> > +int
> > +xlog_write_done(
> > +	struct xlog		*log,
> >  	struct xlog_ticket	*ticket,
> >  	struct xlog_in_core	**iclog,
> > +	xfs_lsn_t		*lsn)
> >  {
> > +	if (XLOG_FORCED_SHUTDOWN(log))
> > +		return -EIO;
> >  
> > +	return xlog_commit_record(log, ticket, iclog, lsn);
> > +}
> 
> Can we just move the XLOG_FORCED_SHUTDOWN check into xlog_commit_record
> and call xlog_commit_record directly?

Next patch, because merging isn't obvious until the split is done.

> > +/*
> > + * Release or regrant the ticket reservation now the transaction is done with
> > + * it depending on caller context. Rolling transactions need the ticket
> > + * regranted, otherwise we release it completely.
> > + */
> > +void
> > +xlog_ticket_done(
> > +	struct xlog		*log,
> > +	struct xlog_ticket	*ticket,
> > +	bool			regrant)
> > +{
> >  	if (!regrant) {
> >  		trace_xfs_log_done_nonperm(log, ticket);
> >  		xlog_ungrant_log_space(log, ticket);
> >  	} else {
> >  		trace_xfs_log_done_perm(log, ticket);
> >  		xlog_regrant_reserve_log_space(log, ticket);
> >  	}
> 
> >  	xfs_log_ticket_put(ticket);
> 
> I'd move the trace points and the call to xfs_log_ticket_put into
> xlog_ungrant_log_space and xlog_regrant_reserve_log_space, and then call
> the two functions directly from the callers.  There is only a single
> place the ever regrants anyway.

Ok.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com