On Wed, Feb 17, 2021 at 11:24:18AM +0800, Ruan Shiyang wrote: > > > On 2021/2/10 下午9:19, Christoph Hellwig wrote: > > On Tue, Feb 09, 2021 at 05:46:13PM +0800, Ruan Shiyang wrote: > > > > > > > > > On 2021/2/9 下午5:34, Christoph Hellwig wrote: > > > > On Tue, Feb 09, 2021 at 05:15:13PM +0800, Ruan Shiyang wrote: > > > > > The dax dedupe comparison need the iomap_ops pointer as argument, so my > > > > > understanding is that we don't modify the argument list of > > > > > generic_remap_file_range_prep(), but move its code into > > > > > __generic_remap_file_range_prep() whose argument list can be modified to > > > > > accepts the iomap_ops pointer. Then it looks like this: > > > > > > > > I'd say just add the iomap_ops pointer to > > > > generic_remap_file_range_prep and do away with the extra wrappers. We > > > > only have three callers anyway. > > > > > > OK. > > > > So looking at this again I think your proposal actaully is better, > > given that the iomap variant is still DAX specific. Sorry for > > the noise. > > > > Also I think dax_file_range_compare should use iomap_apply instead > > of open coding it. > > > > There are two files, which are not reflinked, need to be direct_access() > here. The iomap_apply() can handle one file each time. So, it seems that > iomap_apply() is not suitable for this case... > > > The pseudo code of this process is as follows: > > srclen = ops->begin(&srcmap) > destlen = ops->begin(&destmap) > > direct_access(&srcmap, &saddr) > direct_access(&destmap, &daddr) > > same = memcpy(saddr, daddr, min(srclen,destlen)) > > ops->end(&destmap) > ops->end(&srcmap) > > I think a nested call like this is necessary. That's why I use the open > code way. This might be a good place to implement an iomap_apply2() loop that actually /does/ walk all the extents of file1 and file2. There's now two users of this idiom. (Possibly structured as a "get next mappings from both" generator function like Matthew Wilcox keeps asking for. :)) --D > > -- > Thanks, > Ruan Shiyang. > > > >