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

Dave Chinner
2020-03-04T21:28:23Z

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