| Message ID | 20220906152920.25584-4-jack@suse.cz |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ext4: Fix performance regression with mballoc | expand |
On 22/09/06 05:29PM, Jan Kara wrote: > Curently we don't use any preallocation when a file is already closed > when allocating blocks (from writeback code when converting delayed > allocation). However for small files, using locality group preallocation I had always wondered about this case. But yes for small files it completely make sense to enable preallocations for smaller files even though they are closed. > is actually desirable as that is not specific to a particular file. > Rather it is a method to pack small files together to reduce > fragmentation and for that the fact the file is closed is actually even > stronger hint the file would benefit from packing. So change the logic > to allow locality group preallocation in this case. > One thing which comes to mind is that we never discard lg preallocations. With this change we will always enable lg preallocations for small files. These preallocations will then be only discarded when some allocation request fails which will be retried after doing discard preallocations. Though it is not a problem, since any small file allocation will benefit out of these lgs. But it shouldn't be too large that it starts causing performance hits for large files. Not for this patch but something to remember maybe ^^^^. (While we are internally looking at preallocation space for few optimizations, above is something to keep a note of.) > Signed-off-by: Jan Kara <jack@suse.cz> Looks good to me. Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c index 6251b4a6cc63..af1e49c3603f 100644 --- a/fs/ext4/mballoc.c +++ b/fs/ext4/mballoc.c @@ -5195,6 +5195,7 @@ static void ext4_mb_group_or_file(struct ext4_allocation_context *ac) struct ext4_sb_info *sbi = EXT4_SB(ac->ac_sb); int bsbits = ac->ac_sb->s_blocksize_bits; loff_t size, isize; + bool inode_pa_eligible, group_pa_eligible; if (!(ac->ac_flags & EXT4_MB_HINT_DATA)) return; @@ -5202,25 +5203,27 @@ 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; + inode_pa_eligible = true; size = ac->ac_o_ex.fe_logical + EXT4_C2B(sbi, ac->ac_o_ex.fe_len); isize = (i_size_read(ac->ac_inode) + ac->ac_sb->s_blocksize - 1) >> bsbits; + /* No point in using inode preallocation for closed files */ if ((size == isize) && !ext4_fs_is_busy(sbi) && - !inode_is_open_for_write(ac->ac_inode)) { - ac->ac_flags |= EXT4_MB_HINT_NOPREALLOC; - return; - } + !inode_is_open_for_write(ac->ac_inode)) + inode_pa_eligible = false; - if (sbi->s_mb_group_prealloc <= 0) { - ac->ac_flags |= EXT4_MB_STREAM_ALLOC; - return; - } - - /* don't use group allocation for large files */ size = max(size, isize); - if (size > sbi->s_mb_stream_request) { - ac->ac_flags |= EXT4_MB_STREAM_ALLOC; + /* Don't use group allocation for large files */ + if (size > sbi->s_mb_stream_request) + group_pa_eligible = false; + + if (!group_pa_eligible) { + if (inode_pa_eligible) + ac->ac_flags |= EXT4_MB_STREAM_ALLOC; + else + ac->ac_flags |= EXT4_MB_HINT_NOPREALLOC; return; }
Curently we don't use any preallocation when a file is already closed when allocating blocks (from writeback code when converting delayed allocation). However for small files, using locality group preallocation is actually desirable as that is not specific to a particular file. Rather it is a method to pack small files together to reduce fragmentation and for that the fact the file is closed is actually even stronger hint the file would benefit from packing. So change the logic to allow locality group preallocation in this case. Signed-off-by: Jan Kara <jack@suse.cz> --- fs/ext4/mballoc.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-)