Re: [PATCH 2/9] spaceman/defrag: pick up segments from target file

[Christoph Hellwig <hch@infradead.org>]

On Wed, Jul 17, 2024 at 02:11:16PM +1000, Dave Chinner wrote:
> Yes, I know why you've done it. These were the same arguments made a
> while back for a new way of cloning files on XFS. We solved those
> problems just with a small change to the locking, and didn't need
> new ioctls or lots of new code just to solve the "clone blocks
> concurrent IO" problem.
> 
> I'm looking at this from exactly the same POV. The code presented is
> doing lots of complex, unusable stuff to work around the fact that
> UNSHARE blocks concurrent IO. I don't see any difference between
> CLONE and UNSHARE from the IO perspective - if anything UNSHARE can
> have looser rules than CLONE, because a concurrent write will either
> do the COW of a shared block itself, or hit the exclusive block that
> has already been unshared.
> 
> So if we fix these locking issues in the kernel, then the whole need
> for working around the IO concurrency problems with UNSHARE goes
> away and the userspace code becomes much, much simpler.

Btw, the main problem with unshare isn't just locking, but that is
extremely inefficient.  It synchronously reads one block at a time,
which makes it very, very slow.  That's purely a kernel implementation
detail, of course.