On Fri, Jul 02, 2021 at 09:48:23AM +0100, Christoph Hellwig wrote:
> On Wed, Jun 30, 2021 at 04:38:12PM +1000, Dave Chinner wrote:
> > The problem is that xlog_state_release_iclog() aborts before doing
> > anything if the log is already shut down. It assumes that the
> > callbacks have already been cleaned up, and it doesn't need to do
> > any cleanup.
> >
> > Hence the fix is to remove the xlog_is_shutdown() check from
> > xlog_state_release_iclog() so that reference counts are correctly
> > released from the iclogs, and when the reference count is zero we
> > always transition to SYNCING if the log is shut down. Hence we'll
> > always enter the xlog_sync() path in a shutdown and eventually end
> > up erroring out the iclog IO and running xlog_state_do_callback() to
> > process the callbacks attached to the iclog.
>
> Ok, this answers my question to the previous patch. Maybe add a little
> blurb there?
Done.
> > + if (xlog_is_shutdown(log)) {
> > + /*
> > + * No more references to this iclog, so process the pending
> > + * iclog callbacks that were waiting on the release of this
> > + * iclog.
> > + */
> > + spin_unlock(&log->l_icloglock);
> > + xlog_state_shutdown_callbacks(log);
> > + spin_lock(&log->l_icloglock);
> > + } else if (xlog_state_want_sync(log, iclog)) {
> > spin_unlock(&log->l_icloglock);
> > xlog_sync(log, iclog);
> > spin_lock(&log->l_icloglock);
>
> >
> > +out_check_shutdown:
> > + if (xlog_is_shutdown(log))
> > + return -EIO;
> > return 0;
>
> Nit: we can just return -EIO directly in the first xlog_is_shutdown
> block.. It's not going to make any difference for the CPU, but makes
> the code a little easier to follow.
Fixed.
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
Thanks.
-Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
