On Tue, Sep 10, 2019 at 12:48:14PM +0000, Goldwyn Rodrigues wrote: > > + if (srcmap.type) > > + end = min(end, srcmap.offset + srcmap.length); > > + if (pos + length > end) > > + length = end - pos; > > > > > Yes, that looks more correct. However, can we be smart and not bother > setting the minimum of end and srcmap.offset + srcmap.length if it is > not required? ie in situations where end coincides with block boundary. > Or if srcmap.length falls short, until the last block boundary of > iomap? > > I did think about this scenario. This case is specific to CoW and > thought this is best handled by filesystem's iomap_begin(). If this > goes in, the filesystems would have to "falsify" srcmap length > information to maximize the amount of I/O that goes in one iteration. The problem is really that we can easily run over the srcmap (that's what happened to me with XFS..) One thing you've done in btrfs that I haven't done yet in XFS is to simply not bother with filling out the srcmap if we don't need to (that is if the iteration is fully page aligned in your patch set - the unshare op in this series will complicate things a little). With that optimization the only case where the shortening of the iteration that matters is if the start is unaligned and needs a read-modify-write cycle, but the end is aligned and beyond the end of the srcmap. Is that such an important case? > > -- > Goldwyn---end quoted text---