On Wed, Feb 19, 2020 at 08:52:43AM +1100, Dave Chinner wrote:
> xfs_force_shutdown() will does nothing if the iclog at the head of
> the log->iclog list is marked with XLOG_STATE_IOERROR before IO is
> submitted. In general, that is the case here as the head iclog is
> what xlog_state_get_iclog_space() returns.
>
> i.e. XLOG_STATE_IOERROR here implies the log has already been shut
> down because the state is IOERROR rather than ACTIVE or WANT_SYNC as
> was set when the iclog was obtained from
> xlog_state_get_iclog_space().
>
> > + if (iclog->ic_state == XLOG_STATE_IOERROR)
> > + goto error;
> >
> > if (atomic_dec_and_lock(&iclog->ic_refcnt, &log->l_icloglock)) {
> > + if (iclog->ic_state == XLOG_STATE_IOERROR) {
> > + spin_unlock(&log->l_icloglock);
> > + goto error;
> > + }
> > sync = __xlog_state_release_iclog(log, iclog);
> > spin_unlock(&log->l_icloglock);
> > if (sync)
> > xlog_sync(log, iclog);
> > }
> > return 0;
> > +error:
> > + xfs_force_shutdown(mp, SHUTDOWN_LOG_IO_ERROR);
> > + return -EIO;
>
> Hence I suspect that this should be ASSERT(XLOG_FORCED_SHUTDOWN(log))
> just like is in xfs_log_force_umount() when this pre-existing log
> IO error condition is tripped over...
Indeed, the xfs_force_shutdown is a no-op both for the old and this
new check.
Now the real question, which is a bit out of scope for this patch is
why we even have XLOG_STATE_IOERROR? Wouldn't it make more sense
to just user the shutdown flag in the mount structure and avoid the
extra state complexity and thus clean up this whole mess?
