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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 9 Oct 2020 21:46:47 +0530
From:   Ritesh Harjani <riteshh@...ux.ibm.com>
To:     Harshad Shirwadkar <harshadshirwadkar@...il.com>,
        linux-ext4@...r.kernel.org
Cc:     tytso@....edu
Subject: Re: [PATCH v9 4/9] jbd2: add fast commit machinery



On 9/19/20 6:24 AM, Harshad Shirwadkar wrote:
> This patch implements following APIs in JBD2 to allow for fast
> commits:
> 
> jbd2_fc_start(): Start a new fast commit. This function waits for any
> existing fast commit or full commit to complete.
> 
> jbd2_fc_stop(): Stop fast commit. This function ends current fast
> commit and wakes up either the journal thread or the other fast commit
> waiting for current fast commit to complete.
> 
> jbd2_fc_stop_do_commit(): Stop fast commit and perform a full
> commit. This is same as above but also performs a full commit.
> 
> This patch also adds a cleanup handler in journal_t that is called
> after every full and fast commit.
> 
> Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@...il.com>
> ---
>   fs/ext4/fast_commit.c |  8 ++++++
>   fs/jbd2/commit.c      | 19 ++++++++++++
>   fs/jbd2/journal.c     | 67 +++++++++++++++++++++++++++++++++++++++++++
>   include/linux/jbd2.h  | 21 ++++++++++++++
>   4 files changed, 115 insertions(+)
> 
> diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
> index 0dad8bdb1253..f2d11b4c6b62 100644
> --- a/fs/ext4/fast_commit.c
> +++ b/fs/ext4/fast_commit.c
> @@ -8,11 +8,19 @@
>    * Ext4 fast commits routines.
>    */
>   #include "ext4_jbd2.h"
> +/*
> + * Fast commit cleanup routine. This is called after every fast commit and
> + * full commit. full is true if we are called after a full commit.
> + */
> +static void ext4_fc_cleanup(journal_t *journal, int full)
> +{
> +}
> 
>   void ext4_fc_init(struct super_block *sb, journal_t *journal)
>   {
>   	if (!test_opt2(sb, JOURNAL_FAST_COMMIT))
>   		return;
> +	journal->j_fc_cleanup_callback = ext4_fc_cleanup;
>   	if (jbd2_fc_init(journal, EXT4_NUM_FC_BLKS)) {
>   		pr_warn("Error while enabling fast commits, turning off.");
>   		ext4_clear_feature_fast_commit(sb);
> diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
> index 6d2da8ad0e6f..ba35ecb18616 100644
> --- a/fs/jbd2/commit.c
> +++ b/fs/jbd2/commit.c
> @@ -413,6 +413,20 @@ void jbd2_journal_commit_transaction(journal_t *journal)
>   	J_ASSERT(journal->j_running_transaction != NULL);
>   	J_ASSERT(journal->j_committing_transaction == NULL);
> 
> +	write_lock(&journal->j_state_lock);
> +	journal->j_flags |= JBD2_FULL_COMMIT_ONGOING;

Shouldn't we set this flag only after the while loop ends and before 
releasing the write lock()? Like how we are doing in jbd2_fc_start()?


> +	while (journal->j_flags & JBD2_FAST_COMMIT_ONGOING) {
> +		DEFINE_WAIT(wait);
> +
> +		prepare_to_wait(&journal->j_wait_fc, &wait,
> +				TASK_UNINTERRUPTIBLE);
> +		write_unlock(&journal->j_state_lock);
> +		schedule();
> +		write_lock(&journal->j_state_lock);
> +		finish_wait(&journal->j_wait_fc, &wait);
> +	}
> +	write_unlock(&journal->j_state_lock);
> +
>   	commit_transaction = journal->j_running_transaction;
> 
>   	trace_jbd2_start_commit(journal, commit_transaction);
> @@ -1119,12 +1133,16 @@ void jbd2_journal_commit_transaction(journal_t *journal)
> 
>   	if (journal->j_commit_callback)
>   		journal->j_commit_callback(journal, commit_transaction);
> +	if (journal->j_fc_cleanup_callback)
> +		journal->j_fc_cleanup_callback(journal, 1);
> 
>   	trace_jbd2_end_commit(journal, commit_transaction);
>   	jbd_debug(1, "JBD2: commit %d complete, head %d\n",
>   		  journal->j_commit_sequence, journal->j_tail_sequence);
> 
>   	write_lock(&journal->j_state_lock);
> +	journal->j_flags &= ~JBD2_FULL_COMMIT_ONGOING;
> +	journal->j_flags &= ~JBD2_FAST_COMMIT_ONGOING;
>   	spin_lock(&journal->j_list_lock);
>   	commit_transaction->t_state = T_FINISHED;
>   	/* Check if the transaction can be dropped now that we are finished */
> @@ -1136,6 +1154,7 @@ void jbd2_journal_commit_transaction(journal_t *journal)
>   	spin_unlock(&journal->j_list_lock);
>   	write_unlock(&journal->j_state_lock);
>   	wake_up(&journal->j_wait_done_commit);
> +	wake_up(&journal->j_wait_fc);
> 
>   	/*
>   	 * Calculate overall stats
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 736a1736619f..17a30a2c38f9 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -714,6 +714,72 @@ int jbd2_log_wait_commit(journal_t *journal, tid_t tid)
>   	return err;
>   }
> 
> +/*
> + * Start a fast commit. If there's an ongoing fast or full commit wait for
> + * it to complete. Returns 0 if a new fast commit was started. Returns -EALREADY
> + * if a fast commit is not needed, either because there's an already a commit
> + * going on or this tid has already been committed. Returns -EINVAL if no jbd2
> + * commit has yet been performed.
> + */
> +int jbd2_fc_start(journal_t *journal, tid_t tid)
> +{
> +	/*
> +	 * Fast commits only allowed if at least one full commit has
> +	 * been processed.
> +	 */
> +	if (!journal->j_stats.ts_tid)
> +		return -EINVAL;
> +
> +	if (tid <= journal->j_commit_sequence)
> +		return -EALREADY;
> +
> +	write_lock(&journal->j_state_lock);
> +	if (journal->j_flags & JBD2_FULL_COMMIT_ONGOING ||
> +	    (journal->j_flags & JBD2_FAST_COMMIT_ONGOING)) {
> +		DEFINE_WAIT(wait);
> +
> +		prepare_to_wait(&journal->j_wait_fc, &wait,
> +				TASK_UNINTERRUPTIBLE);
> +		write_unlock(&journal->j_state_lock);
> +		schedule();
> +		finish_wait(&journal->j_wait_fc, &wait);
> +		return -EALREADY;
> +	}
> +	journal->j_flags |= JBD2_FAST_COMMIT_ONGOING;
> +	write_unlock(&journal->j_state_lock);
> +
> +	return 0;
> +}
> +
> +/*
> + * Stop a fast commit. If fallback is set, this function starts commit of
> + * TID tid before any other fast commit can start.
> + */
> +static int __jbd2_fc_stop(journal_t *journal, tid_t tid, bool fallback)
> +{
> +	if (journal->j_fc_cleanup_callback)
> +		journal->j_fc_cleanup_callback(journal, 0);
> +	write_lock(&journal->j_state_lock);
> +	journal->j_flags &= ~JBD2_FAST_COMMIT_ONGOING;
> +	if (fallback)
> +		journal->j_flags |= JBD2_FULL_COMMIT_ONGOING;
> +	write_unlock(&journal->j_state_lock);
> +	wake_up(&journal->j_wait_fc);
> +	if (fallback)
> +		return jbd2_complete_transaction(journal, tid);
> +	return 0;
> +}
> +
> +int jbd2_fc_stop(journal_t *journal)
> +{
> +	return __jbd2_fc_stop(journal, 0, 0);
> +}
> +
> +int jbd2_fc_stop_do_commit(journal_t *journal, tid_t tid)
> +{
> +	return __jbd2_fc_stop(journal, tid, 1);
> +}
> +
>   /* Return 1 when transaction with given tid has already committed. */
>   int jbd2_transaction_committed(journal_t *journal, tid_t tid)
>   {
> @@ -1140,6 +1206,7 @@ static journal_t *journal_init_common(struct block_device *bdev,
>   	init_waitqueue_head(&journal->j_wait_commit);
>   	init_waitqueue_head(&journal->j_wait_updates);
>   	init_waitqueue_head(&journal->j_wait_reserved);
> +	init_waitqueue_head(&journal->j_wait_fc);
>   	mutex_init(&journal->j_abort_mutex);
>   	mutex_init(&journal->j_barrier);
>   	mutex_init(&journal->j_checkpoint_mutex);
> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
> index 36f65a818366..aad986a9f3ff 100644
> --- a/include/linux/jbd2.h
> +++ b/include/linux/jbd2.h
> @@ -858,6 +858,13 @@ struct journal_s
>   	 */
>   	wait_queue_head_t	j_wait_reserved;
> 
> +	/**
> +	 * @j_wait_fc:
> +	 *
> +	 * Wait queue to wait for completion of async fast commits.
> +	 */
> +	wait_queue_head_t	j_wait_fc;

If we follow the naming convention then j_fc_wait, will be more
convenient.

> +
>   	/**
>   	 * @j_checkpoint_mutex:
>   	 *
> @@ -1208,6 +1215,15 @@ struct journal_s
>   	 */
>   	struct lockdep_map	j_trans_commit_map;
>   #endif
> +
> +	/**
> +	 * @j_fc_cleanup_callback:
> +	 *
> +	 * Clean-up after fast commit or full commit. JBD2 calls this function
> +	 * after every commit operation.
> +	 */
> +	void (*j_fc_cleanup_callback)(struct journal_s *journal, int);
> +
>   };
> 
>   #define jbd2_might_wait_for_commit(j) \
> @@ -1292,6 +1308,8 @@ JBD2_FEATURE_INCOMPAT_FUNCS(fast_commit,	FAST_COMMIT)
>   #define JBD2_ABORT_ON_SYNCDATA_ERR	0x040	/* Abort the journal on file
>   						 * data write error in ordered
>   						 * mode */
> +#define JBD2_FAST_COMMIT_ONGOING	0x100	/* Fast commit is ongoing */
> +#define JBD2_FULL_COMMIT_ONGOING	0x200	/* Full commit is ongoing */
> 
>   /*
>    * Function declarations for the journaling transaction and buffer
> @@ -1546,6 +1564,9 @@ extern int jbd2_cleanup_journal_tail(journal_t *);
> 
>   /* Fast commit related APIs */
>   int jbd2_fc_init(journal_t *journal, int num_fc_blks);
> +int jbd2_fc_start(journal_t *journal, tid_t tid);
> +int jbd2_fc_stop(journal_t *journal);
> +int jbd2_fc_stop_do_commit(journal_t *journal, tid_t tid);
>   /*
>    * is_journal_abort
>    *
> 

Powered by blists - more mailing lists