Re: [RFC 0/5] ext4: Implement support for extsize hints

[John Garry <john.g.garry@oracle.com>]

On 11/09/2024 10:01, Ojaswin Mujoo wrote:
> This patchset implements extsize hint feature for ext4. Posting this RFC to get
> some early review comments on the design and implementation bits. This feature
> is similar to what we have in XFS too with some differences.
> 
> extsize on ext4 is a hint to mballoc (multi-block allocator) and extent
> handling layer to do aligned allocations. We use allocation criteria 0
> (CR_POWER2_ALIGNED) for doing aligned power-of-2 allocations. With extsize hint
> we try to align the logical start (m_lblk) and length(m_len) of the allocation
> to be extsize aligned. CR_POWER2_ALIGNED criteria in mballoc automatically make
> sure that we get the aligned physical start (m_pblk) as well. So in this way
> extsize can make sure that lblk, len and pblk all are aligned for the allocated
> extent w.r.t extsize.
> 
> Note that extsize feature is just a hinting mechanism to ext4 multi-block
> allocator. That means that if we are unable to get an aligned allocation for
> some reason, than we drop this flag and continue with unaligned allocation to
> serve the request. However when we will add atomic/untorn writes support, then
> we will enforce the aligned allocation and can return -ENOSPC if aligned
> allocation was not successful.

A few questions/confirmations:
- You have no intention of adding an equivalent of forcealign, right?

- Would you also plan on using FS_IOC_FS(GET/SET)XATTR interface for 
enabling atomic writes on a per-inode basis?

- Can extsize be set at mkfs time?

- Is there any userspace support for this series available?

- how would/could extsize interact with bigalloc?

> 
> Comparison with XFS extsize feature -
> =====================================
> 1. extsize in XFS is a hint for aligning only the logical start and the lengh
>     of the allocation v/s extsize on ext4 make sure the physical start of the
>     extent gets aligned as well.

note that forcealign with extsize aligns AG block also

only for atomic writes do we enforce the AG block is aligned to physical 
block

> 
> 2. eof allocation on XFS trims the blocks allocated beyond eof with extsize
>     hint. That means on XFS for eof allocations (with extsize hint) only logical
>     start gets aligned. However extsize hint in ext4 for eof allocation is not
>     supported in this version of the series.
> 
> 3. XFS allows extsize to be set on file with no extents but delayed data.
>     However, ext4 don't allow that for simplicity. The user is expected to set
>     it on a file before changing it's i_size.
> 
> 4. XFS allows non-power-of-2 values for extsize but ext4 does not, since we
>     primarily would like to support atomic writes with extsize.
> 
> 5. In ext4 we chose to store the extsize value in SYSTEM_XATTR rather than an
>     inode field as it was simple and most flexible, since there might be more
>     features like atomic/untorn writes coming in future.
> 
> 6. In buffered-io path XFS switches to non-delalloc allocations for extsize hint.
>     The same has been kept for EXT4 as well.
> 
> Some TODOs:
> ===========
> 1. EOF allocations support can be added and can be kept similar to XFS

Note that EOF alignment for forcealign may change - it needs to be 
discussed further.

Thanks,
John

.
> 
> Rest of the design details can be found in the individual commit messages.
> 
> Thoughts and suggestions are welcome!
> 
> Ojaswin Mujoo (5):
>    ext4: add aligned allocation hint in mballoc
>    ext4: allow inode preallocation for aligned alloc
>    ext4: Support for extsize hint using FS_IOC_FS(GET/SET)XATTR
>    ext4: pass lblk and len explicitly to ext4_split_extent*()
>    ext4: Add extsize hint support
> 
>   fs/ext4/ext4.h              |  12 +-
>   fs/ext4/ext4_jbd2.h         |  15 ++
>   fs/ext4/extents.c           | 224 ++++++++++++++----
>   fs/ext4/inode.c             | 442 +++++++++++++++++++++++++++++++++---
>   fs/ext4/ioctl.c             | 119 ++++++++++
>   fs/ext4/mballoc.c           | 126 ++++++++--
>   fs/ext4/super.c             |   1 +
>   include/trace/events/ext4.h |   2 +
>   8 files changed, 841 insertions(+), 100 deletions(-)
>