diff mbox series

[20/29] xfs: don't fail repairs on metadata files with no attr fork

Message ID 172919069796.3451313.2227454340362290952.stgit@frogsfrogsfrogs (mailing list archive)
State Accepted, archived
Headers show
Series [01/29] xfs: constify the xfs_sb predicates | expand

Commit Message

Darrick J. Wong Oct. 17, 2024, 6:58 p.m. UTC 
From: Darrick J. Wong <djwong@kernel.org>

Fix a minor bug where we fail repairs on metadata files that do not have
attr forks because xrep_metadata_inode_subtype doesn't filter ENOENT.

Cc: <stable@vger.kernel.org> # v6.8
Fixes: 5a8e07e799721b ("xfs: repair the inode core and forks of a metadata inode")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/scrub/repair.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Greg KH Oct. 18, 2024, 6 a.m. UTC | #1 
On Thu, Oct 17, 2024 at 11:58:10AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Fix a minor bug where we fail repairs on metadata files that do not have
> attr forks because xrep_metadata_inode_subtype doesn't filter ENOENT.
> 
> Cc: <stable@vger.kernel.org> # v6.8
> Fixes: 5a8e07e799721b ("xfs: repair the inode core and forks of a metadata inode")
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/scrub/repair.c |    8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)

Why is a bugfix / stable-tagged-patch, number 20 in a 29 patch series?
Why isn't it first, or better yet, on it's own if it is fixing a bug
that people want merged "soon"?

thanks,

greg k-h
Darrick J. Wong Oct. 21, 2024, 5:27 p.m. UTC | #2 
On Fri, Oct 18, 2024 at 08:00:21AM +0200, Greg KH wrote:
> On Thu, Oct 17, 2024 at 11:58:10AM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Fix a minor bug where we fail repairs on metadata files that do not have
> > attr forks because xrep_metadata_inode_subtype doesn't filter ENOENT.
> > 
> > Cc: <stable@vger.kernel.org> # v6.8
> > Fixes: 5a8e07e799721b ("xfs: repair the inode core and forks of a metadata inode")
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > ---
> >  fs/xfs/scrub/repair.c |    8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> Why is a bugfix / stable-tagged-patch, number 20 in a 29 patch series?
> Why isn't it first, or better yet, on it's own if it is fixing a bug
> that people want merged "soon"?

I have too many patches, and every time I try to get a set through the
review process I end up having to write *more* patches to appease the
reviewers, and fixes get lost.  Look at the copyrights on the other
patches, I've been trying to get this upstreamed since 2018.

This particular bugfix got lost last month probably because I forgot to
ping cem to take it for 6.12-rc1.  Thanks for pushing on this, Greg.

Hey Carlos, can you queue this one up for 6.12-rc5, please?

--D

> thanks,
> 
> greg k-h
>
Carlos Maiolino Oct. 22, 2024, 11:16 a.m. UTC | #3 
On Mon, Oct 21, 2024 at 10:27:51AM GMT, Darrick J. Wong wrote:
> On Fri, Oct 18, 2024 at 08:00:21AM +0200, Greg KH wrote:
> > On Thu, Oct 17, 2024 at 11:58:10AM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <djwong@kernel.org>
> > > 
> > > Fix a minor bug where we fail repairs on metadata files that do not have
> > > attr forks because xrep_metadata_inode_subtype doesn't filter ENOENT.
> > > 
> > > Cc: <stable@vger.kernel.org> # v6.8
> > > Fixes: 5a8e07e799721b ("xfs: repair the inode core and forks of a metadata inode")
> > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > > ---
> > >  fs/xfs/scrub/repair.c |    8 +++++---
> > >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > Why is a bugfix / stable-tagged-patch, number 20 in a 29 patch series?
> > Why isn't it first, or better yet, on it's own if it is fixing a bug
> > that people want merged "soon"?
> 
> I have too many patches, and every time I try to get a set through the
> review process I end up having to write *more* patches to appease the
> reviewers, and fixes get lost.  Look at the copyrights on the other
> patches, I've been trying to get this upstreamed since 2018.
> 
> This particular bugfix got lost last month probably because I forgot to
> ping cem to take it for 6.12-rc1.  Thanks for pushing on this, Greg.
> 
> Hey Carlos, can you queue this one up for 6.12-rc5, please?

Sure, it's queued now in next-rc, I'm testing it and I hope to send it to
for-next today yet. It's the only patch for -rc5.

Cheers.
Carlos

> 
> --D
> 
> > thanks,
> > 
> > greg k-h
> > 
>
Carlos Maiolino Oct. 26, 2024, 7:29 a.m. UTC | #4 
On Thu, 17 Oct 2024 11:58:10 -0700, Darrick J. Wong wrote:
> Fix a minor bug where we fail repairs on metadata files that do not have
> attr forks because xrep_metadata_inode_subtype doesn't filter ENOENT.
> 
> 

Applied to for-next, thanks!

[20/29] xfs: don't fail repairs on metadata files with no attr fork
        commit: af8512c5277d17aae09be5305daa9118d2fa8881

Best regards,
diff mbox series

Patch

diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
index 3b4f6c207576a6..646ac8ade88d0b 100644
--- a/fs/xfs/scrub/repair.c
+++ b/fs/xfs/scrub/repair.c
@@ -1083,9 +1083,11 @@  xrep_metadata_inode_forks(
 		return error;
 
 	/* Make sure the attr fork looks ok before we delete it. */
-	error = xrep_metadata_inode_subtype(sc, XFS_SCRUB_TYPE_BMBTA);
-	if (error)
-		return error;
+	if (xfs_inode_hasattr(sc->ip)) {
+		error = xrep_metadata_inode_subtype(sc, XFS_SCRUB_TYPE_BMBTA);
+		if (error)
+			return error;
+	}
 
 	/* Clear the reflink flag since metadata never shares. */
 	if (xfs_is_reflink_inode(sc->ip)) {