[v1] xfs: fix CoW fork repair error handling

[PATCH 0/2] xfs: fix CoW fork repair error handling

Posted by Yingjie Gao 1 week, 6 days ago

Hi all,

This series fixes two error handling bugs in the CoW fork repair code.

The non-realtime helper can lose real failures in two different ways:
it returns success after the common cleanup labels, and the
force-rebuild path can return directly without freeing the AG repair
context.

The realtime helper has a related cleanup bug in its force-rebuild
path.  If marking the full mapping fails, the code jumps past the
rtgroup scrub cleanup and only drops the local rtgroup reference.

Split the fixes into two patches so that the non-RT and RT changes
remain easy to review and bisect independently.

Patches in this series:
  1. fix non-RT CoW fork repair error returns and cleanup
  2. fix RT CoW fork repair cleanup

Thanks,
Yingjie

Yingjie Gao (2):
  xfs: fix error returns in CoW fork repair
  xfs: fix rtgroup cleanup in CoW fork repair

 fs/xfs/scrub/cow_repair.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

-- 
2.20.1

[PATCH v2 0/2] xfs: fix CoW fork repair error handling

Posted by Yingjie Gao 1 week, 5 days ago

Fix two error handling paths in CoW fork repair.

Patch 1 makes xrep_cow_find_bad() return saved errors after cleanup.
It also lets a force-rebuild bitmap error fall through to the cleanup
labels instead of returning directly.

Patch 2 applies the same fall-through pattern to the realtime helper
so that scrub rtgroup state is released before returning.

Changes since v1:
  - Let the force-rebuild error paths fall through instead of adding
    gotos, as Carlos pointed out.

v1:
  https://lore.kernel.org/linux-xfs/20260526123243.1078867-1-gaoyingjie@uniontech.com/

Yingjie Gao (2):
  xfs: fix error returns in CoW fork repair
  xfs: fix rtgroup cleanup in CoW fork repair

 fs/xfs/scrub/cow_repair.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

-- 
2.20.1

[PATCH v3 0/2] xfs: fix CoW fork repair error handling

Posted by Yingjie Gao 1 week, 5 days ago

Fix two error handling paths in CoW fork repair.

Patch 1 makes xrep_cow_find_bad() return saved errors after cleanup.
It also lets a force-rebuild bitmap error fall through to the cleanup
labels instead of returning directly.

Patch 2 applies the same fall-through pattern to the realtime helper
so that scrub rtgroup state is released before returning.

Changes since v2:
  - Drop the braces around the single-statement force-rebuild if bodies
    in both patches.

v2:
  https://lore.kernel.org/linux-xfs/20260527033902.1524007-1-gaoyingjie@uniontech.com/

Changes since v1:
  - Let the force-rebuild error paths fall through instead of adding
    gotos, as Carlos pointed out.

v1:
  https://lore.kernel.org/linux-xfs/20260526123243.1078867-1-gaoyingjie@uniontech.com/

Yingjie Gao (2):
  xfs: fix error returns in CoW fork repair
  xfs: fix rtgroup cleanup in CoW fork repair

 fs/xfs/scrub/cow_repair.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

-- 
2.20.1

Re: [PATCH v3 0/2] xfs: fix CoW fork repair error handling

Posted by Carlos Maiolino 1 week, 2 days ago

On Wed, 27 May 2026 12:31:32 +0800, Yingjie Gao wrote:
> Fix two error handling paths in CoW fork repair.
> 
> Patch 1 makes xrep_cow_find_bad() return saved errors after cleanup.
> It also lets a force-rebuild bitmap error fall through to the cleanup
> labels instead of returning directly.
> 
> Patch 2 applies the same fall-through pattern to the realtime helper
> so that scrub rtgroup state is released before returning.
> 
> [...]

Applied to for-next, thanks!

[1/2] xfs: fix error returns in CoW fork repair
      commit: fcf4faba9f986b3bb528da11913c9ec5d6e8f689
[2/2] xfs: fix rtgroup cleanup in CoW fork repair
      commit: c3e073894379532c00cca7ba5762e18fafe29da8

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

