Linux-ext4 Archive on lore.kernel.org
 help / color / Atom feed
From: Ritesh Harjani <riteshh@linux.ibm.com>
To: Harshad Shirwadkar <harshadshirwadkar@gmail.com>,
	linux-ext4@vger.kernel.org
Cc: tytso@mit.edu
Subject: Re: [PATCH v9 4/9] jbd2: add fast commit machinery
Date: Fri, 9 Oct 2020 21:46:47 +0530
Message-ID: <96e9210e-f0d1-eaf6-593b-7fae982a8df2@linux.ibm.com> (raw)
In-Reply-To: <20200919005451.3899779-5-harshadshirwadkar@gmail.com>



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@gmail.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
>    *
> 

  reply index

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-19  0:54 [PATCH v9 0/9] ext4: add fast commits feature Harshad Shirwadkar
2020-09-19  0:54 ` [PATCH v9 1/9] doc: update ext4 and journalling docs to include fast commit feature Harshad Shirwadkar
2020-09-22 17:50   ` Darrick J. Wong
2020-09-24  6:56     ` harshad shirwadkar
2020-10-09 18:28   ` Theodore Y. Ts'o
2020-10-13  0:27     ` harshad shirwadkar
2020-09-19  0:54 ` [PATCH v9 2/9] ext4: add fast_commit feature and handling for extended mount options Harshad Shirwadkar
2020-10-09 17:58   ` Theodore Y. Ts'o
2020-10-13  0:27     ` harshad shirwadkar
2020-09-19  0:54 ` [PATCH v9 3/9] ext4 / jbd2: add fast commit initialization Harshad Shirwadkar
2020-09-19 15:22   ` kernel test robot
2020-10-09 16:10   ` Ritesh Harjani
2020-10-13  0:28     ` harshad shirwadkar
2020-09-19  0:54 ` [PATCH v9 4/9] jbd2: add fast commit machinery Harshad Shirwadkar
2020-10-09 16:16   ` Ritesh Harjani [this message]
2020-10-13  0:27     ` harshad shirwadkar
2020-09-19  0:54 ` [PATCH v9 5/9] ext4: main fast-commit commit path Harshad Shirwadkar
2020-09-19  8:19   ` kernel test robot
2020-10-09 17:04   ` Ritesh Harjani
2020-10-13  0:25     ` harshad shirwadkar
2020-10-09 19:14   ` Theodore Y. Ts'o
2020-10-13  0:27     ` harshad shirwadkar
2020-09-19  0:54 ` [PATCH v9 6/9] jbd2: fast commit recovery path Harshad Shirwadkar
2020-09-19  0:54 ` [PATCH v9 7/9] ext4: " Harshad Shirwadkar
2020-09-19 14:15   ` kernel test robot
2020-10-09 17:14   ` Ritesh Harjani
2020-10-13  0:27     ` harshad shirwadkar
2020-09-19  0:54 ` [PATCH v9 8/9] ext4: add a mount opt to forcefully turn fast commits on Harshad Shirwadkar
2020-09-19  0:54 ` [PATCH v9 9/9] ext4: add fast commit stats in procfs Harshad Shirwadkar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=96e9210e-f0d1-eaf6-593b-7fae982a8df2@linux.ibm.com \
    --to=riteshh@linux.ibm.com \
    --cc=harshadshirwadkar@gmail.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-ext4 Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-ext4/0 linux-ext4/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-ext4 linux-ext4/ https://lore.kernel.org/linux-ext4 \
		linux-ext4@vger.kernel.org
	public-inbox-index linux-ext4

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-ext4


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git