Re: [PATCH v2 3/3] ext4: Make sb update interval tunable

[Baokun Li <libaokun1@huawei.com>]

On 2025/3/6 22:28, Ojaswin Mujoo wrote:
> Currently, outside error paths, we auto commit the super block after 1
> hour has passed and 16MB worth of updates have been written since last
> commit. This is a policy decision so make this tunable while keeping the
> defaults same. This is useful if user wants to tweak the superblock
> behavior or for debugging the codepath by allowing to trigger it more
> frequently.
>
> We can now tweak the super block update using sb_update_sec and
> sb_update_kb files in /sys/fs/ext4/<dev>/
>
> Reviewed-by: Jan Kara <jack@suse.cz>
> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
Looks good.

Reviewed-by: Baokun Li <libaokun1@huawei.com>
> ---
>   fs/ext4/ext4.h  |  9 +++++++++
>   fs/ext4/super.c | 15 ++++++++-------
>   fs/ext4/sysfs.c |  4 ++++
>   3 files changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index d48e93bd5690..82c902ed86f3 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1608,6 +1608,8 @@ struct ext4_sb_info {
>   	unsigned int s_mb_prefetch;
>   	unsigned int s_mb_prefetch_limit;
>   	unsigned int s_mb_best_avail_max_trim_order;
> +	unsigned int s_sb_update_sec;
> +	unsigned int s_sb_update_kb;
>   
>   	/* stats for buddy allocator */
>   	atomic_t s_bal_reqs;	/* number of reqs with len > 1 */
> @@ -2281,6 +2283,13 @@ static inline int ext4_forced_shutdown(struct super_block *sb)
>   #define EXT4_DEF_MIN_BATCH_TIME	0
>   #define EXT4_DEF_MAX_BATCH_TIME	15000 /* 15ms */
>   
> +/*
> + * Default values for superblock update
> + */
> +#define EXT4_DEF_SB_UPDATE_INTERVAL_SEC (3600) /* seconds (1 hour) */
> +#define EXT4_DEF_SB_UPDATE_INTERVAL_KB (16384) /* kilobytes (16MB) */
> +
> +
>   /*
>    * Minimum number of groups in a flexgroup before we separate out
>    * directories into the first block group of a flexgroup
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 31552cf0519a..1b47b111c583 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -447,9 +447,6 @@ static time64_t __ext4_get_tstamp(__le32 *lo, __u8 *hi)
>   #define ext4_get_tstamp(es, tstamp) \
>   	__ext4_get_tstamp(&(es)->tstamp, &(es)->tstamp ## _hi)
>   
> -#define EXT4_SB_REFRESH_INTERVAL_SEC (3600) /* seconds (1 hour) */
> -#define EXT4_SB_REFRESH_INTERVAL_KB (16384) /* kilobytes (16MB) */
> -
>   /*
>    * The ext4_maybe_update_superblock() function checks and updates the
>    * superblock if needed.
> @@ -457,8 +454,10 @@ static time64_t __ext4_get_tstamp(__le32 *lo, __u8 *hi)
>    * This function is designed to update the on-disk superblock only under
>    * certain conditions to prevent excessive disk writes and unnecessary
>    * waking of the disk from sleep. The superblock will be updated if:
> - * 1. More than an hour has passed since the last superblock update, and
> - * 2. More than 16MB have been written since the last superblock update.
> + * 1. More than sbi->s_sb_update_sec (def: 1 hour) has passed since the last
> + *    superblock update
> + * 2. More than sbi->s_sb_update_kb (def: 16MB) kbs have been written since the
> + *    last superblock update.
>    *
>    * @sb: The superblock
>    */
> @@ -479,7 +478,7 @@ static void ext4_maybe_update_superblock(struct super_block *sb)
>   	now = ktime_get_real_seconds();
>   	last_update = ext4_get_tstamp(es, s_wtime);
>   
> -	if (likely(now - last_update < EXT4_SB_REFRESH_INTERVAL_SEC))
> +	if (likely(now - last_update < sbi->s_sb_update_sec))
>   		return;
>   
>   	lifetime_write_kbytes = sbi->s_kbytes_written +
> @@ -494,7 +493,7 @@ static void ext4_maybe_update_superblock(struct super_block *sb)
>   	 */
>   	diff_size = lifetime_write_kbytes - le64_to_cpu(es->s_kbytes_written);
>   
> -	if (diff_size > EXT4_SB_REFRESH_INTERVAL_KB)
> +	if (diff_size > sbi->s_sb_update_kb)
>   		schedule_work(&EXT4_SB(sb)->s_sb_upd_work);
>   }
>   
> @@ -5246,6 +5245,8 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
>   	sbi->s_commit_interval = JBD2_DEFAULT_MAX_COMMIT_AGE * HZ;
>   	sbi->s_min_batch_time = EXT4_DEF_MIN_BATCH_TIME;
>   	sbi->s_max_batch_time = EXT4_DEF_MAX_BATCH_TIME;
> +	sbi->s_sb_update_kb = EXT4_DEF_SB_UPDATE_INTERVAL_KB;
> +	sbi->s_sb_update_sec = EXT4_DEF_SB_UPDATE_INTERVAL_SEC;
>   
>   	/*
>   	 * set default s_li_wait_mult for lazyinit, for the case there is
> diff --git a/fs/ext4/sysfs.c b/fs/ext4/sysfs.c
> index ddb54608ca2e..987bd00f916a 100644
> --- a/fs/ext4/sysfs.c
> +++ b/fs/ext4/sysfs.c
> @@ -254,6 +254,8 @@ EXT4_ATTR(journal_task, 0444, journal_task);
>   EXT4_RW_ATTR_SBI_UI(mb_prefetch, s_mb_prefetch);
>   EXT4_RW_ATTR_SBI_UI(mb_prefetch_limit, s_mb_prefetch_limit);
>   EXT4_RW_ATTR_SBI_UL(last_trim_minblks, s_last_trim_minblks);
> +EXT4_RW_ATTR_SBI_UI(sb_update_sec, s_sb_update_sec);
> +EXT4_RW_ATTR_SBI_UI(sb_update_kb, s_sb_update_kb);
>   
>   static unsigned int old_bump_val = 128;
>   EXT4_ATTR_PTR(max_writeback_mb_bump, 0444, pointer_ui, &old_bump_val);
> @@ -305,6 +307,8 @@ static struct attribute *ext4_attrs[] = {
>   	ATTR_LIST(mb_prefetch),
>   	ATTR_LIST(mb_prefetch_limit),
>   	ATTR_LIST(last_trim_minblks),
> +	ATTR_LIST(sb_update_sec),
> +	ATTR_LIST(sb_update_kb),
>   	NULL,
>   };
>   ATTRIBUTE_GROUPS(ext4);