[PATCH v3 1/2] xfs: fix error returns in CoW fork repair

Posted by Yingjie Gao 1 week, 5 days ago

xrep_cow_find_bad() returns success after the cleanup labels even if
AG setup, btree queries, or bitmap updates failed. This can make
repair continue with an incomplete bad-file-offset bitmap instead of
stopping at the original error.

The force-rebuild path has a related cleanup problem. If
xrep_cow_mark_file_range() fails, the function returns directly and
skips the scrub AG context and perag cleanup.

Let the force-rebuild path fall through to the existing cleanup code
and return the saved error after cleanup.

Fixes: dbbdbd008632 ("xfs: repair problems in CoW forks")
Signed-off-by: Yingjie Gao <gaoyingjie@uniontech.com>
---
 fs/xfs/scrub/cow_repair.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/scrub/cow_repair.c b/fs/xfs/scrub/cow_repair.c
index bffc4666ce60..a6ff09ace43d 100644
--- a/fs/xfs/scrub/cow_repair.c
+++ b/fs/xfs/scrub/cow_repair.c
@@ -300,18 +300,15 @@ xrep_cow_find_bad(
 	 * on the debugging knob, replace everything in the CoW fork.
 	 */
 	if ((sc->sm->sm_flags & XFS_SCRUB_IFLAG_FORCE_REBUILD) ||
-	    XFS_TEST_ERROR(sc->mp, XFS_ERRTAG_FORCE_SCRUB_REPAIR)) {
+	    XFS_TEST_ERROR(sc->mp, XFS_ERRTAG_FORCE_SCRUB_REPAIR))
 		error = xrep_cow_mark_file_range(xc, xc->irec.br_startblock,
 				xc->irec.br_blockcount);
-		if (error)
-			return error;
-	}
 
 out_sa:
 	xchk_ag_free(sc, &sc->sa);
 out_pag:
 	xfs_perag_put(pag);
-	return 0;
+	return error;
 }
 
 /*
-- 
2.20.1

Re: [PATCH v3 1/2] xfs: fix error returns in CoW fork repair

Posted by Darrick J. Wong 1 week, 4 days ago

On Wed, May 27, 2026 at 12:31:33PM +0800, Yingjie Gao wrote:
> xrep_cow_find_bad() returns success after the cleanup labels even if
> AG setup, btree queries, or bitmap updates failed. This can make
> repair continue with an incomplete bad-file-offset bitmap instead of
> stopping at the original error.
> 
> The force-rebuild path has a related cleanup problem. If
> xrep_cow_mark_file_range() fails, the function returns directly and
> skips the scrub AG context and perag cleanup.
> 
> Let the force-rebuild path fall through to the existing cleanup code
> and return the saved error after cleanup.
> 

Cc: <stable@vger.kernel.org> # v6.8

(please make life easier for Theodorics A and B who are half-assing LTS
QA these days)

> Fixes: dbbdbd008632 ("xfs: repair problems in CoW forks")
> Signed-off-by: Yingjie Gao <gaoyingjie@uniontech.com>
> ---
>  fs/xfs/scrub/cow_repair.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/xfs/scrub/cow_repair.c b/fs/xfs/scrub/cow_repair.c
> index bffc4666ce60..a6ff09ace43d 100644
> --- a/fs/xfs/scrub/cow_repair.c
> +++ b/fs/xfs/scrub/cow_repair.c
> @@ -300,18 +300,15 @@ xrep_cow_find_bad(
>  	 * on the debugging knob, replace everything in the CoW fork.
>  	 */
>  	if ((sc->sm->sm_flags & XFS_SCRUB_IFLAG_FORCE_REBUILD) ||
> -	    XFS_TEST_ERROR(sc->mp, XFS_ERRTAG_FORCE_SCRUB_REPAIR)) {
> +	    XFS_TEST_ERROR(sc->mp, XFS_ERRTAG_FORCE_SCRUB_REPAIR))
>  		error = xrep_cow_mark_file_range(xc, xc->irec.br_startblock,
>  				xc->irec.br_blockcount);
> -		if (error)
> -			return error;

