linux-ext4 - Re: [PATCH v3] ext4: Add periodic superblock update check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20230828114843.lkaiusxm27gw6iwc@quack3>
Date:   Mon, 28 Aug 2023 13:48:43 +0200
From:   Jan Kara <jack@...e.cz>
To:     vk.en.mail@...il.com
Cc:     linux-ext4@...r.kernel.org, tytso@....edu, jack@...e.cz,
        adilger@...ger.ca
Subject: Re: [PATCH v3] ext4: Add periodic superblock update check

On Sat 26-08-23 23:58:41, vk.en.mail@...il.com wrote:
> From: Vitaliy Kuznetsov <vk.en.mail@...il.com>
> 
> This patch introduces a mechanism to periodically check and update
> the superblock within the ext4 file system. The main purpose of this
> patch is to keep the disk superblock up to date. The update will be
> performed if more than one hour has passed since the last update, and
> if more than 16MB of data have been written to disk.
> 
> This check and update is performed within the ext4_journal_commit_callback
> function, ensuring that the superblock is written while the disk is
> active, rather than based on a timer that may trigger during disk idle
> periods.
> 
> Discussion https://www.spinics.net/lists/linux-ext4/msg85865.html
> 
> Signed-off-by: Vitaliy Kuznetsov <vk.en.mail@...il.com>

Looks good to me. Feel free to add:

Reviewed-by: Jan Kara <jack@...e.cz>

								Honza

> ---
>  fs/ext4/super.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 61 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index c94ebf704616..8bee05118c7a 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -433,6 +433,57 @@ 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.
> + *
> + * 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.
> + *
> + * @sb: The superblock
> + */
> +static void ext4_maybe_update_superblock(struct super_block *sb)
> +{
> +	struct ext4_sb_info *sbi = EXT4_SB(sb);
> +	struct ext4_super_block *es = sbi->s_es;
> +	journal_t *journal = sbi->s_journal;
> +	time64_t now;
> +	__u64 last_update;
> +	__u64 lifetime_write_kbytes;
> +	__u64 diff_size;
> +
> +	if (sb_rdonly(sb) || !(sb->s_flags & SB_ACTIVE) ||
> +	    !journal || (journal->j_flags & JBD2_UNMOUNT))
> +		return;
> +
> +	now = ktime_get_real_seconds();
> +	last_update = ext4_get_tstamp(es, s_wtime);
> +
> +	if (likely(now - last_update < EXT4_SB_REFRESH_INTERVAL_SEC))
> +		return;
> +
> +	lifetime_write_kbytes = sbi->s_kbytes_written +
> +		((part_stat_read(sb->s_bdev, sectors[STAT_WRITE]) -
> +		  sbi->s_sectors_written_start) >> 1);
> +
> +	/* Get the number of kilobytes not written to disk to account
> +	 * for statistics and compare with a multiple of 16 MB. This
> +	 * is used to determine when the next superblock commit should
> +	 * occur (i.e. not more often than once per 16MB if there was
> +	 * less written in an hour).
> +	 */
> +	diff_size = lifetime_write_kbytes - le64_to_cpu(es->s_kbytes_written);
> +
> +	if (diff_size > EXT4_SB_REFRESH_INTERVAL_KB)
> +		schedule_work(&EXT4_SB(sb)->s_error_work);
> +}
> +
>  /*
>   * The del_gendisk() function uninitializes the disk-specific data
>   * structures, including the bdi structure, without telling anyone
> @@ -459,6 +510,7 @@ static void ext4_journal_commit_callback(journal_t *journal, transaction_t *txn)
>  	BUG_ON(txn->t_state == T_FINISHED);
> 
>  	ext4_process_freed_data(sb, txn->t_tid);
> +	ext4_maybe_update_superblock(sb);
> 
>  	spin_lock(&sbi->s_md_lock);
>  	while (!list_empty(&txn->t_private_list)) {
> @@ -715,6 +767,7 @@ static void flush_stashed_error_work(struct work_struct *work)
>  	 */
>  	if (!sb_rdonly(sbi->s_sb) && journal) {
>  		struct buffer_head *sbh = sbi->s_sbh;
> +		bool call_notify_err = false;
>  		handle = jbd2_journal_start(journal, 1);
>  		if (IS_ERR(handle))
>  			goto write_directly;
> @@ -722,6 +775,10 @@ static void flush_stashed_error_work(struct work_struct *work)
>  			jbd2_journal_stop(handle);
>  			goto write_directly;
>  		}
> +
> +		if (sbi->s_add_error_count > 0)
> +			call_notify_err = true;
> +
>  		ext4_update_super(sbi->s_sb);
>  		if (buffer_write_io_error(sbh) || !buffer_uptodate(sbh)) {
>  			ext4_msg(sbi->s_sb, KERN_ERR, "previous I/O error to "
> @@ -735,7 +792,10 @@ static void flush_stashed_error_work(struct work_struct *work)
>  			goto write_directly;
>  		}
>  		jbd2_journal_stop(handle);
> -		ext4_notify_error_sysfs(sbi);
> +
> +		if (call_notify_err)
> +			ext4_notify_error_sysfs(sbi);
> +
>  		return;
>  	}
>  write_directly:
> --
> 2.39.2
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR