Re: [PATCH v2 5/8] fsdax: dedupe: iter two files at the same time

From: Darrick J. Wong
Date: Thu Dec 01 2022 - 19:06:02 EST


On Thu, Dec 01, 2022 at 03:31:41PM +0000, Shiyang Ruan wrote:
> The iomap_iter() on a range of one file may loop more than once. In
> this case, the inner dst_iter can update its iomap but the outer
> src_iter can't. This may cause the wrong remapping in filesystem. Let
> them called at the same time.
>
> Signed-off-by: Shiyang Ruan <ruansy.fnst@xxxxxxxxxxx>

Thank you for adding that explanation, it makes the problem much more
obvious. :)

Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>

--D

> ---
> fs/dax.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/fs/dax.c b/fs/dax.c
> index f1eb59bee0b5..354be56750c2 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -1964,15 +1964,15 @@ int dax_dedupe_file_range_compare(struct inode *src, loff_t srcoff,
> .len = len,
> .flags = IOMAP_DAX,
> };
> - int ret;
> + int ret, compared = 0;
>
> - while ((ret = iomap_iter(&src_iter, ops)) > 0) {
> - while ((ret = iomap_iter(&dst_iter, ops)) > 0) {
> - dst_iter.processed = dax_range_compare_iter(&src_iter,
> - &dst_iter, len, same);
> - }
> - if (ret <= 0)
> - src_iter.processed = ret;
> + while ((ret = iomap_iter(&src_iter, ops)) > 0 &&
> + (ret = iomap_iter(&dst_iter, ops)) > 0) {
> + compared = dax_range_compare_iter(&src_iter, &dst_iter, len,
> + same);
> + if (compared < 0)
> + return ret;
> + src_iter.processed = dst_iter.processed = compared;
> }
> return ret;
> }
> --
> 2.38.1
>