On Thu, Jul 22, 2021 at 08:14:32AM +0100, Christoph Hellwig wrote:
> On Thu, Jul 22, 2021 at 11:53:34AM +1000, Dave Chinner wrote:
> > +static inline int
> > +xlog_force_iclog(
> > + struct xlog_in_core *iclog)
> > +{
> > + atomic_inc(&iclog->ic_refcnt);
> > + iclog->ic_flags |= XLOG_ICL_NEED_FLUSH | XLOG_ICL_NEED_FUA;
> > + if (iclog->ic_state == XLOG_STATE_ACTIVE)
> > + xlog_state_switch_iclogs(iclog->ic_log, iclog, 0);
> > + return xlog_state_release_iclog(iclog->ic_log, iclog, 0);
> > +}
>
> This also combines code move with technical changes. At least it is
> small enough to be reviewable.
Yup, another split really needed here. And the need to catch
WANT_SYNC iclogs for flushing should really be split out, too,
because that's more than just "oops, I forgot to mark ACTIVE iclogs
we force out for flushing"...
> > out_err:
> > - if (error)
> > - xfs_alert(mp, "%s: unmount record failed", __func__);
> > -
> >
>
> > + if (error)
> > + xfs_alert(mp, "%s: unmount record failed", __func__);
> > +
> > }
>
> This now doesn't print an error when the log reservation or log write
> fails, but only one when the log force fails. Also there i a spurious
> empty line at the end.
Ah, I thought it caught them - oh, error gets overwritten. Doh! I'll
fix that up.
> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
Thanks for looking at these quickly, Christoph.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
