Re: [PATCH 8/9] xfs: don't run shutdown callbacks on active iclogs

[Dave Chinner <david@fromorbit.com>]

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@lst.de>

Thanks.

-Dave.
-- 
Dave Chinner
david@fromorbit.com