On Mon, Mar 16, 2020 at 03:42:28PM +0100, Christoph Hellwig wrote:
> Move handling of a shut down log out of xlog_state_iodone_process_iclog
> and into xlog_state_do_callback so that it can be moved into an entirely
> separate branch. While doing so switch to using XLOG_FORCED_SHUTDOWN to
> check the shutdown condition global to the log instead of the per-iclog
> flag, and make sure the comments match reality.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
Seems reasonable,
Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
--D
> ---
> fs/xfs/xfs_log.c | 64 ++++++++++++++++++++----------------------------
> 1 file changed, 26 insertions(+), 38 deletions(-)
>
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index c534d7007aa3..4efaa248a03d 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -2746,8 +2746,7 @@ xlog_state_do_iclog_callbacks(
> static bool
> xlog_state_iodone_process_iclog(
> struct xlog *log,
> - struct xlog_in_core *iclog,
> - bool *ioerror)
> + struct xlog_in_core *iclog)
> {
> xfs_lsn_t lowest_lsn;
> xfs_lsn_t header_lsn;
> @@ -2759,15 +2758,6 @@ xlog_state_iodone_process_iclog(
> * Skip all iclogs in the ACTIVE & DIRTY states:
> */
> return false;
> - case XLOG_STATE_IOERROR:
> - /*
> - * Between marking a filesystem SHUTDOWN and stopping the log,
> - * we do flush all iclogs to disk (if there wasn't a log I/O
> - * error). So, we do want things to go smoothly in case of just
> - * a SHUTDOWN w/o a LOG_IO_ERROR.
> - */
> - *ioerror = true;
> - return false;
> case XLOG_STATE_DONE_SYNC:
> /*
> * Now that we have an iclog that is in the DONE_SYNC state, do
> @@ -2795,39 +2785,41 @@ STATIC void
> xlog_state_do_callback(
> struct xlog *log)
> {
> - struct xlog_in_core *iclog;
> - struct xlog_in_core *first_iclog;
> bool cycled_icloglock;
> - bool ioerror;
> int flushcnt = 0;
> int repeats = 0;
>
> + /*
> + * Scan all iclogs starting with the one pointed to by the log. Reset
> + * this starting point each time the log is unlocked (during callbacks).
> + *
> + * Keep looping through iclogs until one full pass is made without
> + * running any callbacks.
> + *
> + * If the log has been shut down, still perform the callbacks once per
> + * iclog to abort all log items, but don't bother to restart the loop
> + * after dropping the log as no new callbacks can show up.
> + */
> spin_lock(&log->l_icloglock);
> do {
> - /*
> - * Scan all iclogs starting with the one pointed to by the
> - * log. Reset this starting point each time the log is
> - * unlocked (during callbacks).
> - *
> - * Keep looping through iclogs until one full pass is made
> - * without running any callbacks.
> - */
> - first_iclog = log->l_iclog;
> - iclog = log->l_iclog;
> + struct xlog_in_core *first_iclog = log->l_iclog;
> + struct xlog_in_core *iclog = first_iclog;
> +
> cycled_icloglock = false;
> - ioerror = false;
> repeats++;
>
> do {
> - if (xlog_state_iodone_process_iclog(log, iclog,
> - &ioerror))
> + if (XLOG_FORCED_SHUTDOWN(log)) {
> + xlog_state_do_iclog_callbacks(log, iclog);
> + wake_up_all(&iclog->ic_force_wait);
> + continue;
> + }
> +
> + if (xlog_state_iodone_process_iclog(log, iclog))
> break;
>
> - if (iclog->ic_state != XLOG_STATE_CALLBACK &&
> - iclog->ic_state != XLOG_STATE_IOERROR) {
> - iclog = iclog->ic_next;
> + if (iclog->ic_state != XLOG_STATE_CALLBACK)
> continue;
> - }
>
> /*
> * Running callbacks will drop the icloglock which means
> @@ -2835,12 +2827,8 @@ xlog_state_do_callback(
> */
> cycled_icloglock = true;
> xlog_state_do_iclog_callbacks(log, iclog);
> - if (XLOG_FORCED_SHUTDOWN(log))
> - wake_up_all(&iclog->ic_force_wait);
> - else
> - xlog_state_clean_iclog(log, iclog);
> - iclog = iclog->ic_next;
> - } while (first_iclog != iclog);
> + xlog_state_clean_iclog(log, iclog);
> + } while ((iclog = iclog->ic_next) != first_iclog);
>
> if (repeats > 5000) {
> flushcnt += repeats;
> @@ -2849,7 +2837,7 @@ xlog_state_do_callback(
> "%s: possible infinite loop (%d iterations)",
> __func__, flushcnt);
> }
> - } while (!ioerror && cycled_icloglock);
> + } while (cycled_icloglock);
>
> if (log->l_iclog->ic_state == XLOG_STATE_ACTIVE ||
> log->l_iclog->ic_state == XLOG_STATE_IOERROR)
> --
> 2.24.1
>
