Re: [PATCH 3/2] xfs: fix double ijoin in xfs_reflink_clear_inode_flag()

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Wed, Mar 07, 2018 at 08:19:42PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Another assert failure:
> 
> XFS: Assertion failed: !(lip->li_flags & XFS_LI_TRANS), file: fs/xfs/xfs_trans.c, line: 740
> ....
> xfs_trans_add_item+0xcc/0xe0
> xfs_reflink_clear_inode_flag+0x53/0x120
> xfs_reflink_try_clear_inode_flag+0x5b/0xa0
> ? filemap_write_and_wait+0x4f/0x70
> xfs_reflink_unshare+0x18e/0x19d
> xfs_file_fallocate+0x241/0x310
> ? selinux_file_permission+0xd4/0x140
> vfs_fallocate+0x13d/0x260
> SyS_fallocate+0x43/0x80
> 
> Another fix.
> 
> Signed-Off-By: Dave Chinner <dchinner@redhat.com>

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

--D

> ---
>  fs/xfs/xfs_reflink.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 6225d1ea3fdb..5cf60a042e8f 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -1554,7 +1554,12 @@ xfs_reflink_inode_has_shared_extents(
>  	return 0;
>  }
>  
> -/* Clear the inode reflink flag if there are no shared extents. */
> +/*
> + * Clear the inode reflink flag if there are no shared extents.
> + *
> + * The caller is responsible for joining the inode to the transaction passed in.
> + * The inode will be joined to the transaction that is returned to the caller.
> + */
>  int
>  xfs_reflink_clear_inode_flag(
>  	struct xfs_inode	*ip,
> @@ -1573,7 +1578,6 @@ xfs_reflink_clear_inode_flag(
>  	 * We didn't find any shared blocks so turn off the reflink flag.
>  	 * First, get rid of any leftover CoW mappings.
>  	 */
> -	xfs_trans_ijoin(*tpp, ip, 0);
>  	error = xfs_reflink_cancel_cow_blocks(ip, tpp, 0, NULLFILEOFF, true);
>  	if (error)
>  		return error;
> --
> 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