diff mbox

[V2] xfs: do not log swapext extent owner changes for deleted inodes

Message ID b7a4250a-b348-0381-683b-74059c397a7d@sandeen.net (mailing list archive)
State Accepted
Headers show

Commit Message

Eric Sandeen March 24, 2018, 12:13 a.m. UTC
Today if we run fsr and crash, log replay can fail because
the recovery code tries to instantiate the donor inode from
disk to replay the swapext, but it's been deleted and we get
verifier failures when we try to read it off disk with
i_mode == 0.

Strip the extent owner changes out of the logged fields when
we're freeing the inode to avoid this.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

V2: Move the fix to xfs_ifree per bfoster's suggestion


--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Darrick J. Wong March 24, 2018, 1:03 a.m. UTC | #1
On Fri, Mar 23, 2018 at 07:13:43PM -0500, Eric Sandeen wrote:
> Today if we run fsr and crash, log replay can fail because
> the recovery code tries to instantiate the donor inode from
> disk to replay the swapext, but it's been deleted and we get
> verifier failures when we try to read it off disk with
> i_mode == 0.
> 
> Strip the extent owner changes out of the logged fields when
> we're freeing the inode to avoid this.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks ok, will test...
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
> 
> V2: Move the fix to xfs_ifree per bfoster's suggestion
> 
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 604ee38..d17e2d5 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -2470,6 +2470,10 @@
>  	ip->i_d.di_forkoff = 0;		/* mark the attr fork not in use */
>  	ip->i_d.di_format = XFS_DINODE_FMT_EXTENTS;
>  	ip->i_d.di_aformat = XFS_DINODE_FMT_EXTENTS;
> +
> +	/* Don't attempt to replay owner changes for a deleted inode */
> +	ip->i_itemp->ili_fields &= !(XFS_ILOG_AOWNER|XFS_ILOG_DOWNER);
> +
>  	/*
>  	 * Bump the generation count so no one will be confused
>  	 * by reincarnations of this inode.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Brian Foster March 26, 2018, 12:13 p.m. UTC | #2
On Fri, Mar 23, 2018 at 07:13:43PM -0500, Eric Sandeen wrote:
> Today if we run fsr and crash, log replay can fail because
> the recovery code tries to instantiate the donor inode from
> disk to replay the swapext, but it's been deleted and we get
> verifier failures when we try to read it off disk with
> i_mode == 0.
> 
> Strip the extent owner changes out of the logged fields when
> we're freeing the inode to avoid this.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> V2: Move the fix to xfs_ifree per bfoster's suggestion
> 
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 604ee38..d17e2d5 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -2470,6 +2470,10 @@
>  	ip->i_d.di_forkoff = 0;		/* mark the attr fork not in use */
>  	ip->i_d.di_format = XFS_DINODE_FMT_EXTENTS;
>  	ip->i_d.di_aformat = XFS_DINODE_FMT_EXTENTS;
> +
> +	/* Don't attempt to replay owner changes for a deleted inode */
> +	ip->i_itemp->ili_fields &= !(XFS_ILOG_AOWNER|XFS_ILOG_DOWNER);

				   ~(XFS_ILOG_AOWNER|XFS_ILOG_DOWNER)

Brian

> +
>  	/*
>  	 * Bump the generation count so no one will be confused
>  	 * by reincarnations of this inode.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Sandeen March 26, 2018, 1:37 p.m. UTC | #3
On 3/26/18 7:13 AM, Brian Foster wrote:
> On Fri, Mar 23, 2018 at 07:13:43PM -0500, Eric Sandeen wrote:
>> Today if we run fsr and crash, log replay can fail because
>> the recovery code tries to instantiate the donor inode from
>> disk to replay the swapext, but it's been deleted and we get
>> verifier failures when we try to read it off disk with
>> i_mode == 0.
>>
>> Strip the extent owner changes out of the logged fields when
>> we're freeing the inode to avoid this.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> V2: Move the fix to xfs_ifree per bfoster's suggestion
>>
>> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
>> index 604ee38..d17e2d5 100644
>> --- a/fs/xfs/xfs_inode.c
>> +++ b/fs/xfs/xfs_inode.c
>> @@ -2470,6 +2470,10 @@
>>  	ip->i_d.di_forkoff = 0;		/* mark the attr fork not in use */
>>  	ip->i_d.di_format = XFS_DINODE_FMT_EXTENTS;
>>  	ip->i_d.di_aformat = XFS_DINODE_FMT_EXTENTS;
>> +
>> +	/* Don't attempt to replay owner changes for a deleted inode */
>> +	ip->i_itemp->ili_fields &= !(XFS_ILOG_AOWNER|XFS_ILOG_DOWNER);
> 
> 				   ~(XFS_ILOG_AOWNER|XFS_ILOG_DOWNER)
> 
> Brian

Oh man.  :(  Total brain burp, thanks for catching that.
Some days I think my hair is getting too pointy to keep doing this work.  :(

-Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 604ee38..d17e2d5 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2470,6 +2470,10 @@ 
 	ip->i_d.di_forkoff = 0;		/* mark the attr fork not in use */
 	ip->i_d.di_format = XFS_DINODE_FMT_EXTENTS;
 	ip->i_d.di_aformat = XFS_DINODE_FMT_EXTENTS;
+
+	/* Don't attempt to replay owner changes for a deleted inode */
+	ip->i_itemp->ili_fields &= !(XFS_ILOG_AOWNER|XFS_ILOG_DOWNER);
+
 	/*
 	 * Bump the generation count so no one will be confused
 	 * by reincarnations of this inode.