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?
> + 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.
Otherwise looks good:
Reviewed-by: Christoph Hellwig <hch@xxxxxx>
