Re: [PATCH v2 7/7] xfs: mark speculative prealloc CoW fork extents unwritten

[Christoph Hellwig <hch@infradead.org>]

Looks fine (and tests fine):

Reviewed-by: Christoph Hellwig <hch@lst.de>

But a few style nitpicks below:


> +	struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
> +	xfs_fileoff_t		offset_fsb;
> +	xfs_fileoff_t		end_fsb;
> +	xfs_extnum_t		idx;
> +	bool			found;
> +	int			error;
> +
> +	offset_fsb = XFS_B_TO_FSBT(mp, offset);
> +	end_fsb = XFS_B_TO_FSB(mp, offset + count);

Move these into the lines with the variable declarations, please.

> +	xfs_ilock(ip, XFS_ILOCK_EXCL);
> +
> +	/* Convert all the extents to real from unwritten. */
> +	for (found = xfs_iext_lookup_extent(ip, ifp, offset_fsb, &idx, &got);
> +	     found && got.br_startoff < end_fsb;
> +	     found = xfs_iext_get_extent(ifp, ++idx, &got)) {
> +		if (got.br_state == XFS_EXT_NORM)
> +			continue;
> +		error = __xfs_reflink_convert_cow(ip, &got, offset_fsb,
> +				end_fsb - offset_fsb, &dfops);

Move the state check into __xfs_reflink_convert_cow and rename that
to xfs_reflink_convert_cow_extent?

> @@ -389,10 +479,13 @@ xfs_reflink_allocate_cow_range(
>  		if (error) {
>  			trace_xfs_reflink_allocate_cow_range_error(ip, error,
>  					_RET_IP_);
> -			break;
> +			goto out;
>  		}
>  	}
>  
> +	/* Convert the CoW extents to regular. */
> +	error = xfs_reflink_convert_cow(ip, offset, count);
> +out:
>  	return error;

No need for the goto here really, justt return the error directly.