Re: [PATCH 06/42] xfs: don't assert fail on transaction cancel with deferred ops

[Allison Henderson <allison.henderson@oracle.com>]

On Thu, 2023-01-19 at 09:44 +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> We can error out of an allocation transaction when updating BMBT
> blocks when things go wrong. This can be a btree corruption, and
> unexpected ENOSPC, etc. In these cases, we already have deferred ops
> queued for the first allocation that has been done, and we just want
> to cancel out the transaction and shut down the filesystem on error.
> 
> In fact, we do just that for production systems - the assert that we
> can't have a transaction with defer ops attached unless we are
> already shut down is bogus and gets in the way of debugging
> whatever issue is actually causing the transaction to be cancelled.
> 
> Remove the assert because it is causing spurious test failures to
> hang test machines.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
Ok, makes sense
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>

> ---
>  fs/xfs/xfs_trans.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> index 53ab544e4c2c..8afc0c080861 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -1078,10 +1078,10 @@ xfs_trans_cancel(
>         /*
>          * It's never valid to cancel a transaction with deferred ops
> attached,
>          * because the transaction is effectively dirty.  Complain
> about this
> -        * loudly before freeing the in-memory defer items.
> +        * loudly before freeing the in-memory defer items and
> shutting down the
> +        * filesystem.
>          */
>         if (!list_empty(&tp->t_dfops)) {
> -               ASSERT(xfs_is_shutdown(mp) || list_empty(&tp-
> >t_dfops));
>                 ASSERT(tp->t_flags & XFS_TRANS_PERM_LOG_RES);
>                 dirty = true;
>                 xfs_defer_cancel(tp);