diff mbox series

[1/2] xfs: fix deadlock retry tracepoint arguments

Message ID 162086769410.3685697.9016566085994934364.stgit@magnolia (mailing list archive)
State Accepted
Headers show
Series xfs: random pending stuff for 5.13 | expand

Commit Message

Darrick J. Wong May 13, 2021, 1:01 a.m. UTC
From: Darrick J. Wong <djwong@kernel.org>

sc->ip is the inode that's being scrubbed, which means that it's not set
for scrub types that don't involve inodes.  If one of those scrubbers
(e.g. inode btrees) returns EDEADLOCK, we'll trip over the null pointer.
Fix that by reporting either the file being examined or the file that
was used to call scrub.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/scrub/common.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Brian Foster May 13, 2021, 12:07 p.m. UTC | #1
On Wed, May 12, 2021 at 06:01:34PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> sc->ip is the inode that's being scrubbed, which means that it's not set
> for scrub types that don't involve inodes.  If one of those scrubbers
> (e.g. inode btrees) returns EDEADLOCK, we'll trip over the null pointer.
> Fix that by reporting either the file being examined or the file that
> was used to call scrub.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/scrub/common.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/fs/xfs/scrub/common.c b/fs/xfs/scrub/common.c
> index aa874607618a..be38c960da85 100644
> --- a/fs/xfs/scrub/common.c
> +++ b/fs/xfs/scrub/common.c
> @@ -74,7 +74,9 @@ __xchk_process_error(
>  		return true;
>  	case -EDEADLOCK:
>  		/* Used to restart an op with deadlock avoidance. */
> -		trace_xchk_deadlock_retry(sc->ip, sc->sm, *error);
> +		trace_xchk_deadlock_retry(
> +				sc->ip ? sc->ip : XFS_I(file_inode(sc->file)),
> +				sc->sm, *error);
>  		break;
>  	case -EFSBADCRC:
>  	case -EFSCORRUPTED:
>
Christoph Hellwig May 19, 2021, 1:20 p.m. UTC | #2
On Wed, May 12, 2021 at 06:01:34PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> sc->ip is the inode that's being scrubbed, which means that it's not set
> for scrub types that don't involve inodes.  If one of those scrubbers
> (e.g. inode btrees) returns EDEADLOCK, we'll trip over the null pointer.
> Fix that by reporting either the file being examined or the file that
> was used to call scrub.

Without an indication of which one we trace this is a little weird,
isn't it?  Still better than a crash, though..
Darrick J. Wong May 19, 2021, 11:35 p.m. UTC | #3
On Wed, May 19, 2021 at 02:20:22PM +0100, Christoph Hellwig wrote:
> On Wed, May 12, 2021 at 06:01:34PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > sc->ip is the inode that's being scrubbed, which means that it's not set
> > for scrub types that don't involve inodes.  If one of those scrubbers
> > (e.g. inode btrees) returns EDEADLOCK, we'll trip over the null pointer.
> > Fix that by reporting either the file being examined or the file that
> > was used to call scrub.
> 
> Without an indication of which one we trace this is a little weird,
> isn't it?  Still better than a crash, though..

The scrub type is also encoded in the tracepoint, so we can tell that
the inode number is meaningless.

--D
diff mbox series

Patch

diff --git a/fs/xfs/scrub/common.c b/fs/xfs/scrub/common.c
index aa874607618a..be38c960da85 100644
--- a/fs/xfs/scrub/common.c
+++ b/fs/xfs/scrub/common.c
@@ -74,7 +74,9 @@  __xchk_process_error(
 		return true;
 	case -EDEADLOCK:
 		/* Used to restart an op with deadlock avoidance. */
-		trace_xchk_deadlock_retry(sc->ip, sc->sm, *error);
+		trace_xchk_deadlock_retry(
+				sc->ip ? sc->ip : XFS_I(file_inode(sc->file)),
+				sc->sm, *error);
 		break;
 	case -EFSBADCRC:
 	case -EFSCORRUPTED: