From: Christoph Hellwig <hch@lst.de>
To: Shiyang Ruan <ruansy.fnst@fujitsu.com>
Cc: djwong@kernel.org, hch@lst.de, linux-xfs@vger.kernel.org,
dan.j.williams@intel.com, david@fromorbit.com,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
nvdimm@lists.linux.dev, rgoldwyn@suse.de,
viro@zeniv.linux.org.uk, willy@infradead.org,
Goldwyn Rodrigues <rgoldwyn@suse.com>
Subject: Re: [PATCH v7 6/8] fsdax: Dedup file range to use a compare function
Date: Mon, 23 Aug 2021 15:16:42 +0200 [thread overview]
Message-ID: <20210823131642.GD15536@lst.de> (raw)
In-Reply-To: <20210816060359.1442450-7-ruansy.fnst@fujitsu.com>
On Mon, Aug 16, 2021 at 02:03:57PM +0800, Shiyang Ruan wrote:
> + id = dax_read_lock();
> + while ((ret = iomap_iter2(&it_src, &it_dest, ops)) > 0) {
> + it_src.processed = it_dest.processed =
> + dax_range_compare_iter(&it_src, &it_dest, same);
> + }
> + dax_read_unlock(id);
I think it would be better to move the DAX locking into
dax_range_compare_iter to avoid very long hold times.
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(dax_dedupe_file_range_compare);
No need for the export.
> +EXPORT_SYMBOL(dax_remap_file_range_prep);
EXPORT_SYMBOL_GPL, please.
Attached is a totally untested patch that just has two levels of
iterations instead of the new iter2 helper:
diff --git a/fs/dax.c b/fs/dax.c
index 0e0536765a7efc..2b65471785290d 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -1885,6 +1885,7 @@ static loff_t dax_range_compare_iter(struct iomap_iter *it_src,
loff_t len = min(smap->length, dmap->length);
void *saddr, *daddr;
int ret;
+ int id;
if (smap->type == IOMAP_HOLE && dmap->type == IOMAP_HOLE) {
*same = true;
@@ -1896,47 +1897,56 @@ static loff_t dax_range_compare_iter(struct iomap_iter *it_src,
return 0;
}
+ id = dax_read_lock();
ret = dax_iomap_direct_access(smap, pos1, ALIGN(pos1 + len, PAGE_SIZE),
&saddr, NULL);
if (ret < 0)
- return -EIO;
+ goto out_unlock;
ret = dax_iomap_direct_access(dmap, pos2, ALIGN(pos2 + len, PAGE_SIZE),
&daddr, NULL);
if (ret < 0)
- return -EIO;
+ goto out_unlock;
*same = !memcmp(saddr, daddr, len);
if (!*same)
- return 0;
+ len = 0;
+ dax_read_unlock(id);
return len;
+
+out_unlock:
+ dax_read_unlock(id);
+ return -EIO;
}
int dax_dedupe_file_range_compare(struct inode *src, loff_t srcoff,
- struct inode *dest, loff_t destoff, loff_t len, bool *same,
+ struct inode *dst, loff_t dstoff, loff_t len, bool *same,
const struct iomap_ops *ops)
{
- struct iomap_iter it_src = {
+ struct iomap_iter src_iter = {
.inode = src,
.pos = srcoff,
.len = len,
};
- struct iomap_iter it_dest = {
- .inode = dest,
- .pos = destoff,
+ struct iomap_iter dst_iter = {
+ .inode = dst,
+ .pos = dstoff,
.len = len,
};
- int id, ret;
+ int ret;
- id = dax_read_lock();
- while ((ret = iomap_iter2(&it_src, &it_dest, ops)) > 0) {
- it_src.processed = it_dest.processed =
- dax_range_compare_iter(&it_src, &it_dest, same);
+ 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, same);
+ if (dst_iter.processed > 0)
+ src_iter.processed += dst_iter.processed;
+ else if (!src_iter.processed)
+ src_iter.processed = dst_iter.processed;
+ }
}
- dax_read_unlock(id);
return ret;
}
-EXPORT_SYMBOL_GPL(dax_dedupe_file_range_compare);
int dax_remap_file_range_prep(struct file *file_in, loff_t pos_in,
struct file *file_out, loff_t pos_out,
@@ -1946,4 +1956,4 @@ int dax_remap_file_range_prep(struct file *file_in, loff_t pos_in,
return __generic_remap_file_range_prep(file_in, pos_in, file_out,
pos_out, len, remap_flags, ops);
}
-EXPORT_SYMBOL(dax_remap_file_range_prep);
+EXPORT_SYMBOL_GPL(dax_remap_file_range_prep);
next prev parent reply other threads:[~2021-08-23 13:16 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-16 6:03 [PATCH v7 0/8] fsdax,xfs: Add reflink&dedupe support for fsdax Shiyang Ruan
2021-08-16 6:03 ` [PATCH v7 1/8] fsdax: Output address in dax_iomap_pfn() and rename it Shiyang Ruan
2021-08-18 21:01 ` Dan Williams
2021-08-16 6:03 ` [PATCH v7 2/8] fsdax: Introduce dax_iomap_cow_copy() Shiyang Ruan
2021-08-19 22:35 ` Dan Williams
2021-08-16 6:03 ` [PATCH v7 3/8] fsdax: Replace mmap entry in case of CoW Shiyang Ruan
2021-08-19 22:54 ` Dan Williams
2021-08-23 12:57 ` Christoph Hellwig
2021-08-27 3:22 ` Shiyang Ruan
2021-08-27 5:00 ` Dan Williams
2021-08-27 5:26 ` Shiyang Ruan
2021-08-16 6:03 ` [PATCH v7 4/8] fsdax: Add dax_iomap_cow_copy() for dax_iomap_zero Shiyang Ruan
2021-08-20 2:39 ` Dan Williams
2021-08-27 3:23 ` Shiyang Ruan
2021-08-16 6:03 ` [PATCH v7 5/8] iomap: Introduce iomap_iter2 for two files Shiyang Ruan
2021-08-23 12:58 ` Christoph Hellwig
2021-08-16 6:03 ` [PATCH v7 6/8] fsdax: Dedup file range to use a compare function Shiyang Ruan
2021-08-23 13:16 ` Christoph Hellwig [this message]
2021-08-16 6:03 ` [PATCH v7 7/8] fsdax: Introduce dax_iomap_ops for end of reflink Shiyang Ruan
2021-08-20 3:01 ` Dan Williams
[not found] ` <c7e68dc8-5a43-f727-c262-58dcf244c711@fujitsu.com>
2021-08-20 15:18 ` Dan Williams
2021-08-23 13:02 ` Christoph Hellwig
2021-08-27 3:29 ` Shiyang Ruan
2021-08-27 5:04 ` Dan Williams
2021-08-27 5:27 ` Shiyang Ruan
2021-08-16 6:03 ` [PATCH v7 8/8] fs/xfs: Add dax dedupe support Shiyang Ruan
2021-08-20 3:08 ` Dan Williams
2021-08-27 3:36 ` Shiyang Ruan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210823131642.GD15536@lst.de \
--to=hch@lst.de \
--cc=dan.j.williams@intel.com \
--cc=david@fromorbit.com \
--cc=djwong@kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-xfs@vger.kernel.org \
--cc=nvdimm@lists.linux.dev \
--cc=rgoldwyn@suse.com \
--cc=rgoldwyn@suse.de \
--cc=ruansy.fnst@fujitsu.com \
--cc=viro@zeniv.linux.org.uk \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).