Re: [RFC v5 PATCH 9/9] xfs: relog random buffers based on errortag

Allison Collins
2020-02-27T23:49:06Z

On 2/27/20 6:43 AM, Brian Foster wrote:
> Since there is currently no specific use case for buffer relogging,
> add some hacky and experimental code to relog random buffers when
> the associated errortag is enabled. Update the relog reservation
> calculation appropriately and use fixed termination logic to help
> ensure that the relog queue doesn't grow indefinitely.
>
> Note that this patch was useful in causing log reservation deadlocks
> on an fsstress workload if the relog mechanism code is modified to
> acquire its own log reservation rather than rely on the relog
> pre-reservation mechanism. In other words, this helps prove that the
> relog reservation management code effectively avoids log reservation
> deadlocks.
>

Oh i see, so the last three are sort of an internal test case. They
look like they are good sand box tools for testing though. I guess they
dont really get RVBs since they dont apply? Otherwise looks good for
the purpose they are meant for. :-)

Allison

> Signed-off-by: Brian Foster <bfoster@redhat.com>
> ---
> fs/xfs/libxfs/xfs_trans_resv.c | 8 +++++++-
> fs/xfs/xfs_trans.h | 4 +++-
> fs/xfs/xfs_trans_ail.c | 11 +++++++++++
> fs/xfs/xfs_trans_buf.c | 13 +++++++++++++
> 4 files changed, 34 insertions(+), 2 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_trans_resv.c b/fs/xfs/libxfs/xfs_trans_resv.c
> index f49b20c9ca33..59a328a0dec6 100644
> --- a/fs/xfs/libxfs/xfs_trans_resv.c
> +++ b/fs/xfs/libxfs/xfs_trans_resv.c
> @@ -840,7 +840,13 @@ STATIC uint
> xfs_calc_relog_reservation(
> struct xfs_mount *mp)
> {
> - return xfs_calc_qm_quotaoff_reservation(mp);
> + uint res;
> +
> + res = xfs_calc_qm_quotaoff_reservation(mp);
> +#ifdef DEBUG
> + res = max(res, xfs_calc_buf_res(4, XFS_FSB_TO_B(mp, 1)));
> +#endif
> + return res;
> }
>
> void
> diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
> index 81cb42f552d9..1783441f6d03 100644
> --- a/fs/xfs/xfs_trans.h
> +++ b/fs/xfs/xfs_trans.h
> @@ -61,6 +61,7 @@ struct xfs_log_item {
> #define XFS_LI_DIRTY 3 /* log item dirty in transaction */
> #define XFS_LI_RELOG 4 /* automatically relog item */
> #define XFS_LI_RELOGGED 5 /* item relogged (not committed) */
> +#define XFS_LI_RELOG_RAND 6
>
> #define XFS_LI_FLAGS \
> { (1 << XFS_LI_IN_AIL), "IN_AIL" }, \
> @@ -68,7 +69,8 @@ struct xfs_log_item {
> { (1 << XFS_LI_FAILED), "FAILED" }, \
> { (1 << XFS_LI_DIRTY), "DIRTY" }, \
> { (1 << XFS_LI_RELOG), "RELOG" }, \
> - { (1 << XFS_LI_RELOGGED), "RELOGGED" }
> + { (1 << XFS_LI_RELOGGED), "RELOGGED" }, \
> + { (1 << XFS_LI_RELOG_RAND), "RELOG_RAND" }
>
> struct xfs_item_ops {
> unsigned flags;
> diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
> index 103ab62e61be..9b1d7c8df6d8 100644
> --- a/fs/xfs/xfs_trans_ail.c
> +++ b/fs/xfs/xfs_trans_ail.c
> @@ -188,6 +188,17 @@ xfs_ail_relog(
> xfs_log_ticket_put(ailp->ail_relog_tic);
> spin_unlock(&ailp->ail_lock);
>
> + /*
> + * Terminate random/debug relogs at a fixed, aggressive rate to
> + * avoid building up too much relog activity.
> + */
> + if (test_bit(XFS_LI_RELOG_RAND, &lip->li_flags) &&
> + ((prandom_u32() & 1) ||
> + (mp->m_flags & XFS_MOUNT_UNMOUNTING))) {
> + clear_bit(XFS_LI_RELOG_RAND, &lip->li_flags);
> + xfs_trans_relog_item_cancel(lip, false);
> + }
> +
> /*
> * TODO: Ideally, relog transaction management would be pushed
> * down into the ->iop_push() callbacks rather than playing
> diff --git a/fs/xfs/xfs_trans_buf.c b/fs/xfs/xfs_trans_buf.c
> index e17715ac23fc..de7b9a68fe38 100644
> --- a/fs/xfs/xfs_trans_buf.c
> +++ b/fs/xfs/xfs_trans_buf.c
> @@ -14,6 +14,8 @@
> #include "xfs_buf_item.h"
> #include "xfs_trans_priv.h"
> #include "xfs_trace.h"
> +#include "xfs_error.h"
> +#include "xfs_errortag.h"
>
> /*
> * Check to see if a buffer matching the given parameters is already
> @@ -527,6 +529,17 @@ xfs_trans_log_buf(
>
> trace_xfs_trans_log_buf(bip);
> xfs_buf_item_log(bip, first, last);
> +
> + /*
> + * Relog random buffers so long as the transaction is relog enabled and
> + * the buffer wasn't already relogged explicitly.
> + */
> + if (XFS_TEST_ERROR(false, tp->t_mountp, XFS_ERRTAG_RELOG) &&
> + (tp->t_flags & XFS_TRANS_RELOG) &&
> + !test_bit(XFS_LI_RELOG, &bip->bli_item.li_flags)) {
> + if (xfs_trans_relog_buf(tp, bp))
> + set_bit(XFS_LI_RELOG_RAND, &bip->bli_item.li_flags);
> + }
> }
>
>
>