On Wed, May 19, 2021 at 10:13:14PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> Adding a list_sort() call to the CIL push work while the xc_ctx_lock
> is held exclusively has resulted in fairly long lock hold times and
> that stops all front end transaction commits from making progress.
>
> We can move the sorting out of the xc_ctx_lock if we can transfer
> the ordering information to the log vectors as they are detached
> from the log items and then we can sort the log vectors. With these
> changes, we can move the list_sort() call to just before we call
> xlog_write() when we aren't holding any locks at all.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
Looks good to me,
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
> ---
> fs/xfs/xfs_log.h | 1 +
> fs/xfs/xfs_log_cil.c | 23 ++++++++++++++---------
> 2 files changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/fs/xfs/xfs_log.h b/fs/xfs/xfs_log.h
> index b4ad0e37a0c5..93aaee7c276e 100644
> --- a/fs/xfs/xfs_log.h
> +++ b/fs/xfs/xfs_log.h
> @@ -10,6 +10,7 @@ struct xfs_cil_ctx;
>
> struct xfs_log_vec {
> struct list_head lv_list; /* CIL lv chain ptrs */
> + uint32_t lv_order_id; /* chain ordering info */
> int lv_niovecs; /* number of iovecs in lv */
> struct xfs_log_iovec *lv_iovecp; /* iovec array */
> struct xfs_log_item *lv_item; /* owner */
> diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
> index 035f0a60040a..cfd3128399f6 100644
> --- a/fs/xfs/xfs_log_cil.c
> +++ b/fs/xfs/xfs_log_cil.c
> @@ -791,10 +791,10 @@ xlog_cil_order_cmp(
> const struct list_head *a,
> const struct list_head *b)
> {
> - struct xfs_log_item *l1 = container_of(a, struct xfs_log_item, li_cil);
> - struct xfs_log_item *l2 = container_of(b, struct xfs_log_item, li_cil);
> + struct xfs_log_vec *l1 = container_of(a, struct xfs_log_vec, lv_list);
> + struct xfs_log_vec *l2 = container_of(b, struct xfs_log_vec, lv_list);
>
> - return l1->li_order_id > l2->li_order_id;
> + return l1->lv_order_id > l2->lv_order_id;
> }
>
> /*
> @@ -911,24 +911,22 @@ xlog_cil_push_work(
>
> xlog_cil_pcp_aggregate(cil, ctx);
>
> - list_sort(NULL, &ctx->log_items, xlog_cil_order_cmp);
> while (!list_empty(&ctx->log_items)) {
> struct xfs_log_item *item;
>
> item = list_first_entry(&ctx->log_items,
> struct xfs_log_item, li_cil);
> lv = item->li_lv;
> - list_del_init(&item->li_cil);
> - item->li_order_id = 0;
> - item->li_lv = NULL;
> -
> + lv->lv_order_id = item->li_order_id;
> num_iovecs += lv->lv_niovecs;
> /* we don't write ordered log vectors */
> if (lv->lv_buf_len != XFS_LOG_VEC_ORDERED)
> num_bytes += lv->lv_bytes;
>
> list_add_tail(&lv->lv_list, &ctx->lv_chain);
> -
> + list_del_init(&item->li_cil);
> + item->li_order_id = 0;
> + item->li_lv = NULL;
> }
>
> /*
> @@ -961,6 +959,13 @@ xlog_cil_push_work(
> spin_unlock(&cil->xc_push_lock);
> up_write(&cil->xc_ctx_lock);
>
> + /*
> + * Sort the log vector chain before we add the transaction headers.
> + * This ensures we always have the transaction headers at the start
> + * of the chain.
> + */
> + list_sort(NULL, &ctx->lv_chain, xlog_cil_order_cmp);
> +
> /*
> * Build a checkpoint transaction header and write it to the log to
> * begin the transaction. We need to account for the space used by the
> --
> 2.31.1
>
