ext4: Implement support for extsize hints
Subsystems (3)
Ojaswin Mujoo
reviewercover ext4: Implement support for extsize hints
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.
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.
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.
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(-)
Signature · 1 line
2.43.5
Ojaswin Mujoo
reviewerRFC 2/5 ext4: allow inode preallocation for aligned alloc
Enable inode preallocation support for aligned allocations. Inode preallocation will only be used if the preallocated blocks are able to satisfy the length and alignment requirements of the allocations, else we disable preallocation for this particular allocation and proceed as usual. Disabling inode preallocation is required otherwise we might end up with overlapping preallocated ranges which can trigger a BUG() later.
Further, during normalizing, we usually try to round it up to a power of 2 which can still give us aligned allocation. We also make sure not change the goal start so aligned allocation is more straightforward. If for whatever reason the goal is not power of 2 or doesn't contain the original request, then we throw a warning and proceed as normal.
For now, group preallocation is disabled for aligned allocations.
--- fs/ext4/mballoc.c | 96 +++++++++++++++++++++++++++++++---------------- 1 file changed, 63 insertions(+), 33 deletions(-)
fs/ext4/mballoc.c +63 -33
@@ -2178,8 +2178,6 @@ static void ext4_mb_use_best_found(struct ext4_allocation_context *ac,* space in a special descriptor. */ if (ac->ac_o_ex.fe_len < ac->ac_b_ex.fe_len) { -/* Aligned allocation doesn't have preallocation support */ -WARN_ON (ac->ac_flags &EXT4_MB_HINT_ALIGNED );ext4_mb_new_preallocation (ac); }
@@ -3027,8 +3025,7 @@ ext4_mb_regular_allocator(struct ext4_allocation_context *ac)WARN_ON_ONCE (!is_power_of_2 (len));WARN_ON_ONCE (start % len); -/* We don't support preallocation yet */ -WARN_ON_ONCE (ac->ac_b_ex.fe_len != ac->ac_o_ex.fe_len); +WARN_ON_ONCE (ac->ac_b_ex.fe_len < ac->ac_o_ex.fe_len); } exit:
@@ -4479,13 +4476,6 @@ ext4_mb_normalize_request(struct ext4_allocation_context *ac,if (ac->ac_flags &EXT4_MB_HINT_NOPREALLOC )return ; -/* -* caller may have strict alignment requirements. In this case, avoid -* normalization since it is not alignment aware. -*/ -if (ac->ac_flags &EXT4_MB_HINT_ALIGNED ) -return ; -if (ac->ac_flags &EXT4_MB_HINT_GROUP_ALLOC ) {ext4_mb_normalize_group_request (ac);return ;
@@ -4542,6 +4532,21 @@ ext4_mb_normalize_request(struct ext4_allocation_context *ac, size = (loff_t )EXT4_C2B (sbi, ac->ac_o_ex.fe_len) << bsbits; } + +/* +* For aligned allocations, we need to ensure 2 things: +* +* 1. The start should remain same as original start so that finding +* aligned physical blocks for it is straight forward. +* +* 2. The new_size should not be less than the original len. This +* can sometimes happen due to the way we predict size above. +*/ +if (ac->ac_flags &EXT4_MB_HINT_ALIGNED ) { + start_off = ac->ac_o_ex.fe_logical << bsbits; + size =max_t (loff_t, size, +EXT4_C2B (sbi, ac->ac_o_ex.fe_len) << bsbits); + } size = size >> bsbits; start = start_off >> bsbits;
@@ -4792,32 +4797,46 @@ ext4_mb_check_group_pa(ext4_fsblk_t goal_block, }/* -* check if found pa meets EXT4_MB_HINT_GOAL_ONLY +* check if found pa meets EXT4_MB_HINT_GOAL_ONLY or EXT4_MB_HINT_ALIGNED */ static bool -ext4_mb_pa_goal_check (struct ext4_allocation_context *ac, +ext4_mb_pa_check (struct ext4_allocation_context *ac,struct ext4_prealloc_space *pa) {struct ext4_sb_info *sbi =EXT4_SB (ac->ac_sb);ext4_fsblk_t start; -if (likely (!(ac->ac_flags &EXT4_MB_HINT_GOAL_ONLY ))) +if (likely (!(ac->ac_flags &EXT4_MB_HINT_GOAL_ONLY || + ac->ac_flags &EXT4_MB_HINT_ALIGNED )))return true; -/* -* If EXT4_MB_HINT_GOAL_ONLY is set, ac_g_ex will not be adjusted -* in ext4_mb_normalize_request and will keep same with ac_o_ex -* from ext4_mb_initialize_context. Choose ac_g_ex here to keep -* consistent with ext4_mb_find_by_goal. -*/ - start = pa->pa_pstart + - (ac->ac_g_ex.fe_logical - pa->pa_lstart); -if (ext4_grp_offs_to_block (ac->ac_sb, &ac->ac_g_ex) != start) -return false; +if (ac->ac_flags &EXT4_MB_HINT_GOAL_ONLY ) { +/* +* If EXT4_MB_HINT_GOAL_ONLY is set, ac_g_ex will not be adjusted +* in ext4_mb_normalize_request and will keep same with ac_o_ex +* from ext4_mb_initialize_context. Choose ac_g_ex here to keep +* consistent with ext4_mb_find_by_goal. +*/ + start = pa->pa_pstart + + (ac->ac_g_ex.fe_logical - pa->pa_lstart); +if (ext4_grp_offs_to_block (ac->ac_sb, &ac->ac_g_ex) != start) +return false; -if (ac->ac_g_ex.fe_len > pa->pa_len - -EXT4_B2C (sbi, ac->ac_g_ex.fe_logical - pa->pa_lstart)) -return false; +if (ac->ac_g_ex.fe_len > + pa->pa_len -EXT4_B2C (sbi, ac->ac_g_ex.fe_logical - + pa->pa_lstart)) +return false; + }else if (ac->ac_flags &EXT4_MB_HINT_ALIGNED ) { + start = pa->pa_pstart + + (ac->ac_g_ex.fe_logical - pa->pa_lstart); +if (start %EXT4_C2B (sbi, ac->ac_g_ex.fe_len)) +return false; + +if (EXT4_C2B (sbi, ac->ac_g_ex.fe_len) > + (EXT4_C2B (sbi, pa->pa_len) - + (ac->ac_g_ex.fe_logical - pa->pa_lstart))) +return false; + }return true; }
@@ -4840,10 +4859,6 @@ ext4_mb_use_preallocated(struct ext4_allocation_context *ac)if (!(ac->ac_flags &EXT4_MB_HINT_DATA ))return false; -/* using preallocated blocks is not alignment aware. */ -if (ac->ac_flags &EXT4_MB_HINT_ALIGNED ) -return false; -/* * first, try per-file preallocation by searching the inode pa rbtree. *
@@ -4949,7 +4964,7 @@ ext4_mb_use_preallocated(struct ext4_allocation_context *ac) goto try_group_pa; } -if (tmp_pa->pa_free &&likely (ext4_mb_pa_goal_check (ac, tmp_pa))) { +if (tmp_pa->pa_free &&likely (ext4_mb_pa_check (ac, tmp_pa))) {atomic_inc (&tmp_pa->pa_count);ext4_mb_use_inode_pa (ac, tmp_pa);spin_unlock (&tmp_pa->pa_lock);
@@ -4984,6 +4999,19 @@ ext4_mb_use_preallocated(struct ext4_allocation_context *ac)* pa_free == 0. */ WARN_ON_ONCE (tmp_pa->pa_free ==0 ); + +/* +* If, for any reason, we reach here then we need to disable PA +* because otherwise ext4_mb_normalize_request() will try to +* allocate a new PA for this logical range where another PA +* already exists. This is not allowed and will trigger BUG_ONs. +* Hence, as a workaround we disable PA. +* +* NOTE: ideally we would want to have some logic to take care +* of the unusable PA. Maybe a more fine grained discard logic +* that could allow us to discard only specific PAs. +*/ + ac->ac_flags |=EXT4_MB_HINT_NOPREALLOC ; }spin_unlock (&tmp_pa->pa_lock); try_group_pa:
@@ -5790,6 +5818,7 @@ static void ext4_mb_group_or_file(struct ext4_allocation_context *ac)int bsbits = ac->ac_sb->s_blocksize_bits;loff_t size, isize;bool inode_pa_eligible, group_pa_eligible; +bool is_aligned = (ac->ac_flags &EXT4_MB_HINT_ALIGNED );if (!(ac->ac_flags &EXT4_MB_HINT_DATA ))return ;
@@ -5797,7 +5826,8 @@ static void ext4_mb_group_or_file(struct ext4_allocation_context *ac)if (unlikely (ac->ac_flags &EXT4_MB_HINT_GOAL_ONLY ))return ; - group_pa_eligible = sbi->s_mb_group_prealloc >0 ; +/* Aligned allocation does not support group pa */ + group_pa_eligible = (!is_aligned && sbi->s_mb_group_prealloc >0 ); inode_pa_eligible = true; size =extent_logical_end (sbi, &ac->ac_o_ex); isize = (i_size_read (ac->ac_inode) + ac->ac_sb->s_blocksize -1 )
Signature · 1 line
2.43.5
Ojaswin Mujoo
reviewerRFC 1/5 ext4: add aligned allocation hint in mballoc
In reply to Ojaswin MujooAdd support in mballoc for allocating blocks that are aligned to a certain power-of-2 offset.
1. We define a new flag EXT4_MB_ALIGNED_HINT to indicate that we want an aligned allocation. This is just a hint, mballoc tries its best to provide aligned blocks but if it can't then it'll fallback to normal allocation
2. The alignment is determined by the length of the allocation, for example if we ask for 8192 bytes, then the alignment of physical blocks will also be 8192 bytes aligned (ie 2 blocks aligned on 4k blocksize).
3. We dont yet support arbitrary alignment. For aligned writes, the length/alignment must be power of 2 in blocks, ie for 4k blocksize we can get 4k byte aligned, 8k byte aligned, 16k byte aligned ... allocation but not 12k byte aligned.
4. We use CR_POWER2_ALIGNED criteria for aligned allocation which by design allocates in an aligned manner. Since CR_POWER2_ALIGNED needs the ac->ac_g_ex.fe_len to be power of 2, thats where the restriction in point 3 above comes from. Since right now aligned allocation support is added mainly for atomic writes use case, this restriction should be fine since atomic write capable devices usually support only power of 2 alignments
5. For ease of review enabling inode preallocation support is done in upcoming patches and is disabled in this patch.
--- fs/ext4/ext4.h | 2 ++ fs/ext4/mballoc.c | 60 +++++++++++++++++++++++++++++++++---- include/trace/events/ext4.h | 1 + 3 files changed, 58 insertions(+), 5 deletions(-)
fs/ext4/ext4.h +2 -0
@@ -222,6 +222,8 @@ enum criteria {/* Avg fragment size rb tree lookup succeeded at least once for * CR_BEST_AVAIL_LEN */ #define EXT4_MB_CR_BEST_AVAIL_LEN_OPTIMIZED 0x00020000 +/* The allocation must respect alignment requirements for physical blocks */ +#define EXT4_MB_HINT_ALIGNED 0x40000struct ext4_allocation_request {/* target inode for block we're allocating */
fs/ext4/mballoc.c +55 -5
100 lines not on this page · raw message
include/trace/events/ext4.h +1 -0
@@ -36,6 +36,7 @@ struct partial_cluster; { EXT4_MB_STREAM_ALLOC, "STREAM_ALLOC" }, \ { EXT4_MB_USE_ROOT_BLOCKS, "USE_ROOT_BLKS" }, \ { EXT4_MB_USE_RESERVED, "USE_RESV" }, \ + { EXT4_MB_HINT_ALIGNED, "HINT_ALIGNED" }, \ { EXT4_MB_STRICT_CHECK, "STRICT_CHECK" }) #define show_map_flags(flags) __print_flags(flags, "|", \
Signature · 1 line
2.43.5
Ojaswin Mujoo
reviewerRFC 4/5 ext4: pass lblk and len explicitly to ext4_split_extent*()
In reply to Ojaswin MujooSince these functions only use the map to determine lblk and len of the split, pass them explicitly. This is in preparation for making them work with extent size hints cleanly.
No functional change in this patch.
--- fs/ext4/extents.c | 57 +++++++++++++++++++++++++---------------------- 1 file changed, 30 insertions(+), 27 deletions(-)
fs/ext4/extents.c +30 -27
@@ -3347,7 +3347,8 @@ static struct ext4_ext_path *ext4_split_extent_at(handle_t *handle,static struct ext4_ext_path *ext4_split_extent (handle_t *handle,struct inode *inode,struct ext4_ext_path *path, -struct ext4_map_blocks *map, +ext4_lblk_t lblk, +unsigned int len,int split_flag,int flags,unsigned int *allocated) {
@@ -3363,7 +3364,7 @@ static struct ext4_ext_path *ext4_split_extent(handle_t *handle, ee_len =ext4_ext_get_actual_len (ex); unwritten =ext4_ext_is_unwritten (ex); -if (map->m_lblk + map->m_len < ee_block + ee_len) { +if (lblk + len < ee_block + ee_len) { split_flag1 = split_flag &EXT4_EXT_MAY_ZEROOUT ; flags1 = flags |EXT4_GET_BLOCKS_PRE_IO ;if (unwritten)
@@ -3372,28 +3373,28 @@ static struct ext4_ext_path *ext4_split_extent(handle_t *handle,if (split_flag &EXT4_EXT_DATA_VALID2 ) split_flag1 |=EXT4_EXT_DATA_VALID1 ; path =ext4_split_extent_at (handle, inode, path, - map->m_lblk + map->m_len, split_flag1, flags1); + lblk + len, split_flag1, flags1);if (IS_ERR (path))return path;/* * Update path is required because previous ext4_split_extent_at * may result in split of original leaf or extent zeroout. */ - path =ext4_find_extent (inode, map->m_lblk, path, flags); + path =ext4_find_extent (inode, lblk, path, flags);if (IS_ERR (path))return path; depth =ext_depth (inode); ex = path[depth].p_ext;if (!ex) {EXT4_ERROR_INODE (inode,"unexpected hole at %lu" , - (unsigned long ) map->m_lblk); + (unsigned long ) lblk);ext4_free_ext_path (path);return ERR_PTR (-EFSCORRUPTED ); } unwritten =ext4_ext_is_unwritten (ex); } -if (map->m_lblk >= ee_block) { +if (lblk >= ee_block) { split_flag1 = split_flag &EXT4_EXT_DATA_VALID2 ;if (unwritten) { split_flag1 |=EXT4_EXT_MARK_UNWRIT1 ;
@@ -3401,16 +3402,16 @@ static struct ext4_ext_path *ext4_split_extent(handle_t *handle, EXT4_EXT_MARK_UNWRIT2); } path =ext4_split_extent_at (handle, inode, path, - map->m_lblk, split_flag1, flags); + lblk, split_flag1, flags);if (IS_ERR (path))return path; }if (allocated) { -if (map->m_lblk + map->m_len > ee_block + ee_len) - *allocated = ee_len - (map->m_lblk - ee_block); +if (lblk + len > ee_block + ee_len) + *allocated = ee_len - (lblk - ee_block);else - *allocated = map->m_len; + *allocated = len; }ext4_ext_show_leaf (inode, path);return path;
@@ -3658,8 +3659,8 @@ ext4_ext_convert_to_initialized(handle_t *handle, struct inode *inode, } fallback: - path =ext4_split_extent (handle, inode, path, &split_map, split_flag, - flags,NULL ); + path =ext4_split_extent (handle, inode, path, split_map.m_lblk, + split_map.m_len, split_flag, flags,NULL );if (IS_ERR (path))return path; out:
@@ -3699,11 +3700,11 @@ ext4_ext_convert_to_initialized(handle_t *handle, struct inode *inode,* allocated pointer. Return an extent path pointer on success, or an error * pointer on failure. */ -static struct ext4_ext_path *ext4_split_convert_extents (handle_t *handle, -struct inode *inode, -struct ext4_map_blocks *map, -struct ext4_ext_path *path, -int flags,unsigned int *allocated) +static struct ext4_ext_path * +ext4_split_convert_extents (handle_t *handle,struct inode *inode, +ext4_lblk_t lblk,unsigned int len, +struct ext4_ext_path *path,int flags, +unsigned int *allocated) {ext4_lblk_t eof_block;ext4_lblk_t ee_block;
@@ -3712,12 +3713,12 @@ static struct ext4_ext_path *ext4_split_convert_extents(handle_t *handle,int split_flag =0 , depth;ext_debug (inode,"logical block %llu, max_blocks %u\n" , - (unsigned long long )map->m_lblk, map->m_len); + (unsigned long long )lblk, len); eof_block = (EXT4_I (inode)->i_disksize + inode->i_sb->s_blocksize -1 ) >> inode->i_sb->s_blocksize_bits; -if (eof_block < map->m_lblk + map->m_len) - eof_block = map->m_lblk + map->m_len; +if (eof_block < lblk + len) + eof_block = lblk + len;/* * It is safe to convert extent to initialized via explicit * zeroout only if extent is fully inside i_size or new_size.
@@ -3737,8 +3738,8 @@ static struct ext4_ext_path *ext4_split_convert_extents(handle_t *handle, split_flag |= (EXT4_EXT_MARK_UNWRIT2 |EXT4_EXT_DATA_VALID2 ); } flags |=EXT4_GET_BLOCKS_PRE_IO ; -return ext4_split_extent (handle, inode, path, map, split_flag, flags, - allocated); +return ext4_split_extent (handle, inode, path, lblk, len, split_flag, + flags, allocated); } static struct ext4_ext_path *
@@ -3773,7 +3774,7 @@ ext4_convert_unwritten_extents_endio(handle_t *handle, struct inode *inode, inode->i_ino, (unsigned long long )ee_block, ee_len, (unsigned long long)map->m_lblk, map->m_len);#endif - path =ext4_split_convert_extents (handle, inode, map, path, + path =ext4_split_convert_extents (handle, inode, map->m_lblk, map->m_len, path,EXT4_GET_BLOCKS_CONVERT ,NULL );if (IS_ERR (path))return path;
@@ -3837,8 +3838,9 @@ convert_initialized_extent(handle_t *handle, struct inode *inode, (unsigned long long)ee_block, ee_len);if (ee_block != map->m_lblk || ee_len > map->m_len) { - path =ext4_split_convert_extents (handle, inode, map, path, -EXT4_GET_BLOCKS_CONVERT_UNWRITTEN ,NULL ); + path =ext4_split_convert_extents ( + handle, inode, map->m_lblk, map->m_len, path, +EXT4_GET_BLOCKS_CONVERT_UNWRITTEN ,NULL );if (IS_ERR (path))return path;
@@ -3909,8 +3911,9 @@ ext4_ext_handle_unwritten_extents(handle_t *handle, struct inode *inode,/* get_block() before submitting IO, split the extent */ if (flags &EXT4_GET_BLOCKS_PRE_IO ) { - path =ext4_split_convert_extents (handle, inode, map, path, - flags |EXT4_GET_BLOCKS_CONVERT , allocated); + path =ext4_split_convert_extents ( + handle, inode, map->m_lblk, map->m_len, path, + flags |EXT4_GET_BLOCKS_CONVERT , allocated);if (IS_ERR (path))return path;/*
Signature · 1 line
2.43.5
Ojaswin Mujoo
reviewerRFC 3/5 ext4: Support for extsize hint using FS_IOC_FS(GET/SET)XATTR
In reply to Ojaswin MujooThis patch adds support for getting and setting extsize hint using FS_IOC_GETXATTR and FS_IOC_SETXATTR interface. The extsize is stored in xattr of type EXT4_XATTR_INDEX_SYSTEM.
Restrictions on setting extsize:
1. extsize can't be set on files with data
2. extsize can't be set on non regular files
3. extsize hint can't be used with bigalloc (yet)
4. extsize (in blocks) should be power-of-2 for simplicity.
5. extsize must be a multiple of block size
The ioctl behavior has been kept as close to the XFS equivalent as possible.
ext4: Some modifications to extsize ioctl (To be Squashed)
--- fs/ext4/ext4.h | 6 +++ fs/ext4/inode.c | 89 ++++++++++++++++++++++++++++++++++++ fs/ext4/ioctl.c | 119 ++++++++++++++++++++++++++++++++++++++++++++++++ fs/ext4/super.c | 1 + 4 files changed, 215 insertions(+)
fs/ext4/ext4.h +6 -0
@@ -1170,6 +1170,8 @@ struct ext4_inode_info {__u32 i_csum_seed;kprojid_t i_projid; +/* The extentsize hint for the inode in blocks */ +ext4_grpblk_t i_extsize; };/*
@@ -3037,6 +3039,10 @@ extern void ext4_da_update_reserve_space(struct inode *inode, int used, int quota_claim);extern int ext4_issue_zeroout (struct inode *inode,ext4_lblk_t lblk,ext4_fsblk_t pblk,ext4_lblk_t len); +int ext4_inode_xattr_get_extsize (struct inode *inode); +int ext4_inode_xattr_set_extsize (struct inode *inode,ext4_grpblk_t extsize); +ext4_grpblk_t ext4_inode_get_extsize (struct ext4_inode_info *ei); +void ext4_inode_set_extsize (struct ext4_inode_info *ei,ext4_grpblk_t extsize);/* indirect.c */ extern int ext4_ind_map_blocks(handle_t *handle, struct inode *inode,
fs/ext4/inode.c +89 -0
98 lines not on this page · raw message
fs/ext4/ioctl.c +119 -0
143 lines not on this page · raw message
fs/ext4/super.c +1 -0
@@ -1421,6 +1421,7 @@ static struct inode *ext4_alloc_inode(struct super_block *sb)spin_lock_init (&ei->i_completed_io_lock); ei->i_sync_tid =0 ; ei->i_datasync_tid =0 ; + ei->i_extsize =0 ;atomic_set (&ei->i_unwritten,0 );INIT_WORK (&ei->i_rsv_conversion_work, ext4_end_io_rsv_work);ext4_fc_init_inode (&ei->vfs_inode);
Signature · 1 line
2.43.5
Ojaswin Mujoo
reviewerRFC 5/5 ext4: Add extsize hint support
In reply to Ojaswin MujooNow that the ioctl is in place, add the underlying infrastructure to support extent size hints.
** MOTIVATION **
1. This feature allows us to ask the allocator for blocks that are logically AS WELL AS physically aligned to an extent size hint (aka extsize), that is generally a power of 2.
2. This means both start and the length of the physical and logical range should be aligned to the extsize.
3. This sets up the infra we'll eventually need for supporting non-torn/atomic writes that need to follow a certain alignment as required by hardware.
4. This can also be extent to other use cases like stripe alignment
** DESIGN NOTES **
* Physical Alignment *
1. Since the extsize is always a power-of-2 (for now) in fs blocks, we leverage CR_POWER2_ALIGNED allocation to get the blocks. This ensures the blocks are physically aligned
2. Since this is just a hint, incase we are not able to get any aligned blocks we simply drop back to non aligned allocation.
* Logical Alignment *
The flow of extsize aligned allocation with bufferred and direct IO:
+--------------------------------------------------------+
| Buffered IO |
+--------------------------------------------------------+
| ext4_map_blocks() call with extsize allocation |
+--------------------------------------------------------+
|
+--------------------------------------------+
| Adjust lblk and len to align to extsize |
+--------------------------------------------+
|
+--------------------------------------------------------+
|Pre-existing written/unwritten blocks in extsize range? |
+--------------------------+-----------------------------+
YES NO
| |
+---------------v---------------+ +-----------------v-------------------+
| Covers orig range? | | Allocate extsize range |
+---------------+---------------+ +---------------------+--------------+
| | |
YES NO |
| | +---------------v------------------+
+--------v-------+ +-------v---------+ | Mark allocated extent as |
| Return blocks | | Fallback to | | unwritten |
+----------------+ | non-extsize | +----------------+-----------------+
| allocation | |
+-----------------+ +----------------v-----------------+
| Insert extsize extent |
| into tree |
+----------------+-----------------+
|
+----------------v-----------------+
| Return allocated blocks |
+----------------------------------+ +--------------------------------------------+
| During writeback: |
+--------------------------------------------+
| Use PRE_IO to split only the dirty extent |
+--------------------------------------------+ +--------------------------------------------+
| After IO: |
+--------------------------------------------+
| Convert the extent under IO to written |
+--------------------------------------------+Same flow for direct IO:
+----------------------------------------------------------------------+
| Direct IO |
+----------------------------------------------------------------------+
| ext4_map_blocks() called with extsize allocation and PRE-IO |
+----------------------------------------------------------------------+
|
+----------------------------------------------------------------------+
| Adjust lblk and len to align to extsize |
+----------------------------------------------------------------------+
|
+----------------------------------------------------------------------+
| Pre-existing written blocks in extsize range? |
+----------------------------------+-----------------------------------+
YES NO
| |
+---------v----------+ +------------------v-----------------+
| Covers orig range? | | Unwritten blocks in extsize range? |
+---------+----------+ +------------------+-----------------+
| | | |
YES NO YES NO
| | | |
+-------v----+ +-----v--------+ +----------v----------+ +-------v----------+
| Return | | Fallback to | | Call ext4_ext_map_ | | Allocate extsize |
| blocks | | non-extsize | | blocks() ->ext4_ext | | range |
+------------+ | allocation | | _handle_unwritten_ | +-------+----------+
+--------------+ | extents() | |
+----------+----------+ +-------v----------+
| | Mark complete |
+----------v----------+ | range unwritten |
| Split orig range | | & insert in |
| from bigger | | tree |
| unwritten extent | +-------+----------+
+----------+----------+ |
| +-------v----------+
+----------v----------+ | Split orig range |
| Mark split extent | | from bigger |
| as unwritten | | allocated extent |
+----------+----------+ +-------+----------+
| |
+----------v----------+ +-------v----------+
| Return split extent | | Mark split extent|
| to user | | as unwritten |
+---------------------+ +-------+----------+
|
+-------v----------+
| Return split |
| extent to user |
+------------------+ +--------------------------------------------+
| After IO: |
+--------------------------------------------+
| Convert the extent under IO to written |
+--------------------------------------------+** IMPLEMENTATION NOTES **
* Callers of ext4_map_blocks work under the assumption that ext4_map_blocks will always only return as much as requested or less but now we might end up allocating more so make changes to ext4_map_blocks to make sure we adjust the allocated map to only return as much as user requested.
* Further, we now maintain 2 maps in ext4_map_blocks - the original map and the extsize map that is used when extsize hint allocation is taking place. We also pass these 2 maps down because some functions might now need information of the original map as well as the extsize map.
* For example, when we go for direct IO and there's a hole in the orig range requested, we allocate based on extsize range and then split the bigger unwritten extent onto smaller unwritten extents based on orig range. (Needed so we dont have to split after IO). For this, we need the information of extsize range as well as orig range hence 2 maps.
* Since now we allocate more than the user requested, to avoid stale data exposure, we mark the bigger extsize extent as unwritten and then use the similar flow of dioread_nolock to only mark the extent under write as written.
* We disable extsize hints when writes are beyond EOF.
* When extsize is set on an inode, we drop to no delalloc allocations similar to XFS.
--- fs/ext4/ext4.h | 4 +- fs/ext4/ext4_jbd2.h | 15 ++ fs/ext4/extents.c | 169 +++++++++++++++-- fs/ext4/inode.c | 353 ++++++++++++++++++++++++++++++++---- include/trace/events/ext4.h | 1 + 5 files changed, 491 insertions(+), 51 deletions(-)
fs/ext4/ext4.h +3 -1
@@ -722,6 +722,7 @@ enum {#define EXT4_GET_BLOCKS_IO_SUBMIT 0x0400/* Caller is in the atomic contex, find extent if it has been cached */ #define EXT4_GET_BLOCKS_CACHED_NOWAIT 0x0800 +#define EXT4_GET_BLOCKS_EXTSIZE 0x1000/* * The bit position of these flags must not overlap with any of the
@@ -3702,7 +3703,8 @@ struct ext4_extent;extern void ext4_ext_tree_init (handle_t *handle,struct inode *inode);extern int ext4_ext_index_trans_blocks (struct inode *inode,int extents);extern int ext4_ext_map_blocks (handle_t *handle,struct inode *inode, -struct ext4_map_blocks *map,int flags); +struct ext4_map_blocks *orig_map, +struct ext4_map_blocks *extsize_map,int flags);extern int ext4_ext_truncate (handle_t *,struct inode *);extern int ext4_ext_remove_space (struct inode *inode,ext4_lblk_t start,ext4_lblk_t end);
fs/ext4/ext4_jbd2.h +15 -0
@@ -513,4 +513,19 @@ static inline int ext4_should_dioread_nolock(struct inode *inode) return 1; } +static inline int ext4_should_use_extsize (struct inode *inode) +{ +if (!S_ISREG (inode->i_mode)) +return 0 ; +if (!(ext4_test_inode_flag (inode,EXT4_INODE_EXTENTS ))) +return 0 ; +return (ext4_inode_get_extsize (EXT4_I (inode)) >0 ); +} + +static inline int ext4_should_use_unwrit_extents (struct inode *inode) +{ +return (ext4_should_dioread_nolock (inode) || +ext4_should_use_extsize (inode)); +} +#endif /* _EXT4_JBD2_H */
fs/ext4/extents.c +156 -13
255 lines not on this page · raw message
fs/ext4/inode.c +316 -37
558 lines not on this page · raw message
include/trace/events/ext4.h +1 -0
@@ -50,6 +50,7 @@ struct partial_cluster; { EXT4_GET_BLOCKS_CONVERT_UNWRITTEN, "CONVERT_UNWRITTEN" }, \ { EXT4_GET_BLOCKS_ZERO, "ZERO" }, \ { EXT4_GET_BLOCKS_IO_SUBMIT, "IO_SUBMIT" }, \ + { EXT4_GET_BLOCKS_EXTSIZE, "EXTSIZE" }, \ { EXT4_EX_NOCACHE, "EX_NOCACHE" })/*
Signature · 1 line
2.43.5
Quoted 13 lines · Ojaswin Mujoo
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) andlength(m_len)of the allocation to be extsize aligned.CR_POWER2_ALIGNEDcriteria 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.
Quoted 6 lines
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
-ENOSPCif 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?
Quoted 6 lines
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
Quoted 17 lines
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_XATTRrather than an inode field as it was simple and most flexible, since there might be more features like atomic/untorn writes coming in future.
Quoted 6 lines
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
.
Quoted 22 lines
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 supportfs/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(-)
Ritesh Harjani (IBM)
reviewerQuoted 21 lines · John Garry
Quoted 13 lines · Ojaswin Mujoo
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) andlength(m_len)of the allocation to be extsize aligned.CR_POWER2_ALIGNEDcriteria 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.Quoted 6 lines
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
-ENOSPCif aligned allocation was not successful.
Quoted 2 lines
A few questions/confirmations:
- You have no intention of adding an equivalent of forcealign, right?
extsize is just a hinting mechanism that too only for __allocation__ path. But for atomic writes we do require some form of forcealign (like how we have in XFS). So we could either call this directly as atomic write feature or can may as well call this forcealign feature and make atomic writes depend upon it, like how XFS is doing it.
I still haven't understood if there is/will be a user specifically for forcealign other than atomic writes.
Since you asked, I am more curious to know if there is some more context to your question?
Quoted 3 lines
- Would you also plan on using FS_IOC_FS(GET/SET)XATTR interface for enabling atomic writes on a per-inode basis?
Yes, that interface should indeed be kept same for EXT4 too.
Quoted 2 lines
- Can extsize be set at mkfs time?
Good point. For now in this series, extsize can only be set using the same ioctl on a per inode basis.
IIUC, XFS supports doing both right. We can do this on a per-inode basis during ioctl or it also supports setting this during mkfs.xfs time. (maybe xfsprogs only allows setting this at mkfs time for rtvolumes for now)
So if this is set during mkfs.xfs time and then by default all inodes will have this extsize attribute value set right?
BTW, this brings me to another question that I had asked here too [1].
1. For XFS, atomic writes can only be enabled with a fresh mkfs.xfs -d atomic-writes=1 right?
2. For atomic writes to be enabled, we need all 3 features to be enabled during mkfs.xfs time itself right?
i.e.
"mkfs.xfs -i forcealign=1 -d extsize=16384 -d atomic-writes=1"
[1]: https://lore.kernel.org/linux-xfs/[email protected]/
Quoted 2 lines
- Is there any userspace support for this series available?
Make sense to maybe provide a userspace support link too. For now, a quick hack would be to just allow setting extsize hint for other fileystems as well in xfs_io.
io/open.c +1 -1
@@ -980,7 +980,7 @@ open_init(void) extsize_cmd.args =_ ("[-D | -R] [extsize]" ); extsize_cmd.argmin =0 ; extsize_cmd.argmax =-1 ; - extsize_cmd.flags =CMD_NOMAP_OK ; + extsize_cmd.flags =CMD_NOMAP_OK |CMD_FOREIGN_OK ; extsize_cmd.oneline =_ ("get/set preferred extent size (in bytes) for the open file" ); extsize_cmd.help = extsize_help;
<e.g>/dev/loop6 on /mnt1/test type ext4 (rw,relatime)
root@qemu:~/xt/xfsprogs-dev# ./io/xfs_io -fc "extsize" /mnt1/test/f1 [0] /mnt1/test/f1 root@qemu:~/xt/xfsprogs-dev# ./io/xfs_io -c "extsize 16384" /mnt1/test/f1 root@qemu:~/xt/xfsprogs-dev# ./io/xfs_io -c "extsize" /mnt1/test/f1 [16384] /mnt1/test/f1
Quoted 3 lines
- how would/could extsize interact with bigalloc?
As of now it is kept disabled with bigalloc.
+ if (sbi->s_cluster_ratio > 1) {
+ msg = "Can't use extsize hint with bigalloc";
+ err = -EINVAL;
+ goto error;
+ }
Quoted 8 lines
Quoted 6 lines
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
Can you expand that on a bit. You mean during mkfs.xfs time we ensure agblock boundaries are extsize aligned?
Quoted 4 lines
only for atomic writes do we enforce the AG block is aligned to physical block
If you could expand that a bit please? You meant during mkfs.xfs time for atomic writes we ensure ag block start bounaries are extsize aligned?
Quoted 20 lines
Quoted 20 lines
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_XATTRrather 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.
Quoted 6 lines
Quoted 3 lines
Some TODOs:
===========
1. EOF allocations support can be added and can be kept similar to XFSNote that EOF alignment for forcealign may change - it needs to be discussed further.
Sure, thanks for pointing that out.
I guess you are referring to mainly the truncate related EOF alignment change required with forcealign for XFS.
-ritesh
Quoted 32 lines · Ritesh Harjani (IBM)
Quoted 21 lines · John Garry
Quoted 13 lines · Ojaswin Mujoo
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) andlength(m_len)of the allocation to be extsize aligned.CR_POWER2_ALIGNEDcriteria 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.Quoted 6 lines
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
-ENOSPCif aligned allocation was not successful.Quoted 2 lines
A few questions/confirmations:
- You have no intention of adding an equivalent of forcealign, right?extsize is just a hinting mechanism that too only for
__allocation__path. But for atomic writes we do require some form of forcealign (like how we have in XFS). So we could either call this directly as atomic write feature or can may as well call this forcealign feature and make atomic writes depend upon it, like how XFS is doing it.
Quoted 4 lines
I still haven't understood if there is/will be a user specifically for forcealign other than atomic writes.
Quoted 1 line
Since you asked, I am more curious to know if there is some more context
to your question?
As Darrick mentioned at the following, forcealign could be used for DAX:
https://lore.kernel.org/linux-xfs/170404855884.1770028.10371509002317647981.stgit@frogsfrogsfrogs/
Quoted 8 lines
Quoted 4 lines
- Would you also plan on using FS_IOC_FS(GET/SET)XATTR interface for enabling atomic writes on a per-inode basis?
Yes, that interface should indeed be kept same for EXT4 too.
Quoted 1 line
Quoted 7 lines
Quoted 1 line
- Can extsize be set at mkfs time?
Good point. For now in this series, extsize can only be set using the same ioctl on a per inode basis.
IIUC, XFS supports doing both right. We can do this on a per-inode basis during ioctl or it also supports setting this during mkfs.xfs time.
Right
Quoted 1 line
(maybe xfsprogs only allows setting this at mkfs time for rtvolumes for now)
extsize hint can already be set at mkfs time for both rtvol and !rtvol today.
Quoted 3 lines
So if this is set during mkfs.xfs time and then by default all inodes will have this extsize attribute value set right?
Right
But there is still the option to set this later with xfs_io -c "extsize" per-inode.
Quoted 4 lines
BTW, this brings me to another question that I had asked here too [1].
1. For XFS, atomic writes can only be enabled with a fresh mkfs.xfs -d atomic-writes=1 right?
Correct
Setting atomic-writes=1 enables the feature in the SB
Quoted 2 lines
2. For atomic writes to be enabled, we need all 3 features to be enabled during mkfs.xfs time itself right?
Right, that is how it is currently done. But you could easily set extsize=4K at mkfs time so that not all inodes have a 16KB extsize, as in the example below. In this case, certain atomic write inodes could have their extsize increased to 16KB.
Quoted 26 lines
i.e.
"mkfs.xfs -i forcealign=1 -d extsize=16384 -d atomic-writes=1"Quoted 2 lines
- Is there any userspace support for this series available?
Make sense to maybe provide a userspace support link too. For now, a quick hack would be to just allow setting extsize hint for other fileystems as well in
xfs_io.io/open.c +1
-1@@ -980,7 +980,7 @@ open_init(void) extsize_cmd.args =_ ("[-D | -R] [extsize]" ); extsize_cmd.argmin =0 ; extsize_cmd.argmax =-1 ; - extsize_cmd.flags =CMD_NOMAP_OK ; + extsize_cmd.flags =CMD_NOMAP_OK |CMD_FOREIGN_OK ; extsize_cmd.oneline =_ ("get/set preferred extent size (in bytes) for the open file" ); extsize_cmd.help = extsize_help;
Quoted 8 lines
<e.g>
/dev/loop6on /mnt1/test type ext4 (rw,relatime)root@qemu:~/xt/xfsprogs-dev# ./io/xfs_io -fc "extsize" /mnt1/test/f1 [0] /mnt1/test/f1 root@qemu:~/xt/xfsprogs-dev# ./io/xfs_io -c "extsize 16384" /mnt1/test/f1 root@qemu:~/xt/xfsprogs-dev# ./io/xfs_io -c "extsize" /mnt1/test/f1 [16384] /mnt1/test/f1
ok
Quoted 22 lines
Quoted 5 lines
- how would/could extsize interact with bigalloc?
As of now it is kept disabled with bigalloc.
io/open.c +5
-0+if (sbi->s_cluster_ratio >1 ) { + msg ="Can't use extsize hint with bigalloc" ; + err = -EINVAL ; + goto error; + }Quoted 9 lines
Quoted 8 lines
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.
Quoted 4 lines
Quoted 1 line
note that forcealign with extsize aligns AG block also
Can you expand that on a bit. You mean during mkfs.xfs time we ensure agblock boundaries are extsize aligned?
Yes, see align_ag_geometry() at https://github.com/johnpgarry/xfsprogs-dev/commits/atomic-writes/
Quoted 8 lines
Quoted 5 lines
only for atomic writes do we enforce the AG block is aligned to physical block
If you could expand that a bit please? You meant during mkfs.xfs time for atomic writes we ensure ag block start bounaries are extsize aligned?
We do this for forcealign with the extsize value supplied at mkfs time.
There are 2x things to consider about this:
- mkfs-specified extsize need not necessarily be a power-of-2
- even if this mkfs-specified extsize is a power-of-2, attempting to increase extsize for an inode enabled for atomic writes may be restricted, as the new extsize may not align with the AG count.
For example, extsize was 64KB and AG count = 16400 FSB (1025 * 64KB), then we cannot enable an inode for atomic writes with extsize = 128KB, as the disk block would not be aligned with the AG block.
Quoted 26 lines
Quoted 26 lines
Quoted 25 lines
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_XATTRrather 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
Quoted 7 lines
Quoted 2 lines
Note that EOF alignment for forcealign may change - it needs to be discussed further.
Sure, thanks for pointing that out.
I guess you are referring to mainly the truncate related EOF alignment change required with forcealign for XFS.
Thanks,
John
kernel test robot
build botRe: [RFC 5/5] ext4: Add extsize hint support
In reply to Ojaswin Mujookernel test robot noticed the following build warnings:
Hi Ojaswin,
[This is a private test report for your RFC patch.] kernel test robot noticed the following build warnings:
[auto build test WARNING on tytso-ext4/dev]
[also build test WARNING on next-20240913]
[cannot apply to linus/master v6.11-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Ojaswin-Mujoo/ext4-add-aligned-allocation-hint-in-mballoc/20240911-170427 base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev patch link: https://lore.kernel.org/r/d906230bd4a1bc414926f6e9e38b75d41f2a6f47.1726034272.git.ojaswin%40linux.ibm.com patch subject: [RFC 5/5] ext4: Add extsize hint support config: x86_64-defconfig (https://download.01.org/0day-ci/archive/20240914/[email protected]/config) compiler: gcc-11 (Debian 11.3.0-12) 11.3.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240914/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
fs/ext4/inode.c: In function 'ext4_map_blocks':
>> fs/ext4/inode.c:711:22: warning: variable 'orig_mlen' set but not used [-Wunused-but-set-variable]
711 | unsigned int orig_mlen, extsize_mlen;
| ^~~~~~~~~
--
>> fs/ext4/inode.c:607: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst* Extsize hint will change the mapped range and hence we'll end up mapping more.
vim +/orig_mlen +711 fs/ext4/inode.c
605
606 /**
> 607 * Extsize hint will change the mapped range and hence we'll end up mapping more.
608 * To not confuse the caller, adjust the struct ext4_map_blocks to reflect the
609 * original mapping requested by them.
610 *
611 * @cur_map: The block mapping we are working with (for sanity check)
612 * @orig_map: The originally requested mapping
613 * @extsize_map: The mapping after adjusting for extsize hint
614 * @flags Get block flags (for sanity check)
615 *
616 * This function assumes that the orig_mlblk is contained within the mapping
617 * held in extsize_map. Caller must make sure this is true.
618 */
619 static inline unsigned int ext4_extsize_adjust_map(struct ext4_map_blocks *cur_map,
620 struct ext4_map_blocks *orig_map,
621 struct ext4_map_blocks *extsize_map,
622 int flags)
623 {
624 __u64 map_end = (__u64)extsize_map->m_lblk + extsize_map->m_len;
625
626 BUG_ON(cur_map != extsize_map || !(flags & EXT4_GET_BLOCKS_EXTSIZE));
627
628 orig_map->m_len = min(orig_map->m_len, map_end - orig_map->m_lblk);
629 orig_map->m_pblk =
630 extsize_map->m_pblk + (orig_map->m_lblk - extsize_map->m_lblk);
631 orig_map->m_flags = extsize_map->m_flags;
632
633 return orig_map->m_len;
634 }
635
636 /**
637 * ext4_error_adjust_map - Adjust map returned upon error in ext4_map_blocks()
638 *
639 * @cur_map: current map we are working with
640 * @orig_map: original map that would be returned to the user.
641 *
642 * Most of the callers of ext4_map_blocks() ignore the map on error, however
643 * some use it for debug logging. In this case, they log state of the map just
644 * before the error, hence this function ensures that map returned to caller is
645 * the one we were working with when error happened. Mostly useful when extsize
646 * hints are enabled.
647 */
648 static inline void ext4_error_adjust_map(struct ext4_map_blocks *cur_map,
649 struct ext4_map_blocks *orig_map)
650 {
651 if (cur_map != orig_map)
652 memcpy(orig_map, cur_map, sizeof(*cur_map));
653 }
654
655 /*
656 * This functions resets the mapping to it's original state after it has been
657 * modified due to extent size hint and drops the extsize hint. To be used
658 * incase we want to fallback from extsize based aligned allocation to normal
659 * allocation
660 *
661 * @map: The block mapping where lblk and len have been modified
662 * because of extsize hint
663 * @flags: The get_block flags
664 * @orig_mlblk: The originally requested logical block to map
665 * @orig_mlen: The originally requested len to map
666 * @orig_flags: The originally requested get_block flags
667 */
668 static inline void ext4_extsize_reset_map(struct ext4_map_blocks *map,
669 int *flags, ext4_lblk_t orig_mlblk,
670 unsigned int orig_mlen,
671 int orig_flags)
672 {
673 /* Drop the extsize hint from original flags */
674 *flags = orig_flags & ~EXT4_GET_BLOCKS_EXTSIZE;
675 map->m_lblk = orig_mlblk;
676 map->m_len = orig_mlen;
677 map->m_flags = 0;
678 }
679
680 /*
681 * The ext4_map_blocks() function tries to look up the requested blocks,
682 * and returns if the blocks are already mapped.
683 *
684 * Otherwise it takes the write lock of the i_data_sem and allocate blocks
685 * and store the allocated blocks in the result buffer head and mark it
686 * mapped.
687 *
688 * If file type is extents based, it will call ext4_ext_map_blocks(),
689 * Otherwise, call with ext4_ind_map_blocks() to handle indirect mapping
690 * based files
691 *
692 * On success, it returns the number of blocks being mapped or allocated.
693 * If flags doesn't contain EXT4_GET_BLOCKS_CREATE the blocks are
694 * pre-allocated and unwritten, the resulting @map is marked as unwritten.
695 * If the flags contain EXT4_GET_BLOCKS_CREATE, it will mark @map as mapped.
696 *
697 * It returns 0 if plain look up failed (blocks have not been allocated), in
698 * that case, @map is returned as unmapped but we still do fill map->m_len to
699 * indicate the length of a hole starting at map->m_lblk.
700 *
701 * It returns the error in case of allocation failure.
702 */
703 int ext4_map_blocks(handle_t *handle, struct inode *inode,
704 struct ext4_map_blocks *orig_map, int flags)
705 {
706 struct extent_status es;
707 int retval;
708 int ret = 0;
709
710 ext4_lblk_t orig_mlblk, extsize_mlblk;
> 711 unsigned int orig_mlen, extsize_mlen; 712 int orig_flags;
713
714 struct ext4_map_blocks *map = NULL;
715 struct ext4_map_blocks extsize_map = {0};
716
717 __u32 extsize = ext4_inode_get_extsize(EXT4_I(inode));
718 bool should_extsize = false;
719
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
kernel test robot
build botRe: [RFC 1/5] ext4: add aligned allocation hint in mballoc
In reply to Ojaswin Mujookernel test robot noticed the following build errors:
Hi Ojaswin,
[This is a private test report for your RFC patch.] kernel test robot noticed the following build errors:
[auto build test ERROR on tytso-ext4/dev]
[also build test ERROR on linus/master v6.11-rc7 next-20240913] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Ojaswin-Mujoo/ext4-add-aligned-allocation-hint-in-mballoc/20240911-170427 base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev patch link: https://lore.kernel.org/r/3a17d8d55c88799a3af9a7d337f183863912e28e.1726034272.git.ojaswin%40linux.ibm.com patch subject: [RFC 1/5] ext4: add aligned allocation hint in mballoc config: i386-allyesconfig (https://download.01.org/0day-ci/archive/20240914/[email protected]/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240914/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
ld: fs/ext4/mballoc.o: in function `ext4_mb_regular_allocator':
>> mballoc.c:(.text+0xde5c): undefined reference to `__umoddi3'
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
kernel test robot
build botRe: [RFC 1/5] ext4: add aligned allocation hint in mballoc
In reply to Ojaswin Mujookernel test robot noticed the following build errors:
Hi Ojaswin,
[This is a private test report for your RFC patch.] kernel test robot noticed the following build errors:
[auto build test ERROR on tytso-ext4/dev]
[also build test ERROR on linus/master v6.11-rc7 next-20240913] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Ojaswin-Mujoo/ext4-add-aligned-allocation-hint-in-mballoc/20240911-170427 base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev patch link: https://lore.kernel.org/r/3a17d8d55c88799a3af9a7d337f183863912e28e.1726034272.git.ojaswin%40linux.ibm.com patch subject: [RFC 1/5] ext4: add aligned allocation hint in mballoc config: i386-allmodconfig (https://download.01.org/0day-ci/archive/20240915/[email protected]/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240915/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>, old ones prefixed by <<):
WARNING: modpost: missing MODULE_DESCRIPTION() in arch/x86/mm/testmmiotrace.o
WARNING: modpost: missing MODULE_DESCRIPTION() in kernel/locking/test-ww_mutex.o
WARNING: modpost: missing MODULE_DESCRIPTION() in lib/test_objpool.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/imx/mxc-clk.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/imx/clk-imxrt1050.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/devfreq/governor_simpleondemand.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/devfreq/governor_performance.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/devfreq/governor_powersave.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/devfreq/governor_userspace.o
>> ERROR: modpost: "__umoddi3" [fs/ext4/ext4.ko] undefined!
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Quoted 20 lines · Ojaswin Mujoo
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) andlength(m_len)of the allocation to be extsize aligned.CR_POWER2_ALIGNEDcriteria 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
-ENOSPCif aligned allocation was not successful.
Quoted 5 lines
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.
What happens when you can't align the physical start of the extent? It fails the allocation with ENOSPC?
For XFS, the existing extent size behaviour is a hint, and so we ignore the hint if we cannot perform the allocation with the suggested alignment. i.e. We should not fail an allocation with an extent size hint until we are actually very near ENOSPC.
With the new force-align feature, the physical alignment within an AG gets aligned to the extent size. In this case, if we can't find an aligned free extent to allocate, we fail the allocation (ENOSPC). Hence with forced alignment, we can have ENOSPC occur when there are large amounts of free space available in the filesystem.
This is almost certainly what most people -don't want-, but it is a requirement for atomic writes. To make matters worse, this behaviour will almost certainly get worst as filesystem ages and free space slowly fragments over time.
IOWs, by making the ext4 extsize have forced alignment semantics by default, it means users will see ENOSPC at lot more frequently and in situations where it is most definitely not expected.
We also have to keep in mind that there are applications out there that set and use extent size hints, and so enabling extsize in ext4 will result in those applications silently starting to use them. If ext4 supporting extsize hints drastically changes the behaviour of the filesystem then that is going to cause significant unexpected regressions for users as they upgrade kernels and filesystems.
Hence I strongly suggest that ext4 implements extent size hints in the same way that XFS does. i.e. unless forced alignment has been enabled for the inode, extsize is just a hint that gets discarded if aligned allocation does not succeed.
Behaviour such as extent size hinting *should* be the same across all filesystems that provide this functionality. This makes using extent size hints much easier for users, admins and application developers. The last thing I want to hear is application devs tell me at conferences that "we don't use extent size hints anymore because ext4..."
Quoted 3 lines
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.
I'm not sure I understand what you are saying here. XFS does extsize alignment of both the start and end of post-eof extents the same as it does for extents within EOF. For example:
# xfs_io -fdc "truncate 0" -c "extsize 16k" -c "pwrite 0 4k" -c "bmap -vvp" foo
wrote 4096/4096 bytes at offset 0
4 KiB, 1 ops; 0.0308 sec (129.815 KiB/sec and 32.4538 ops/sec)
foo:
EXT: FILE-OFFSET BLOCK-RANGE AG AG-OFFSET TOTAL FLAGS
0: [0..7]: 256504..256511 0 (256504..256511) 8 000000
1: [8..31]: 256512..256535 0 (256512..256535) 24 010000
FLAG Values:
0100000 Shared extent
0010000 Unwritten preallocated extentThere's a 4k written extent at 0, and a 12k unwritten extent beyond EOF at 4k. I.e. we have an extent of 16kB as the hint required that is correctly aligned beyond EOF.
If I then write another 4k at 20k (beyond both EOF and the unwritten extent beyond EOF:
# xfs_io -fdc "truncate 0" -c "extsize 16k" -c "pwrite 0 4k" -c "pwrite 20k 4k" -c "bmap -vvp" foo
wrote 4096/4096 bytes at offset 0
4 KiB, 1 ops; 0.0210 sec (190.195 KiB/sec and 47.5489 ops/sec)
wrote 4096/4096 bytes at offset 20480
4 KiB, 1 ops; 0.0001 sec (21.701 MiB/sec and 5555.5556 ops/sec)
foo:
EXT: FILE-OFFSET BLOCK-RANGE AG AG-OFFSET TOTAL FLAGS
0: [0..7]: 180000..180007 0 (180000..180007) 8 000000
1: [8..39]: 180008..180039 0 (180008..180039) 32 010000
2: [40..47]: 180040..180047 0 (180040..180047) 8 000000
3: [48..63]: 180048..180063 0 (180048..180063) 16 010000
FLAG Values:
0100000 Shared extent
0010000 Unwritten preallocated extentYou can see we did contiguous allocation of another 16kB at offset 16kB, and then wrote to 20k for 4kB.. i.e. the new extent was correctly aligned at both sides as the extsize hint says it should be....
Quoted 2 lines
However extsize hint in ext4 for eof allocation is not supported in this version of the series.
If you can't do extsize aligned allocations for EOF extension, then how to applications use atomic writes to atomically extend the file?
Quoted 1 line
3. XFS allows extsize to be set on file with no extents but delayed data.
It does?
<looks>
Yep, it doesn't check ip->i_delayed_blks is zero when changing extsize.
I think that's simply a bug, not intended behaviour, because delalloc will not have reserved space for the extsize hint rounding needed when writeback occurs. Can you send a patch to add this check?
Quoted 2 lines
However, ext4 don't allow that for simplicity. The user is expected to set it on a file before changing it's
i_size.
We don't actually care about i_size in XFS - the determining factor is whether there are extents allocated on disk. i.e. we can truncate up and then set the extent size hint because there are no extents allocated even though the size is non-zero.
There are almost certainly applications out there that change extent size after truncating to a non-zero size, so this needs to work on ext4 the same way it does on XFS. Otherwise people are going to complain that their applications suddenly stop working properly on ext4....
Quoted 2 lines
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.
Yes, ext4 can make that restriction if desired.
Keep in mind that the XFS atomic write support is still evolving, and I think the way we are using extent size hints isn't fully solidified yet.
Indeed, I think that we can allow non-power-of-2 extent sizes for atomic writes, because integer multiples of the atomic write unit will still ensure that physical extents are properly aligned for atomic writes to succeed. e.g. 24kB extent size is compatible with 8kB atomic write sizes.
To make that work efficiently unwritten extent boundaries need to be maintained at atomic write alignments (8kB), not extent size alignment (24kB), but other than that I don't think anything else is needed....
This is desirable because it will allow extent size hints to remain usable for their original purposes even with atomic writes on XFS. i.e. fragmentation minimisation for small random DIO write worklaods (exactly the sort of IO you'd consider using atomic writes for!), alignment of extents to [non-power-of-2] RAID stripe geometry, etc.
Quoted 3 lines
5. In ext4 we chose to store the extsize value in
SYSTEM_XATTRrather than an inode field as it was simple and most flexible, since there might be more features like atomic/untorn writes coming in future.
Does that mean you can query and set it through the user xattr interfaces? If so, how do you enforce the values users set are correct?
Quoted 2 lines
6. In buffered-io path XFS switches to non-delalloc allocations for extsize hint. The same has been kept for EXT4 as well.
That's an internal XFS implementation detail that you don't need to replicate. Historically speaking, we didn't use unwritten extents for delayed allocation and so we couldn't do within-EOF extsize unaligned writes without adding special additional zero-around code to ensure that we never exposed stale data to userspace from the extra allocation that the data write did not cover.
We now use unwritten extents for delalloc conversion, so this istale data exposure issue no longer exists. We should really switch this code back to using delalloc because it is much faster and less fragmentation prone than direct extsize allocation....
-Dave.
Signature · 2 lines
Dave Chinner [email protected]
Ojaswin Mujoo
reviewerQuoted 21 lines · Dave Chinner
Quoted 20 lines · Ojaswin Mujoo
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) andlength(m_len)of the allocation to be extsize aligned.CR_POWER2_ALIGNEDcriteria 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
-ENOSPCif aligned allocation was not successful.
Quoted 8 lines
Quoted 5 lines
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.What happens when you can't align the physical start of the extent? It fails the allocation with ENOSPC?
Hi Dave, thanks for the review.
No, ext4 treats it as a hint as well and we fallback to nonaligned allocation
Quoted 33 lines
For XFS, the existing extent size behaviour is a hint, and so we ignore the hint if we cannot perform the allocation with the suggested alignment. i.e. We should not fail an allocation with an extent size hint until we are actually very near ENOSPC.
With the new force-align feature, the physical alignment within an AG gets aligned to the extent size. In this case, if we can't find an aligned free extent to allocate, we fail the allocation (ENOSPC). Hence with forced alignment, we can have ENOSPC occur when there are large amounts of free space available in the filesystem.
This is almost certainly what most people -don't want-, but it is a requirement for atomic writes. To make matters worse, this behaviour will almost certainly get worst as filesystem ages and free space slowly fragments over time.
IOWs, by making the ext4 extsize have forced alignment semantics by default, it means users will see ENOSPC at lot more frequently and in situations where it is most definitely not expected.
We also have to keep in mind that there are applications out there that set and use extent size hints, and so enabling extsize in ext4 will result in those applications silently starting to use them. If ext4 supporting extsize hints drastically changes the behaviour of the filesystem then that is going to cause significant unexpected regressions for users as they upgrade kernels and filesystems.
Hence I strongly suggest that ext4 implements extent size hints in the same way that XFS does. i.e. unless forced alignment has been enabled for the inode, extsize is just a hint that gets discarded if aligned allocation does not succeed.
Quoted 6 lines
Behaviour such as extent size hinting *should* be the same across all filesystems that provide this functionality. This makes using extent size hints much easier for users, admins and application developers. The last thing I want to hear is application devs tell me at conferences that "we don't use extent size hints anymore because ext4..."
Yes, makes sense :)
Nothing to worry here tho as ext4 also treats the extsize value as a hint exactly like XFS. We have tried to keep the behavior as similar to XFS as possible for the exact reasons you mentioned.
And yes, we do plan to add a forcealign (or similar) feature for ext4 as well for atomic writes which would change the hint to a mandate
Quoted 42 lines
Quoted 4 lines
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.
I'm not sure I understand what you are saying here. XFS does extsize alignment of both the start and end of post-eof extents the same as it does for extents within EOF. For example:
# xfs_io -fdc "truncate 0" -c "extsize 16k" -c "pwrite 0 4k" -c "bmap -vvp" foo wrote 4096/4096 bytes at offset 0 4 KiB, 1 ops; 0.0308 sec (129.815 KiB/sec and 32.4538 ops/sec) foo: EXT: FILE-OFFSET BLOCK-RANGE AG AG-OFFSET TOTAL FLAGS 0: [0..7]: 256504..256511 0 (256504..256511) 8 000000 1: [8..31]: 256512..256535 0 (256512..256535) 24 010000 FLAG Values: 0100000 Shared extent 0010000 Unwritten preallocated extentThere's a 4k written extent at 0, and a 12k unwritten extent beyond EOF at 4k. I.e. we have an extent of 16kB as the hint required that is correctly aligned beyond EOF.
If I then write another 4k at 20k (beyond both EOF and the unwritten extent beyond EOF:
# xfs_io -fdc "truncate 0" -c "extsize 16k" -c "pwrite 0 4k" -c "pwrite 20k 4k" -c "bmap -vvp" foo wrote 4096/4096 bytes at offset 0 4 KiB, 1 ops; 0.0210 sec (190.195 KiB/sec and 47.5489 ops/sec) wrote 4096/4096 bytes at offset 20480 4 KiB, 1 ops; 0.0001 sec (21.701 MiB/sec and 5555.5556 ops/sec) foo: EXT: FILE-OFFSET BLOCK-RANGE AG AG-OFFSET TOTAL FLAGS 0: [0..7]: 180000..180007 0 (180000..180007) 8 000000 1: [8..39]: 180008..180039 0 (180008..180039) 32 010000 2: [40..47]: 180040..180047 0 (180040..180047) 8 000000 3: [48..63]: 180048..180063 0 (180048..180063) 16 010000 FLAG Values: 0100000 Shared extent 0010000 Unwritten preallocated extent
Quoted 4 lines
You can see we did contiguous allocation of another 16kB at offset 16kB, and then wrote to 20k for 4kB.. i.e. the new extent was correctly aligned at both sides as the extsize hint says it should be....
Sorry for the confusion Dave. What was meant is that XFS would indeed respect extsize hint for EOF allocations but if we close the file, since we trim the blocks past EOF upon close, we would only see that the lstart is aligned but the end would not.
For example:
$ xfs_io -c "open -dft foo" -c "truncate 0" -c "extsize 16k" -c "pwrite 0 4k" -c "bmap -vvp" -c "close" -c "open foo" -c "bmap -vvp"
wrote 4096/4096 bytes at offset 0
4 KiB, 1 ops; 0.0003 sec (9.864 MiB/sec and 2525.2525 ops/sec)
foo:
EXT: FILE-OFFSET BLOCK-RANGE AG AG-OFFSET TOTAL FLAGS 0: [0..7]: 384..391 0 (384..391) 8 000000 1: [8..31]: 392..415 0 (392..415) 24 010000
FLAG Values:
0010000 Unwritten preallocated extent
foo:
EXT: FILE-OFFSET BLOCK-RANGE AG AG-OFFSET TOTAL FLAGS
0: [0..7]: 384..391 0 (384..391) 8 000000
Quoted 7 lines
Quoted 3 lines
However extsize hint in ext4 for eof allocation is not supported in this version of the series.
If you can't do extsize aligned allocations for EOF extension, then how to applications use atomic writes to atomically extend the file?
In this particular RFC we can't and we'll have to go via the 'set extsize hint and then truncate' route. But we do plan to add this in next revision.
Quoted 6 lines
Quoted 1 line
3. XFS allows extsize to be set on file with no extents but delayed data.
It does?
<looks>
Quoted 7 lines
Yep, it doesn't check
ip->i_delayed_blksis zero when changing extsize.I think that's simply a bug, not intended behaviour, because delalloc will not have reserved space for the extsize hint rounding needed when writeback occurs. Can you send a patch to add this check?
Got it, sure I can send a patch for this.
Quoted 8 lines
Quoted 3 lines
However, ext4 don't allow that for simplicity. The user is expected to set it on a file before changing it's
i_size.We don't actually care about
i_sizein XFS - the determining factor is whether there are extents allocated on disk. i.e. we can truncate up and then set the extent size hint because there are no extents allocated even though the size is non-zero.
That's right Dave, my intention was also to just make sure that before setting extsize:
1. We dont have dellayed allocs in flight
2. We dont have blocks allocated on disk
So ideally truncate followed by extsize set should have worked.
And in that sense, you are right that using i_size (or i_disksize in ext4) is not correct. I will fix this behavior in next revision, thanks.
Quoted 27 lines
There are almost certainly applications out there that change extent size after truncating to a non-zero size, so this needs to work on ext4 the same way it does on XFS. Otherwise people are going to complain that their applications suddenly stop working properly on ext4....
Quoted 2 lines
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.
Yes, ext4 can make that restriction if desired.
Keep in mind that the XFS atomic write support is still evolving, and I think the way we are using extent size hints isn't fully solidified yet.
Indeed, I think that we can allow non-power-of-2 extent sizes for atomic writes, because integer multiples of the atomic write unit will still ensure that physical extents are properly aligned for atomic writes to succeed. e.g. 24kB extent size is compatible with 8kB atomic write sizes.
To make that work efficiently unwritten extent boundaries need to be maintained at atomic write alignments (8kB), not extent size alignment (24kB), but other than that I don't think anything else is needed....
Quoted 5 lines
This is desirable because it will allow extent size hints to remain usable for their original purposes even with atomic writes on XFS. i.e. fragmentation minimisation for small random DIO write worklaods (exactly the sort of IO you'd consider using atomic writes for!), alignment of extents to [non-power-of-2] RAID stripe geometry, etc.
Got it, I agree that extsize doesn't **have** to be power-of-2 but for this revision we have kept it that way cause getting power of 2 aligned blocks comes almost without much changes in ext4 allocator.
However, it shouldn't be a problem to support non power-of-2 blocks. We already have some aligned allocation logic for RAID use case which can be leveraged.
Quoted 8 lines
Quoted 4 lines
5. In ext4 we chose to store the extsize value in
SYSTEM_XATTRrather than an inode field as it was simple and most flexible, since there might be more features like atomic/untorn writes coming in future.Does that mean you can query and set it through the user xattr interfaces? If so, how do you enforce the values users set are correct?
AFAICU, ext4 (and XFS) doesn't provide a handler for system xattrs, so its not possible for a user to get/set it from the xattr interface. They'd have to go through the ioctl.
$ getfattr -n system.extsize test test: system.extsize: Operation not supported
That being said, in case in future we would for some reason want to add a handler for system xattrs to ext4, we'll have to be mindful of making sure users can't get or set extsize through the xattr interfaces.
Quoted 11 lines
Quoted 3 lines
6. In buffered-io path XFS switches to non-delalloc allocations for extsize hint. The same has been kept for EXT4 as well.
That's an internal XFS implementation detail that you don't need to replicate. Historically speaking, we didn't use unwritten extents for delayed allocation and so we couldn't do within-EOF extsize unaligned writes without adding special additional zero-around code to ensure that we never exposed stale data to userspace from the extra allocation that the data write did not cover.
Quoted 4 lines
We now use unwritten extents for delalloc conversion, so this istale data exposure issue no longer exists. We should really switch this code back to using delalloc because it is much faster and less fragmentation prone than direct extsize allocation....
Thanks for the context Dave, I didn't implement it this time around as I wanted to be sure what challenges XFS faced and ext4 will face while trying extsize with delalloc. I think this clears things up and this can be added in the next revisions.
Quoted 5 lines
-Dave.
Signature · 2 lines
Dave Chinner [email protected]
Quoted 9 lines · Ojaswin Mujoo
Quoted 7 lines · Dave Chinner
On Wed, Sep 11, 2024 at 02:31:04PM +0530, Ojaswin Mujoo wrote:
Behaviour such as extent size hinting *should* be the same across all filesystems that provide this functionality. This makes using extent size hints much easier for users, admins and application developers. The last thing I want to hear is application devs tell me at conferences that "we don't use extent size hints anymore because ext4..."
Quoted 5 lines
Yes, makes sense :)
Nothing to worry here tho as ext4 also treats the extsize value as a hint exactly like XFS. We have tried to keep the behavior as similar to XFS as possible for the exact reasons you mentioned.
It is worth explicitly stating this (i.e. all the behaviours that are the same) in the design documentation rather than just the corner cases where it is different. It was certainly not clear how failures were treated.
Quoted 2 lines
And yes, we do plan to add a forcealign (or similar) feature for ext4 as well for atomic writes which would change the hint to a mandate
Ok. That should be stated, too.
FWIW, it would be a good idea to document this all in the kernel documentation itself, so there is a guideline for other filesystems to implement the same behaviour. e.g. in Documentation/filesystems/extent-size-hints.rst
Quoted 46 lines
Quoted 41 lines
Quoted 3 lines
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.
I'm not sure I understand what you are saying here. XFS does extsize alignment of both the start and end of post-eof extents the same as it does for extents within EOF. For example:
# xfs_io -fdc "truncate 0" -c "extsize 16k" -c "pwrite 0 4k" -c "bmap -vvp" foo wrote 4096/4096 bytes at offset 0 4 KiB, 1 ops; 0.0308 sec (129.815 KiB/sec and 32.4538 ops/sec) foo: EXT: FILE-OFFSET BLOCK-RANGE AG AG-OFFSET TOTAL FLAGS 0: [0..7]: 256504..256511 0 (256504..256511) 8 000000 1: [8..31]: 256512..256535 0 (256512..256535) 24 010000 FLAG Values: 0100000 Shared extent 0010000 Unwritten preallocated extentThere's a 4k written extent at 0, and a 12k unwritten extent beyond EOF at 4k. I.e. we have an extent of 16kB as the hint required that is correctly aligned beyond EOF.
If I then write another 4k at 20k (beyond both EOF and the unwritten extent beyond EOF:
# xfs_io -fdc "truncate 0" -c "extsize 16k" -c "pwrite 0 4k" -c "pwrite 20k 4k" -c "bmap -vvp" foo wrote 4096/4096 bytes at offset 0 4 KiB, 1 ops; 0.0210 sec (190.195 KiB/sec and 47.5489 ops/sec) wrote 4096/4096 bytes at offset 20480 4 KiB, 1 ops; 0.0001 sec (21.701 MiB/sec and 5555.5556 ops/sec) foo: EXT: FILE-OFFSET BLOCK-RANGE AG AG-OFFSET TOTAL FLAGS 0: [0..7]: 180000..180007 0 (180000..180007) 8 000000 1: [8..39]: 180008..180039 0 (180008..180039) 32 010000 2: [40..47]: 180040..180047 0 (180040..180047) 8 000000 3: [48..63]: 180048..180063 0 (180048..180063) 16 010000 FLAG Values: 0100000 Shared extent 0010000 Unwritten preallocated extentQuoted 4 lines
You can see we did contiguous allocation of another 16kB at offset 16kB, and then wrote to 20k for 4kB.. i.e. the new extent was correctly aligned at both sides as the extsize hint says it should be....
Quoted 4 lines
Sorry for the confusion Dave. What was meant is that XFS would indeed respect extsize hint for EOF allocations but if we close the file, since we trim the blocks past EOF upon close, we would only see that the lstart is aligned but the end would not.
Right, but that is desired behaviour, especially when extsize is large. i.e. when the file is closed it is an indication that the file will not be written again, so we don't need to keep post-eof blocks around for fragmentation prevention reasons.
Removing post-EOF extents on close prevents large extsize hints from consuming lots of unused space on files that are never going to be written to again(*). That's user visible, and because it can cause premature ENOSPC, users will report this excessive space usage behaviour as a bug (and they are right). Hence removing post-eof extents on file close when extent size hints are in use comes under the guise of Good Behaviour To Have.
(*) think about how much space is wasted if you clone a kernel git tree under a 1MB extent size hint directory. All those tiny header files now take up 1MB of space on disk....
Keep in mind that when the file is opened for write again, the extent size hint still gets applied to the new extents. If the extending write starts beyond the EOF extsize range, then the new extent after the hole at EOF will be fully extsize aligned, as expected.
If the new write is exactly extending the file, then the new extents will not be extsize aligned - the start will be at the EOF block, and they will be extsize -length-. IOWs, the extent size is maintained, just the logical alignment is not exactly extsize aligned. This could be considered a bug, but it's never been an issue for anyone because, in XFS, physical extent alignment is separate (and maintained regardless of logical alignment) for extent size hint based allocations.
Adding force-align will prevent this behaviour from occurring, as post-eof trimming will be done to extsize alignment, not to the EOF block. Hence open/close/open will not affect logical or physical alignment of force-align extents (and hence won't affect atomic writes).
-Dave.
Signature · 2 lines
Dave Chinner [email protected]
Ojaswin Mujoo
reviewerQuoted 12 lines · Dave Chinner
Quoted 11 lines · Ojaswin Mujoo
Quoted 7 lines · Dave Chinner
On Wed, Sep 11, 2024 at 02:31:04PM +0530, Ojaswin Mujoo wrote:
Behaviour such as extent size hinting *should* be the same across all filesystems that provide this functionality. This makes using extent size hints much easier for users, admins and application developers. The last thing I want to hear is application devs tell me at conferences that "we don't use extent size hints anymore because ext4..."Yes, makes sense :)
Quoted 8 lines
Quoted 3 lines
Nothing to worry here tho as ext4 also treats the extsize value as a hint exactly like XFS. We have tried to keep the behavior as similar to XFS as possible for the exact reasons you mentioned.
It is worth explicitly stating this (i.e. all the behaviours that are the same) in the design documentation rather than just the corner cases where it is different. It was certainly not clear how failures were treated.
Got it Dave, I did mention it in the actual commit 5/5 but I agree. I will update the cover letter to be more clear about the design in future revisions.
Quoted 10 lines
Quoted 3 lines
And yes, we do plan to add a forcealign (or similar) feature for ext4 as well for atomic writes which would change the hint to a mandate
Ok. That should be stated, too.
FWIW, it would be a good idea to document this all in the kernel documentation itself, so there is a guideline for other filesystems to implement the same behaviour. e.g. in
Documentation/filesystems/extent-size-hints.rst
Okay makes sense, I can look into this as a next step.
Quoted 84 lines
Quoted 47 lines
Quoted 42 lines
Quoted 4 lines
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.
I'm not sure I understand what you are saying here. XFS does extsize alignment of both the start and end of post-eof extents the same as it does for extents within EOF. For example:
# xfs_io -fdc "truncate 0" -c "extsize 16k" -c "pwrite 0 4k" -c "bmap -vvp" foo wrote 4096/4096 bytes at offset 0 4 KiB, 1 ops; 0.0308 sec (129.815 KiB/sec and 32.4538 ops/sec) foo: EXT: FILE-OFFSET BLOCK-RANGE AG AG-OFFSET TOTAL FLAGS 0: [0..7]: 256504..256511 0 (256504..256511) 8 000000 1: [8..31]: 256512..256535 0 (256512..256535) 24 010000 FLAG Values: 0100000 Shared extent 0010000 Unwritten preallocated extentThere's a 4k written extent at 0, and a 12k unwritten extent beyond EOF at 4k. I.e. we have an extent of 16kB as the hint required that is correctly aligned beyond EOF.
If I then write another 4k at 20k (beyond both EOF and the unwritten extent beyond EOF:
# xfs_io -fdc "truncate 0" -c "extsize 16k" -c "pwrite 0 4k" -c "pwrite 20k 4k" -c "bmap -vvp" foo wrote 4096/4096 bytes at offset 0 4 KiB, 1 ops; 0.0210 sec (190.195 KiB/sec and 47.5489 ops/sec) wrote 4096/4096 bytes at offset 20480 4 KiB, 1 ops; 0.0001 sec (21.701 MiB/sec and 5555.5556 ops/sec) foo: EXT: FILE-OFFSET BLOCK-RANGE AG AG-OFFSET TOTAL FLAGS 0: [0..7]: 180000..180007 0 (180000..180007) 8 000000 1: [8..39]: 180008..180039 0 (180008..180039) 32 010000 2: [40..47]: 180040..180047 0 (180040..180047) 8 000000 3: [48..63]: 180048..180063 0 (180048..180063) 16 010000 FLAG Values: 0100000 Shared extent 0010000 Unwritten preallocated extentQuoted 4 lines
You can see we did contiguous allocation of another 16kB at offset 16kB, and then wrote to 20k for 4kB.. i.e. the new extent was correctly aligned at both sides as the extsize hint says it should be....
Quoted 4 lines
Sorry for the confusion Dave. What was meant is that XFS would indeed respect extsize hint for EOF allocations but if we close the file, since we trim the blocks past EOF upon close, we would only see that the lstart is aligned but the end would not.
Right, but that is desired behaviour, especially when extsize is large. i.e. when the file is closed it is an indication that the file will not be written again, so we don't need to keep post-eof blocks around for fragmentation prevention reasons.
Removing post-EOF extents on close prevents large extsize hints from consuming lots of unused space on files that are never going to be written to
again(*). That's user visible, and because it can cause premature ENOSPC, users will report this excessive space usage behaviour as a bug (and they are right). Hence removing post-eof extents on file close when extent size hints are in use comes under the guise of Good Behaviour To Have.(*) think about how much space is wasted if you clone a kernel git tree under a 1MB extent size hint directory. All those tiny header files now take up 1MB of space on disk....
Keep in mind that when the file is opened for write again, the extent size hint still gets applied to the new extents. If the extending write starts beyond the EOF extsize range, then the new extent after the hole at EOF will be fully extsize aligned, as expected.
If the new write is exactly extending the file, then the new extents will not be extsize aligned - the start will be at the EOF block, and they will be extsize -length-. IOWs, the extent size is maintained, just the logical alignment is not exactly extsize aligned. This could be considered a bug, but it's never been an issue for anyone because, in XFS, physical extent alignment is separate (and maintained regardless of logical alignment) for extent size hint based allocations.
Quoted 5 lines
Adding force-align will prevent this behaviour from occurring, as post-eof trimming will be done to extsize alignment, not to the EOF block. Hence open/close/open will not affect logical or physical alignment of force-align extents (and hence won't affect atomic writes).
Thanks for the context, I will try to keep this behavior similar to XFS once we implement the EOF support for extsize hints in next revision.
Regards,
Ojaswin
Quoted 5 lines
-Dave.
Signature · 2 lines
Dave Chinner [email protected]