I personally would have stuck with the v1 logic of turning that into a
"goto out_sa" but either way is correct.

With the cc tag added,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D


> -	}
>  
>  out_sa:
>  	xchk_ag_free(sc, &sc->sa);
>  out_pag:
>  	xfs_perag_put(pag);
> -	return 0;
> +	return error;
>  }
>  
>  /*
> -- 
> 2.20.1
> 
>

Re: [PATCH v3 1/2] xfs: fix error returns in CoW fork repair

Posted by Carlos Maiolino 1 week, 4 days ago

On Wed, May 27, 2026 at 09:44:42PM -0700, Darrick J. Wong wrote:
> On Wed, May 27, 2026 at 12:31:33PM +0800, Yingjie Gao wrote:
> > xrep_cow_find_bad() returns success after the cleanup labels even if
> > AG setup, btree queries, or bitmap updates failed. This can make
> > repair continue with an incomplete bad-file-offset bitmap instead of
> > stopping at the original error.
> > 
> > The force-rebuild path has a related cleanup problem. If
> > xrep_cow_mark_file_range() fails, the function returns directly and
> > skips the scrub AG context and perag cleanup.
> > 
> > Let the force-rebuild path fall through to the existing cleanup code
> > and return the saved error after cleanup.
> > 
> 
> Cc: <stable@vger.kernel.org> # v6.8
> 
> (please make life easier for Theodorics A and B who are half-assing LTS
> QA these days)

I can add this at commit time

> 
> > Fixes: dbbdbd008632 ("xfs: repair problems in CoW forks")
> > Signed-off-by: Yingjie Gao <gaoyingjie@uniontech.com>
> > ---
> >  fs/xfs/scrub/cow_repair.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> > 
> > diff --git a/fs/xfs/scrub/cow_repair.c b/fs/xfs/scrub/cow_repair.c
> > index bffc4666ce60..a6ff09ace43d 100644
> > --- a/fs/xfs/scrub/cow_repair.c
> > +++ b/fs/xfs/scrub/cow_repair.c
> > @@ -300,18 +300,15 @@ xrep_cow_find_bad(
> >  	 * on the debugging knob, replace everything in the CoW fork.
> >  	 */
> >  	if ((sc->sm->sm_flags & XFS_SCRUB_IFLAG_FORCE_REBUILD) ||
> > -	    XFS_TEST_ERROR(sc->mp, XFS_ERRTAG_FORCE_SCRUB_REPAIR)) {
> > +	    XFS_TEST_ERROR(sc->mp, XFS_ERRTAG_FORCE_SCRUB_REPAIR))
> >  		error = xrep_cow_mark_file_range(xc, xc->irec.br_startblock,
> >  				xc->irec.br_blockcount);
> > -		if (error)
> > -			return error;
> 
> I personally would have stuck with the v1 logic of turning that into a
> "goto out_sa" but either way is correct.
> 
> With the cc tag added,
> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
> 
> --D
> 
> 
> > -	}
> >  
> >  out_sa:
> >  	xchk_ag_free(sc, &sc->sa);
> >  out_pag:
> >  	xfs_perag_put(pag);
> > -	return 0;
> > +	return error;
> >  }
> >  
> >  /*
> > -- 
> > 2.20.1
> > 
> > 
>

[PATCH v3 2/2] xfs: fix rtgroup cleanup in CoW fork repair

Posted by Yingjie Gao 1 week, 5 days ago

xrep_cow_find_bad_rt() initializes scrub rtgroup state before the
force-rebuild path calls xrep_cow_mark_file_range(). If that call
fails, the code jumps directly to out_rtg, which skips the scrub
rtgroup cleanup and only drops the local rtgroup reference.

Remove the unnecessary jump so the function falls through to out_sr,
ensuring the realtime cursors, lock state, and sr->rtg reference are
released before returning.

Fixes: fd97fe111208 ("xfs: fix CoW forks for realtime files")
Signed-off-by: Yingjie Gao <gaoyingjie@uniontech.com>
---
 fs/xfs/scrub/cow_repair.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/fs/xfs/scrub/cow_repair.c b/fs/xfs/scrub/cow_repair.c
index a6ff09ace43d..c25716fc4fee 100644
--- a/fs/xfs/scrub/cow_repair.c
+++ b/fs/xfs/scrub/cow_repair.c
@@ -382,12 +382,9 @@ xrep_cow_find_bad_rt(
 	 * CoW fork and then scan for staging extents in the refcountbt.
 	 */
 	if ((sc->sm->sm_flags & XFS_SCRUB_IFLAG_FORCE_REBUILD) ||
-	    XFS_TEST_ERROR(sc->mp, XFS_ERRTAG_FORCE_SCRUB_REPAIR)) {
+	    XFS_TEST_ERROR(sc->mp, XFS_ERRTAG_FORCE_SCRUB_REPAIR))
 		error = xrep_cow_mark_file_range(xc, xc->irec.br_startblock,
 				xc->irec.br_blockcount);
