From: Brian Foster <bfoster@redhat.com>
To: Christoph Hellwig <hch@lst.de>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 08/11] xfs: refactor xfs_defer_finish_noroll
Date: Thu, 30 Apr 2020 07:04:24 -0400 [thread overview]
Message-ID: <20200430110424.GH5349@bfoster> (raw)
In-Reply-To: <20200429150511.2191150-9-hch@lst.de>
On Wed, Apr 29, 2020 at 05:05:08PM +0200, Christoph Hellwig wrote:
> Split out a helper that operates on a single xfs_defer_pending structure
> to untangle the code.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
Reviewed-by: Brian Foster <bfoster@redhat.com>
> fs/xfs/libxfs/xfs_defer.c | 128 ++++++++++++++++++--------------------
> 1 file changed, 59 insertions(+), 69 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
> index 5402a7bf31108..20950b56cdd07 100644
> --- a/fs/xfs/libxfs/xfs_defer.c
> +++ b/fs/xfs/libxfs/xfs_defer.c
> @@ -351,6 +351,53 @@ xfs_defer_cancel_list(
> }
> }
>
> +/*
> + * Log an intent-done item for the first pending intent, and finish the work
> + * items.
> + */
> +static int
> +xfs_defer_finish_one(
> + struct xfs_trans *tp,
> + struct xfs_defer_pending *dfp)
> +{
> + const struct xfs_defer_op_type *ops = defer_op_types[dfp->dfp_type];
> + void *state = NULL;
> + struct list_head *li, *n;
> + int error;
> +
> + trace_xfs_defer_pending_finish(tp->t_mountp, dfp);
> +
> + dfp->dfp_done = ops->create_done(tp, dfp->dfp_intent, dfp->dfp_count);
> + list_for_each_safe(li, n, &dfp->dfp_work) {
> + list_del(li);
> + dfp->dfp_count--;
> + error = ops->finish_item(tp, li, dfp->dfp_done, &state);
> + if (error == -EAGAIN) {
> + /*
> + * Caller wants a fresh transaction; put the work item
> + * back on the list and log a new log intent item to
> + * replace the old one. See "Requesting a Fresh
> + * Transaction while Finishing Deferred Work" above.
> + */
> + list_add(li, &dfp->dfp_work);
> + dfp->dfp_count++;
> + dfp->dfp_done = NULL;
> + xfs_defer_create_intent(tp, dfp, false);
> + }
> +
> + if (error)
> + goto out;
> + }
> +
> + /* Done with the dfp, free it. */
> + list_del(&dfp->dfp_list);
> + kmem_free(dfp);
> +out:
> + if (ops->finish_cleanup)
> + ops->finish_cleanup(tp, state, error);
> + return error;
> +}
> +
> /*
> * Finish all the pending work. This involves logging intent items for
> * any work items that wandered in since the last transaction roll (if
> @@ -364,11 +411,7 @@ xfs_defer_finish_noroll(
> struct xfs_trans **tp)
> {
> struct xfs_defer_pending *dfp;
> - struct list_head *li;
> - struct list_head *n;
> - void *state;
> int error = 0;
> - const struct xfs_defer_op_type *ops;
> LIST_HEAD(dop_pending);
>
> ASSERT((*tp)->t_flags & XFS_TRANS_PERM_LOG_RES);
> @@ -377,83 +420,30 @@ xfs_defer_finish_noroll(
>
> /* Until we run out of pending work to finish... */
> while (!list_empty(&dop_pending) || !list_empty(&(*tp)->t_dfops)) {
> - /* log intents and pull in intake items */
> xfs_defer_create_intents(*tp);
> list_splice_tail_init(&(*tp)->t_dfops, &dop_pending);
>
> - /*
> - * Roll the transaction.
> - */
> error = xfs_defer_trans_roll(tp);
> if (error)
> - goto out;
> + goto out_shutdown;
>
> - /* Log an intent-done item for the first pending item. */
> dfp = list_first_entry(&dop_pending, struct xfs_defer_pending,
> dfp_list);
> - ops = defer_op_types[dfp->dfp_type];
> - trace_xfs_defer_pending_finish((*tp)->t_mountp, dfp);
> - dfp->dfp_done = ops->create_done(*tp, dfp->dfp_intent,
> - dfp->dfp_count);
> -
> - /* Finish the work items. */
> - state = NULL;
> - list_for_each_safe(li, n, &dfp->dfp_work) {
> - list_del(li);
> - dfp->dfp_count--;
> - error = ops->finish_item(*tp, li, dfp->dfp_done,
> - &state);
> - if (error == -EAGAIN) {
> - /*
> - * Caller wants a fresh transaction;
> - * put the work item back on the list
> - * and jump out.
> - */
> - list_add(li, &dfp->dfp_work);
> - dfp->dfp_count++;
> - break;
> - } else if (error) {
> - /*
> - * Clean up after ourselves and jump out.
> - * xfs_defer_cancel will take care of freeing
> - * all these lists and stuff.
> - */
> - if (ops->finish_cleanup)
> - ops->finish_cleanup(*tp, state, error);
> - goto out;
> - }
> - }
> - if (error == -EAGAIN) {
> - /*
> - * Caller wants a fresh transaction, so log a new log
> - * intent item to replace the old one and roll the
> - * transaction. See "Requesting a Fresh Transaction
> - * while Finishing Deferred Work" above.
> - */
> - dfp->dfp_done = NULL;
> - xfs_defer_create_intent(*tp, dfp, false);
> - } else {
> - /* Done with the dfp, free it. */
> - list_del(&dfp->dfp_list);
> - kmem_free(dfp);
> - }
> -
> - if (ops->finish_cleanup)
> - ops->finish_cleanup(*tp, state, error);
> - }
> -
> -out:
> - if (error) {
> - xfs_defer_trans_abort(*tp, &dop_pending);
> - xfs_force_shutdown((*tp)->t_mountp, SHUTDOWN_CORRUPT_INCORE);
> - trace_xfs_defer_finish_error(*tp, error);
> - xfs_defer_cancel_list((*tp)->t_mountp, &dop_pending);
> - xfs_defer_cancel(*tp);
> - return error;
> + error = xfs_defer_finish_one(*tp, dfp);
> + if (error && error != -EAGAIN)
> + goto out_shutdown;
> }
>
> trace_xfs_defer_finish_done(*tp, _RET_IP_);
> return 0;
> +
> +out_shutdown:
> + xfs_defer_trans_abort(*tp, &dop_pending);
> + xfs_force_shutdown((*tp)->t_mountp, SHUTDOWN_CORRUPT_INCORE);
> + trace_xfs_defer_finish_error(*tp, error);
> + xfs_defer_cancel_list((*tp)->t_mountp, &dop_pending);
> + xfs_defer_cancel(*tp);
> + return error;
> }
>
> int
> --
> 2.26.2
>
next prev parent reply other threads:[~2020-04-30 11:04 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-29 15:05 deferred operations cleanup Christoph Hellwig
2020-04-29 15:05 ` [PATCH 01/11] xfs: remove the xfs_efi_log_item_t typedef Christoph Hellwig
2020-04-30 11:02 ` Brian Foster
2020-04-29 15:05 ` [PATCH 02/11] xfs: remove the xfs_efd_log_item_t typedef Christoph Hellwig
2020-04-30 11:03 ` Brian Foster
2020-04-29 15:05 ` [PATCH 03/11] xfs: remove the xfs_inode_log_item_t typedef Christoph Hellwig
2020-04-30 11:03 ` Brian Foster
2020-04-29 15:05 ` [PATCH 04/11] xfs: factor out a xfs_defer_create_intent helper Christoph Hellwig
2020-04-30 11:03 ` Brian Foster
2020-04-29 15:05 ` [PATCH 05/11] xfs: merge the ->log_item defer op into ->create_intent Christoph Hellwig
2020-04-30 11:04 ` Brian Foster
2020-04-29 15:05 ` [PATCH 06/11] xfs: merge the ->diff_items " Christoph Hellwig
2020-04-30 11:04 ` Brian Foster
2020-04-29 15:05 ` [PATCH 07/11] xfs: turn dfp_intent into a xfs_log_item Christoph Hellwig
2020-04-30 11:04 ` Brian Foster
2020-04-29 15:05 ` [PATCH 08/11] xfs: refactor xfs_defer_finish_noroll Christoph Hellwig
2020-04-30 11:04 ` Brian Foster [this message]
2020-04-29 15:05 ` [PATCH 09/11] xfs: turn dfp_done into a xfs_log_item Christoph Hellwig
2020-04-30 11:04 ` Brian Foster
2020-04-29 15:05 ` [PATCH 10/11] xfs: use a xfs_btree_cur for the ->finish_cleanup state Christoph Hellwig
2020-04-30 11:04 ` Brian Foster
2020-04-29 15:05 ` [PATCH 11/11] xfs: spell out the parameter name for ->cancel_item Christoph Hellwig
2020-04-30 11:04 ` Brian Foster
2020-04-30 15:35 ` deferred operations cleanup Darrick J. Wong
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=20200430110424.GH5349@bfoster \
--to=bfoster@redhat.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;
as well as URLs for NNTP newsgroup(s).