On Wed, Sep 18, 2019 at 07:25:45PM +0200, Christoph Hellwig wrote: > On Wed, Sep 18, 2019 at 10:17:33AM -0700, Darrick J. Wong wrote: > > > /* > > > * Pre-COW all shared blocks within a given byte range of a file and turn off > > > * the reflink flag if we unshare all of the file's blocks. > > > + * > > > + * Let iomap iterate all extents to see which are shared and not unwritten or > > > + * delalloc and read them into the page cache, dirty them, fsync them back out, > > > + * and then we can update the inode flag. What happens if we run out of > > > + * memory? :) > > > > I don't know, what /does/ happen? :) > > > > It /should/ be fine, right? Writeback will start pushing the dirty > > cache pages to disk, and since writeback only takes the ILOCK, it should > > be able to perform the COW even while the unshare process sits on the > > IOLOCK/MMAPLOCK. True, the unshare process and writeback will both be > > contending on the ILOCK, but that shouldn't be a problem... > > > > ...unless I'm missing something? It sure does look nice to drain all > > this other code out. > > No idea. This is your old code just moved down to this function. But > yes, the comment looks rather confusing and maybe we should just remove > it. I know. I remember testing it way back in 4.9 when we first merged it to make sure it really did work that way, and I think the locking model hasn't changed so it should be fine. But just wanted another set of eyes to make sure things haven't subtlely shifted since then. :) --D