-		if (error)
-			goto out_rtg;
-	}
 
 out_sr:
 	xchk_rtgroup_btcur_free(&sc->sr);
-- 
2.20.1

Re: [PATCH v3 2/2] xfs: fix rtgroup cleanup in CoW fork repair

Posted by Darrick J. Wong 1 week, 4 days ago

On Wed, May 27, 2026 at 12:31:34PM +0800, Yingjie Gao wrote:
> xrep_cow_find_bad_rt() initializes scrub rtgroup state before the
> force-rebuild path calls xrep_cow_mark_file_range(). If that call
> fails, the code jumps directly to out_rtg, which skips the scrub
> rtgroup cleanup and only drops the local rtgroup reference.
> 
> Remove the unnecessary jump so the function falls through to out_sr,
> ensuring the realtime cursors, lock state, and sr->rtg reference are
> released before returning.

Cc: <stable@vger.kernel.org> # v6.14

> Fixes: fd97fe111208 ("xfs: fix CoW forks for realtime files")
> Signed-off-by: Yingjie Gao <gaoyingjie@uniontech.com>

With the cc tag, this looks good to me
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/scrub/cow_repair.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/scrub/cow_repair.c b/fs/xfs/scrub/cow_repair.c
> index a6ff09ace43d..c25716fc4fee 100644
> --- a/fs/xfs/scrub/cow_repair.c
> +++ b/fs/xfs/scrub/cow_repair.c
> @@ -382,12 +382,9 @@ xrep_cow_find_bad_rt(
>  	 * CoW fork and then scan for staging extents in the refcountbt.
>  	 */
>  	if ((sc->sm->sm_flags & XFS_SCRUB_IFLAG_FORCE_REBUILD) ||
> -	    XFS_TEST_ERROR(sc->mp, XFS_ERRTAG_FORCE_SCRUB_REPAIR)) {
> +	    XFS_TEST_ERROR(sc->mp, XFS_ERRTAG_FORCE_SCRUB_REPAIR))
>  		error = xrep_cow_mark_file_range(xc, xc->irec.br_startblock,
>  				xc->irec.br_blockcount);
> -		if (error)
> -			goto out_rtg;
> -	}
>  
>  out_sr:
>  	xchk_rtgroup_btcur_free(&sc->sr);
> -- 
> 2.20.1
> 
>

[PATCH v2 1/2] xfs: fix error returns in CoW fork repair

Posted by Yingjie Gao 1 week, 5 days ago

xrep_cow_find_bad() returns success after the cleanup labels even if
AG setup, btree queries, or bitmap updates failed. This can make
repair continue with an incomplete bad-file-offset bitmap instead of
stopping at the original error.

The force-rebuild path has a related cleanup problem. If
xrep_cow_mark_file_range() fails, the function returns directly and
skips the scrub AG context and perag cleanup.

Let the force-rebuild path fall through to the existing cleanup code
and return the saved error after cleanup.

