From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Christoph Hellwig <hch@lst.de>
Cc: linux-xfs@vger.kernel.org, Dave Chinner <david@fromorbit.com>,
Brian Foster <bfoster@redhat.com>
Subject: Re: [PATCH 5/8] xfs: remove the aborted parameter to xlog_state_done_syncing
Date: Mon, 23 Mar 2020 08:27:06 -0700 [thread overview]
Message-ID: <20200323152706.GB29339@magnolia> (raw)
In-Reply-To: <20200320065311.28134-6-hch@lst.de>
On Fri, Mar 20, 2020 at 07:53:08AM +0100, Christoph Hellwig wrote:
> We can just check for a shut down log all the way down in
> xlog_cil_committed instead of passing the parameter. This means a
> slight behavior change in that we now also abort log items if the
> shutdown came in halfway into the I/O completion processing, which
> actually is the right thing to do.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Brian Foster <bfoster@redhat.com>
Looks ok now that I've gotten myself straightened out :)
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> fs/xfs/xfs_log.c | 48 +++++++++++++++-----------------------------
> fs/xfs/xfs_log.h | 2 +-
> fs/xfs/xfs_log_cil.c | 12 +++++------
> 3 files changed, 23 insertions(+), 39 deletions(-)
>
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 7af9c292540b..4f9303524efb 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -47,8 +47,7 @@ xlog_dealloc_log(
>
> /* local state machine functions */
> STATIC void xlog_state_done_syncing(
> - struct xlog_in_core *iclog,
> - bool aborted);
> + struct xlog_in_core *iclog);
> STATIC int
> xlog_state_get_iclog_space(
> struct xlog *log,
> @@ -1254,7 +1253,6 @@ xlog_ioend_work(
> struct xlog_in_core *iclog =
> container_of(work, struct xlog_in_core, ic_end_io_work);
> struct xlog *log = iclog->ic_log;
> - bool aborted = false;
> int error;
>
> error = blk_status_to_errno(iclog->ic_bio.bi_status);
> @@ -1270,17 +1268,9 @@ xlog_ioend_work(
> if (XFS_TEST_ERROR(error, log->l_mp, XFS_ERRTAG_IODONE_IOERR)) {
> xfs_alert(log->l_mp, "log I/O error %d", error);
> xfs_force_shutdown(log->l_mp, SHUTDOWN_LOG_IO_ERROR);
> - /*
> - * This flag will be propagated to the trans-committed
> - * callback routines to let them know that the log-commit
> - * didn't succeed.
> - */
> - aborted = true;
> - } else if (iclog->ic_state == XLOG_STATE_IOERROR) {
> - aborted = true;
> }
>
> - xlog_state_done_syncing(iclog, aborted);
> + xlog_state_done_syncing(iclog);
> bio_uninit(&iclog->ic_bio);
>
> /*
> @@ -1759,7 +1749,7 @@ xlog_write_iclog(
> * the buffer manually, the code needs to be kept in sync
> * with the I/O completion path.
> */
> - xlog_state_done_syncing(iclog, true);
> + xlog_state_done_syncing(iclog);
> up(&iclog->ic_sema);
> return;
> }
> @@ -2783,8 +2773,7 @@ xlog_state_iodone_process_iclog(
> static void
> xlog_state_do_iclog_callbacks(
> struct xlog *log,
> - struct xlog_in_core *iclog,
> - bool aborted)
> + struct xlog_in_core *iclog)
> __releases(&log->l_icloglock)
> __acquires(&log->l_icloglock)
> {
> @@ -2796,7 +2785,7 @@ xlog_state_do_iclog_callbacks(
> list_splice_init(&iclog->ic_callbacks, &tmp);
>
> spin_unlock(&iclog->ic_callback_lock);
> - xlog_cil_process_committed(&tmp, aborted);
> + xlog_cil_process_committed(&tmp);
> spin_lock(&iclog->ic_callback_lock);
> }
>
> @@ -2811,8 +2800,7 @@ xlog_state_do_iclog_callbacks(
>
> STATIC void
> xlog_state_do_callback(
> - struct xlog *log,
> - bool aborted)
> + struct xlog *log)
> {
> struct xlog_in_core *iclog;
> struct xlog_in_core *first_iclog;
> @@ -2853,7 +2841,7 @@ xlog_state_do_callback(
> * we'll have to run at least one more complete loop.
> */
> cycled_icloglock = true;
> - xlog_state_do_iclog_callbacks(log, iclog, aborted);
> + xlog_state_do_iclog_callbacks(log, iclog);
>
> xlog_state_clean_iclog(log, iclog);
> iclog = iclog->ic_next;
> @@ -2891,25 +2879,22 @@ xlog_state_do_callback(
> */
> STATIC void
> xlog_state_done_syncing(
> - struct xlog_in_core *iclog,
> - bool aborted)
> + struct xlog_in_core *iclog)
> {
> struct xlog *log = iclog->ic_log;
>
> spin_lock(&log->l_icloglock);
> -
> ASSERT(atomic_read(&iclog->ic_refcnt) == 0);
>
> /*
> * If we got an error, either on the first buffer, or in the case of
> - * split log writes, on the second, we mark ALL iclogs STATE_IOERROR,
> - * and none should ever be attempted to be written to disk
> - * again.
> + * split log writes, on the second, we shut down the file system and
> + * no iclogs should ever be attempted to be written to disk again.
> */
> - if (iclog->ic_state == XLOG_STATE_SYNCING)
> + if (!XLOG_FORCED_SHUTDOWN(log)) {
> + ASSERT(iclog->ic_state == XLOG_STATE_SYNCING);
> iclog->ic_state = XLOG_STATE_DONE_SYNC;
> - else
> - ASSERT(iclog->ic_state == XLOG_STATE_IOERROR);
> + }
>
> /*
> * Someone could be sleeping prior to writing out the next
> @@ -2918,9 +2903,8 @@ xlog_state_done_syncing(
> */
> wake_up_all(&iclog->ic_write_wait);
> spin_unlock(&log->l_icloglock);
> - xlog_state_do_callback(log, aborted); /* also cleans log */
> -} /* xlog_state_done_syncing */
> -
> + xlog_state_do_callback(log); /* also cleans log */
> +}
>
> /*
> * If the head of the in-core log ring is not (ACTIVE or DIRTY), then we must
> @@ -3884,7 +3868,7 @@ xfs_log_force_umount(
> spin_lock(&log->l_cilp->xc_push_lock);
> wake_up_all(&log->l_cilp->xc_commit_wait);
> spin_unlock(&log->l_cilp->xc_push_lock);
> - xlog_state_do_callback(log, true);
> + xlog_state_do_callback(log);
>
> /* return non-zero if log IOERROR transition had already happened */
> return retval;
> diff --git a/fs/xfs/xfs_log.h b/fs/xfs/xfs_log.h
> index b38602216c5a..cc77cc36560a 100644
> --- a/fs/xfs/xfs_log.h
> +++ b/fs/xfs/xfs_log.h
> @@ -137,7 +137,7 @@ void xfs_log_ticket_put(struct xlog_ticket *ticket);
>
> void xfs_log_commit_cil(struct xfs_mount *mp, struct xfs_trans *tp,
> xfs_lsn_t *commit_lsn, bool regrant);
> -void xlog_cil_process_committed(struct list_head *list, bool aborted);
> +void xlog_cil_process_committed(struct list_head *list);
> bool xfs_log_item_in_current_chkpt(struct xfs_log_item *lip);
>
> void xfs_log_work_queue(struct xfs_mount *mp);
> diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
> index 278166811c80..64cc0bf2ab3b 100644
> --- a/fs/xfs/xfs_log_cil.c
> +++ b/fs/xfs/xfs_log_cil.c
> @@ -574,10 +574,10 @@ xlog_discard_busy_extents(
> */
> static void
> xlog_cil_committed(
> - struct xfs_cil_ctx *ctx,
> - bool abort)
> + struct xfs_cil_ctx *ctx)
> {
> struct xfs_mount *mp = ctx->cil->xc_log->l_mp;
> + bool abort = XLOG_FORCED_SHUTDOWN(ctx->cil->xc_log);
>
> /*
> * If the I/O failed, we're aborting the commit and already shutdown.
> @@ -613,15 +613,14 @@ xlog_cil_committed(
>
> void
> xlog_cil_process_committed(
> - struct list_head *list,
> - bool aborted)
> + struct list_head *list)
> {
> struct xfs_cil_ctx *ctx;
>
> while ((ctx = list_first_entry_or_null(list,
> struct xfs_cil_ctx, iclog_entry))) {
> list_del(&ctx->iclog_entry);
> - xlog_cil_committed(ctx, aborted);
> + xlog_cil_committed(ctx);
> }
> }
>
> @@ -878,7 +877,8 @@ xlog_cil_push_work(
> out_abort_free_ticket:
> xfs_log_ticket_put(tic);
> out_abort:
> - xlog_cil_committed(ctx, true);
> + ASSERT(XLOG_FORCED_SHUTDOWN(log));
> + xlog_cil_committed(ctx);
> }
>
> /*
> --
> 2.25.1
>
next prev parent reply other threads:[~2020-03-23 15:27 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-20 6:53 more log cleanups Christoph Hellwig
2020-03-20 6:53 ` [PATCH 1/8] xfs: merge xlog_cil_push into xlog_cil_push_work Christoph Hellwig
2020-03-20 6:53 ` [PATCH 2/8] xfs: factor out a xlog_wait_on_iclog helper Christoph Hellwig
2020-03-20 6:53 ` [PATCH 3/8] xfs: simplify the xfs_log_release_iclog calling convention Christoph Hellwig
2020-03-20 6:53 ` [PATCH 4/8] xfs: simplify log shutdown checking in xfs_log_release_iclog Christoph Hellwig
2020-03-20 6:53 ` [PATCH 5/8] xfs: remove the aborted parameter to xlog_state_done_syncing Christoph Hellwig
2020-03-23 15:27 ` Darrick J. Wong [this message]
2020-03-20 6:53 ` [PATCH 6/8] xfs: refactor xlog_state_clean_iclog Christoph Hellwig
2020-03-20 11:51 ` Brian Foster
2020-03-20 6:53 ` [PATCH 7/8] xfs: move the ioerror check out of xlog_state_clean_iclog Christoph Hellwig
2020-03-20 6:53 ` [PATCH 8/8] xfs: remove xlog_state_want_sync Christoph Hellwig
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200323152706.GB29339@magnolia \
--to=darrick.wong@oracle.com \
--cc=bfoster@redhat.com \
--cc=david@fromorbit.com \
--cc=hch@lst.de \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox