On Tue, Sep 10, 2019 at 04:39:59PM +0200, hch@xxxxxx wrote: > 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? <shrug> You might as well, since it's still going to be a few weeks until I carve off a iomap work branch for 5.5... :) (He says stumbling in from vacation with half a brain) --D > > > > -- > > Goldwyn---end quoted text---