Re: [PATCH v2 5/5] xfs: toggle XFS_DIFLAG2_IOMAP_IMMUTABLE in response to fallocate

[Darrick J. Wong]

On Thu, Aug 03, 2017 at 07:28:35PM -0700, Dan Williams wrote:
> After validating the state of the file as not having holes, shared
> extents, or active mappings try to commit the
> XFS_DIFLAG2_IOMAP_IMMUTABLE flag to the on-disk inode metadata. If that
> succeeds then allow the S_IOMAP_IMMUTABLE to be set on the vfs inode.
> 
> Cc: Jan Kara <jack@xxxxxxx>
> Cc: Jeff Moyer <jmoyer@xxxxxxxxxx>
> Cc: Christoph Hellwig <hch@xxxxxx>
> Cc: Ross Zwisler <ross.zwisler@xxxxxxxxxxxxxxx>
> Suggested-by: Dave Chinner <david@xxxxxxxxxxxxx>
> Suggested-by: "Darrick J. Wong" <darrick.wong@xxxxxxxxxx>
> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
> ---
>  fs/xfs/xfs_bmap_util.c |   32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index 70ac2d33ab27..8464c25a2403 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -1436,9 +1436,11 @@ xfs_seal_file_space(
>  	xfs_off_t		offset,
>  	xfs_off_t		len)
>  {
> +	struct xfs_mount	*mp = ip->i_mount;
>  	struct inode		*inode = VFS_I(ip);
>  	struct address_space	*mapping = inode->i_mapping;
>  	int			error;
> +	struct xfs_trans	*tp;
>
>  	ASSERT(xfs_isilocked(ip, XFS_MMAPLOCK_EXCL));
>  
> @@ -1454,6 +1456,10 @@ xfs_seal_file_space(
>  	if (error)
>  		return error;
>  
> +	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_ichange, 0, 0, 0, &tp);
> +	if (error)
> +		return error;
> +
>  	xfs_ilock(ip, XFS_ILOCK_EXCL);
>  	/*
>  	 * Either the size changed after we performed allocation /
> @@ -1486,10 +1492,20 @@ xfs_seal_file_space(
>  	if (error < 0)
>  		goto out_unlock;
>  
> +	xfs_trans_ijoin(tp, ip, 0);

FWIW if you change that third parameter to XFS_ILOCK_EXCL then
xfs_trans_commit will do the xfs_iunlock(ip, XFS_ILOCK_EXCL) for you if
the commit succeeds...

> +	ip->i_d.di_flags2 |= XFS_DIFLAG2_IOMAP_IMMUTABLE;
> +	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
> +	error = xfs_trans_commit(tp);
> +	tp = NULL; /* nothing to cancel */
> +	if (error)
> +		goto out_unlock;
> +
>  	inode->i_flags |= S_IOMAP_IMMUTABLE;

...and then you can just return out here.

--D

>  out_unlock:
>  	xfs_iunlock(ip, XFS_ILOCK_EXCL);
> +	if (tp)
> +		xfs_trans_cancel(tp);
>  
>  	return error;
>  }
> @@ -1500,15 +1516,21 @@ xfs_unseal_file_space(
>  	xfs_off_t		offset,
>  	xfs_off_t		len)
>  {
> +	struct xfs_mount	*mp = ip->i_mount;
>  	struct inode		*inode = VFS_I(ip);
>  	struct address_space	*mapping = inode->i_mapping;
>  	int			error;
> +	struct xfs_trans	*tp;
>  
>  	ASSERT(xfs_isilocked(ip, XFS_MMAPLOCK_EXCL));
>  
>  	if (offset)
>  		return -EINVAL;
>  
> +	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_ichange, 0, 0, 0, &tp);
> +	if (error)
> +		return error;
> +
>  	xfs_ilock(ip, XFS_ILOCK_EXCL);
>  	/*
>  	 * It does not make sense to unseal less than the full range of
> @@ -1527,11 +1549,21 @@ xfs_unseal_file_space(
>  	if (mapping_mapped(mapping))
>  		goto out_unlock;
>  
> +	xfs_trans_ijoin(tp, ip, 0);
> +	ip->i_d.di_flags2 &= ~XFS_DIFLAG2_IOMAP_IMMUTABLE;
> +	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
> +	error = xfs_trans_commit(tp);
> +	tp = NULL; /* nothing to cancel */
> +	if (error)
> +		goto out_unlock;
> +
>  	inode->i_flags &= ~S_IOMAP_IMMUTABLE;
>  	error = 0;
>  
>  out_unlock:
>  	xfs_iunlock(ip, XFS_ILOCK_EXCL);
> +	if (tp)
> +		xfs_trans_cancel(tp);
>  
>  	return error;
>  }
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html