[PATCH] xfs: fix frozen file system assert in xfs_trans_alloc

[PATCH] xfs: fix frozen file system assert in xfs_trans_alloc

[Christoph Hellwig]

Commit 83a80e95e797 ("xfs: decouple xfs_trans_alloc_empty from
xfs_trans_alloc") move the place of the assert for a frozen file system
after the sb_start_intwrite call that ensures it doesn't run on frozen
file systems, and thus allows to incorrect trigger it.

Fix that by moving it back to where it belongs.

Fixes: 83a80e95e797 ("xfs: decouple xfs_trans_alloc_empty from xfs_trans_alloc")
Reported-by: Dave Chinner <david@fromorbit.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_trans.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index 4917b7d390a3..9010dd682591 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -253,8 +253,8 @@ xfs_trans_alloc(
 	 * by doing GFP_KERNEL allocations inside sb_start_intwrite().
 	 */
 retry:
-	WARN_ON(mp->m_super->s_writers.frozen == SB_FREEZE_COMPLETE);
 	tp = __xfs_trans_alloc(mp, flags);
+	WARN_ON(mp->m_super->s_writers.frozen == SB_FREEZE_COMPLETE);
 	error = xfs_trans_reserve(tp, resp, blocks, rtextents);
 	if (error == -ENOSPC && want_retry) {
 		xfs_trans_cancel(tp);
-- 
2.47.2

Re: [PATCH] xfs: fix frozen file system assert in xfs_trans_alloc

[Darrick J. Wong]

On Thu, Jul 31, 2025 at 07:19:41AM -0700, Christoph Hellwig wrote:
> Commit 83a80e95e797 ("xfs: decouple xfs_trans_alloc_empty from
> xfs_trans_alloc") move the place of the assert for a frozen file system
> after the sb_start_intwrite call that ensures it doesn't run on frozen
> file systems, and thus allows to incorrect trigger it.
> 
> Fix that by moving it back to where it belongs.
> 
> Fixes: 83a80e95e797 ("xfs: decouple xfs_trans_alloc_empty from xfs_trans_alloc")
> Reported-by: Dave Chinner <david@fromorbit.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_trans.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> index 4917b7d390a3..9010dd682591 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -253,8 +253,8 @@ xfs_trans_alloc(
>  	 * by doing GFP_KERNEL allocations inside sb_start_intwrite().
>  	 */
>  retry:
> -	WARN_ON(mp->m_super->s_writers.frozen == SB_FREEZE_COMPLETE);
>  	tp = __xfs_trans_alloc(mp, flags);
> +	WARN_ON(mp->m_super->s_writers.frozen == SB_FREEZE_COMPLETE);

Empty transactions can run during a freeze, so please put back the
original warning and comment:

	/*
	 * Zero-reservation ("empty") transactions can't modify anything, so
	 * they're allowed to run while we're frozen.
	 */
	WARN_ON(resp->tr_logres > 0 &&
		mp->m_super->s_writers.frozen == SB_FREEZE_COMPLETE);

--D

>  	error = xfs_trans_reserve(tp, resp, blocks, rtextents);
>  	if (error == -ENOSPC && want_retry) {
>  		xfs_trans_cancel(tp);
> -- 
> 2.47.2
> 
> 

Re: [PATCH] xfs: fix frozen file system assert in xfs_trans_alloc

[Christoph Hellwig]

On Thu, Jul 31, 2025 at 08:59:41AM -0700, Darrick J. Wong wrote:
> >  retry:
> > -	WARN_ON(mp->m_super->s_writers.frozen == SB_FREEZE_COMPLETE);
> >  	tp = __xfs_trans_alloc(mp, flags);
> > +	WARN_ON(mp->m_super->s_writers.frozen == SB_FREEZE_COMPLETE);
> 
> Empty transactions can run during a freeze, so please put back the
> original warning and comment:

Empty transactions don't end up in this code path any more.

Re: [PATCH] xfs: fix frozen file system assert in xfs_trans_alloc

[Darrick J. Wong]

On Thu, Jul 31, 2025 at 06:15:58PM +0200, Christoph Hellwig wrote:
> On Thu, Jul 31, 2025 at 08:59:41AM -0700, Darrick J. Wong wrote:
> > >  retry:
> > > -	WARN_ON(mp->m_super->s_writers.frozen == SB_FREEZE_COMPLETE);
> > >  	tp = __xfs_trans_alloc(mp, flags);
> > > +	WARN_ON(mp->m_super->s_writers.frozen == SB_FREEZE_COMPLETE);
> > 
> > Empty transactions can run during a freeze, so please put back the
> > original warning and comment:
> 
> Empty transactions don't end up in this code path any more.

Oh, hah, silly me, I forgot that also got changed by that patchset. :(
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

Re: [PATCH] xfs: fix frozen file system assert in xfs_trans_alloc

[Carlos Maiolino]

On Thu, 31 Jul 2025 07:19:41 -0700, Christoph Hellwig wrote:
> Commit 83a80e95e797 ("xfs: decouple xfs_trans_alloc_empty from
> xfs_trans_alloc") move the place of the assert for a frozen file system
> after the sb_start_intwrite call that ensures it doesn't run on frozen
> file systems, and thus allows to incorrect trigger it.
> 
> Fix that by moving it back to where it belongs.
> 
> [...]

Applied to for-next, thanks!

[1/1] xfs: fix frozen file system assert in xfs_trans_alloc
      commit: 647b3d59c768d7638dd17c78c8044178364383ca

Best regards,
-- 
Carlos Maiolino <cem@kernel.org>