On Thu, Feb 27, 2020 at 08:43:16AM -0500, Brian Foster wrote:
> As implemented by the previous patch, relogging can be enabled on
> any item via a relog enabled transaction (which holds a reference to
> an active relog ticket). Add a couple log item flags to track relog
> state of an arbitrary log item. The item holds a reference to the
> global relog ticket when relogging is enabled and releases the
> reference when relogging is disabled.
>
> Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
> ---
> fs/xfs/xfs_trace.h | 2 ++
> fs/xfs/xfs_trans.c | 36 ++++++++++++++++++++++++++++++++++++
> fs/xfs/xfs_trans.h | 6 +++++-
> fs/xfs/xfs_trans_priv.h | 2 ++
> 4 files changed, 45 insertions(+), 1 deletion(-)
>
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index a86be7f807ee..a066617ec54d 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -1063,6 +1063,8 @@ DEFINE_LOG_ITEM_EVENT(xfs_ail_push);
> DEFINE_LOG_ITEM_EVENT(xfs_ail_pinned);
> DEFINE_LOG_ITEM_EVENT(xfs_ail_locked);
> DEFINE_LOG_ITEM_EVENT(xfs_ail_flushing);
> +DEFINE_LOG_ITEM_EVENT(xfs_relog_item);
> +DEFINE_LOG_ITEM_EVENT(xfs_relog_item_cancel);
>
> DECLARE_EVENT_CLASS(xfs_ail_class,
> TP_PROTO(struct xfs_log_item *lip, xfs_lsn_t old_lsn, xfs_lsn_t new_lsn),
> diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> index 8ac05ed8deda..f7f2411ead4e 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -778,6 +778,41 @@ xfs_trans_del_item(
> list_del_init(&lip->li_trans);
> }
>
> +void
> +xfs_trans_relog_item(
> + struct xfs_log_item *lip)
> +{
> + if (!test_and_set_bit(XFS_LI_RELOG, &lip->li_flags)) {
> + xfs_trans_ail_relog_get(lip->li_mountp);
> + trace_xfs_relog_item(lip);
> + }
What if xfs_trans_ail_relog_get() fails to get a reference here
because there is no current ail relog ticket? Isn't the transaction
it was reserved in required to be checked here for XFS_TRANS_RELOG
being set?
> +}
> +
> +void
> +xfs_trans_relog_item_cancel(
> + struct xfs_log_item *lip,
> + bool drain) /* wait for relogging to cease */
> +{
> + struct xfs_mount *mp = lip->li_mountp;
> +
> + if (!test_and_clear_bit(XFS_LI_RELOG, &lip->li_flags))
> + return;
> + xfs_trans_ail_relog_put(lip->li_mountp);
> + trace_xfs_relog_item_cancel(lip);
> +
> + if (!drain)
> + return;
> +
> + /*
> + * Some operations might require relog activity to cease before they can
> + * proceed. For example, an operation must wait before including a
> + * non-lockable log item (i.e. intent) in another transaction.
> + */
> + while (wait_on_bit_timeout(&lip->li_flags, XFS_LI_RELOGGED,
> + TASK_UNINTERRUPTIBLE, HZ))
> + xfs_log_force(mp, XFS_LOG_SYNC);
> +}
What is a "cancel" operation? Is it something you do when cancelling
a transaction (i.e. on operation failure) or is is something the
final transaction does to remove the relog item from the AIL (i.e.
part of the normal successful finish to a long running transaction)?
> /* Detach and unlock all of the items in a transaction */
> static void
> xfs_trans_free_items(
> @@ -863,6 +898,7 @@ xfs_trans_committed_bulk(
>
> if (aborted)
> set_bit(XFS_LI_ABORTED, &lip->li_flags);
> + clear_and_wake_up_bit(XFS_LI_RELOGGED, &lip->li_flags);
I don't know what the XFS_LI_RELOGGED flag really means in this
patch because I don't know what sets it. Perhaps this would be
better moved into the patch that first sets the RELOGGED flag?
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