Fixes: dbbdbd008632 ("xfs: repair problems in CoW forks")
Signed-off-by: Yingjie Gao <gaoyingjie@uniontech.com>
---
 fs/xfs/scrub/cow_repair.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/xfs/scrub/cow_repair.c b/fs/xfs/scrub/cow_repair.c
index bffc4666ce60..d23238830216 100644
--- a/fs/xfs/scrub/cow_repair.c
+++ b/fs/xfs/scrub/cow_repair.c
@@ -303,15 +303,13 @@ xrep_cow_find_bad(
 	    XFS_TEST_ERROR(sc->mp, XFS_ERRTAG_FORCE_SCRUB_REPAIR)) {
 		error = xrep_cow_mark_file_range(xc, xc->irec.br_startblock,
 				xc->irec.br_blockcount);
-		if (error)
-			return error;
 	}
 
 out_sa:
 	xchk_ag_free(sc, &sc->sa);
 out_pag:
 	xfs_perag_put(pag);
-	return 0;
+	return error;
 }
 
 /*
-- 
2.20.1

[PATCH v2 2/2] xfs: fix rtgroup cleanup in CoW fork repair

Posted by Yingjie Gao 1 week, 5 days ago

xrep_cow_find_bad_rt() initializes scrub rtgroup state before the
force-rebuild path calls xrep_cow_mark_file_range(). If that call
fails, the code jumps directly to out_rtg, which skips the scrub
rtgroup cleanup and only drops the local rtgroup reference.

Remove the unnecessary jump so the function falls through to out_sr,
ensuring the realtime cursors, lock state, and sr->rtg reference are
released before returning.

Fixes: fd97fe111208 ("xfs: fix CoW forks for realtime files")
Signed-off-by: Yingjie Gao <gaoyingjie@uniontech.com>
---
 fs/xfs/scrub/cow_repair.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/xfs/scrub/cow_repair.c b/fs/xfs/scrub/cow_repair.c
index d23238830216..e4ca24623135 100644
--- a/fs/xfs/scrub/cow_repair.c
+++ b/fs/xfs/scrub/cow_repair.c
@@ -386,8 +386,6 @@ xrep_cow_find_bad_rt(
 	    XFS_TEST_ERROR(sc->mp, XFS_ERRTAG_FORCE_SCRUB_REPAIR)) {
 		error = xrep_cow_mark_file_range(xc, xc->irec.br_startblock,
 				xc->irec.br_blockcount);
-		if (error)
-			goto out_rtg;
 	}
 
 out_sr:
-- 
2.20.1

Re: [PATCH 0/2] xfs: fix CoW fork repair error handling

Posted by Carlos Maiolino 1 week, 6 days ago

On Tue, May 26, 2026 at 08:32:41PM +0800, Yingjie Gao wrote:
> Hi all,
> 
> This series fixes two error handling bugs in the CoW fork repair code.
> 
> The non-realtime helper can lose real failures in two different ways:
> it returns success after the common cleanup labels, and the
> force-rebuild path can return directly without freeing the AG repair
> context.
> 
> The realtime helper has a related cleanup bug in its force-rebuild
> path.  If marking the full mapping fails, the code jumps past the
> rtgroup scrub cleanup and only drops the local rtgroup reference.
> 
> Split the fixes into two patches so that the non-RT and RT changes
> remain easy to review and bisect independently.
> 
> Patches in this series:
>   1. fix non-RT CoW fork repair error returns and cleanup
>   2. fix RT CoW fork repair cleanup

Was this generated by any LLM model?

> 
> Thanks,
> Yingjie
> 
> Yingjie Gao (2):
>   xfs: fix error returns in CoW fork repair
>   xfs: fix rtgroup cleanup in CoW fork repair
> 
>  fs/xfs/scrub/cow_repair.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> -- 
> 2.20.1
>

Re: [PATCH 0/2] xfs: fix CoW fork repair error handling

Posted by Yingjie Gao 1 week, 5 days ago

Hi Carlos,

Yes, I used LLM assistance for the cover letter wording.

I've prepared a v2 with the cleanup changes you pointed out.

Thanks,
Yingjie