Re: [PATCH 1/4] iomap: Lift blocksize restriction on atomic writes

[Dave Chinner]

On Wed, Jan 22, 2025 at 10:45:34AM +0000, John Garry wrote:
> On 22/01/2025 06:42, Christoph Hellwig wrote:
> > On Fri, Jan 17, 2025 at 10:49:34AM -0800, Darrick J. Wong wrote:
> > > The trouble is that the br_startoff attribute of cow staging mappings
> > > aren't persisted on disk anywhere, which is why exchange-range can't
> > > handle the cow fork.  You could open an O_TMPFILE and swap between the
> > > two files, though that gets expensive per-io unless you're willing to
> > > stash that temp file somewhere.
> > 
> > Needing another inode is better than trying to steal ranges from the
> > actual inode we're operating on.  But we might just need a different
> > kind of COW staging for that.
> > 
> > > 
> > > At this point I think we should slap the usual EXPERIMENTAL warning on
> > > atomic writes through xfs and let John land the simplest multi-fsblock
> > > untorn write support, which only handles the corner case where all the
> > > stars are <cough> aligned; and then make an exchange-range prototype
> > > and/or all the other forcealign stuff.
> > 
> > That is the worst of all possible outcomes.  Combing up with an
> > atomic API that fails for random reasons only on aged file systems
> > is literally the worst thing we can do.  NAK.
> > 
> > 
> 
> I did my own quick PoC to use CoW for misaligned blocks atomic writes
> fallback.
> 
> I am finding that the block allocator is often giving misaligned blocks wrt
> atomic write length, like this:

Of course - I'm pretty sure this needs force-align to ensure that
the large allocated extent is aligned to file offset and hardware
atomic write alignment constraints....

> Since we are not considering forcealign ATM, can we still consider some
> other alignment hint to the block allocator? It could be similar to how
> stripe alignment is handled.

Perhaps we should finish off the the remaining bits needed to make
force-align work everywhere before going any further?

> Some other thoughts:
> - I am not sure what atomic write unit max we would now use.

What statx exposes should be the size/alignment for hardware offload
to take place (i.e. no change), regardless of what the filesystem
can do software offloads for. i.e. like statx->stx_blksize is the
"preferred block size for efficient IO", the atomic write unit
information is the "preferred atomic write size and alignment for
efficient IO", not the maximum sizes supported...

> - Anything written back with CoW/exchange range will need FUA to ensure that
> the write is fully persisted.

I don't think so. The journal commit for the exchange range
operation will issue a cache flush before the journal IO is
submitted. that will make the new data stable before the first
xchgrange transaction becomes stable.

Hence we get the correct data/metadata ordering on stable storage
simply by doing the exchange-range operation at data IO completion.
This the same data/metadata ordering semantics that unwritten extent
conversion is based on....

-Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx