On 2023/6/12 20:40, Jinke Han wrote:
> From: Jinke Han <hanjinke.666@xxxxxxxxxxxxx>
>
> When releasing space in jbd, we traverse s_freed_data_list to get the
> free range belonging to the current commit transaction. In extreme cases,
> the time spent may not be small, and we have observed cases exceeding
> 10ms. This patch makes running and commit transactions manage their own
> free_data_list respectively, eliminating unnecessary traversal.
>
> And in the callback phase of the commit transaction, no one will touch
> it except the jbd thread itself, so s_md_lock is no longer needed.
>
> Signed-off-by: Jinke Han <hanjinke.666@xxxxxxxxxxxxx>
Thanks for the patch, it looks good to me.
Reviewed-by: Zhang Yi <yi.zhang@xxxxxxxxxx>
> ---
> fs/ext4/ext4.h | 2 +-
> fs/ext4/mballoc.c | 19 +++++--------------
> 2 files changed, 6 insertions(+), 15 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 813b4da098a0..356905357dc9 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1557,7 +1557,7 @@ struct ext4_sb_info {
> unsigned int *s_mb_maxs;
> unsigned int s_group_info_size;
> unsigned int s_mb_free_pending;
> - struct list_head s_freed_data_list; /* List of blocks to be freed
> + struct list_head s_freed_data_list[2]; /* List of blocks to be freed
> after commit completed */
> struct list_head s_discard_list;
> struct work_struct s_discard_work;
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 4f2a1df98141..8fab5720a979 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -3626,7 +3626,8 @@ int ext4_mb_init(struct super_block *sb)
>
> spin_lock_init(&sbi->s_md_lock);
> sbi->s_mb_free_pending = 0;
> - INIT_LIST_HEAD(&sbi->s_freed_data_list);
> + INIT_LIST_HEAD(&sbi->s_freed_data_list[0]);
> + INIT_LIST_HEAD(&sbi->s_freed_data_list[1]);
> INIT_LIST_HEAD(&sbi->s_discard_list);
> INIT_WORK(&sbi->s_discard_work, ext4_discard_work);
> atomic_set(&sbi->s_retry_alloc_pending, 0);
> @@ -3878,21 +3879,11 @@ void ext4_process_freed_data(struct super_block *sb, tid_t commit_tid)
> struct ext4_sb_info *sbi = EXT4_SB(sb);
> struct ext4_free_data *entry, *tmp;
> struct list_head freed_data_list;
> - struct list_head *cut_pos = NULL;
> + struct list_head *s_freed_head = &sbi->s_freed_data_list[commit_tid & 1];
> bool wake;
>
> INIT_LIST_HEAD(&freed_data_list);
> -
> - spin_lock(&sbi->s_md_lock);
> - list_for_each_entry(entry, &sbi->s_freed_data_list, efd_list) {
> - if (entry->efd_tid != commit_tid)
> - break;
> - cut_pos = &entry->efd_list;
> - }
> - if (cut_pos)
> - list_cut_position(&freed_data_list, &sbi->s_freed_data_list,
> - cut_pos);
> - spin_unlock(&sbi->s_md_lock);
> + list_replace_init(s_freed_head, &freed_data_list);
>
> list_for_each_entry(entry, &freed_data_list, efd_list)
> ext4_free_data_in_buddy(sb, entry);
> @@ -6298,7 +6289,7 @@ ext4_mb_free_metadata(handle_t *handle, struct ext4_buddy *e4b,
> }
>
> spin_lock(&sbi->s_md_lock);
> - list_add_tail(&new_entry->efd_list, &sbi->s_freed_data_list);
> + list_add_tail(&new_entry->efd_list, &sbi->s_freed_data_list[new_entry->efd_tid & 1]);
> sbi->s_mb_free_pending += clusters;
> spin_unlock(&sbi->s_md_lock);
> }
>
