Re: [PATCH v2 5/5] ext4: update code documentation

[Jan Kara <jack@suse.cz>]

On Tue 08-03-22 08:33:19, Harshad Shirwadkar wrote:
> From: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
> 
> This patch updates code documentation to reflect the commit path changes
> made in this series.
> 
> Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>

Looks good. Feel free to add:

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

								Honza

> ---
>  fs/ext4/fast_commit.c | 36 ++++++++++++++++++++++--------------
>  1 file changed, 22 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
> index eea19e3ea9ba..c14e6d34d552 100644
> --- a/fs/ext4/fast_commit.c
> +++ b/fs/ext4/fast_commit.c
> @@ -49,14 +49,21 @@
>   * that need to be committed during a fast commit in another in memory queue of
>   * inodes. During the commit operation, we commit in the following order:
>   *
> - * [1] Lock inodes for any further data updates by setting COMMITTING state
> - * [2] Submit data buffers of all the inodes
> - * [3] Wait for [2] to complete
> - * [4] Commit all the directory entry updates in the fast commit space
> - * [5] Commit all the changed inode structures
> - * [6] Write tail tag (this tag ensures the atomicity, please read the following
> + * [1] Lock the journal by calling jbd2_journal_lock_updates. This ensures that
> + *     all the exsiting handles finish and no new handles can start.
> + * [2] Mark all the fast commit eligible inodes as undergoing fast commit
> + *     by setting "EXT4_STATE_FC_COMMITTING" state.
> + * [3] Unlock the journal by calling jbd2_journal_unlock_updates. This allows
> + *     starting of new handles. If new handles try to start an update on
> + *     any of the inodes that are being committed, ext4_fc_track_inode()
> + *     will block until those inodes have finished the fast commit.
> + * [4] Submit data buffers of all the committing inodes.
> + * [5] Wait for [4] to complete.
> + * [6] Commit all the directory entry updates in the fast commit space.
> + * [7] Commit all the changed inodes in the fast commit space and clear
> + *     "EXT4_STATE_FC_COMMITTING" for these inodes.
> + * [8] Write tail tag (this tag ensures the atomicity, please read the following
>   *     section for more details).
> - * [7] Wait for [4], [5] and [6] to complete.
>   *
>   * All the inode updates must call ext4_fc_start_update() before starting an
>   * update. If such an ongoing update is present, fast commit waits for it to
> @@ -142,6 +149,13 @@
>   * similarly. Thus, by converting a non-idempotent procedure into a series of
>   * idempotent outcomes, fast commits ensured idempotence during the replay.
>   *
> + * Locking
> + * -------
> + * sbi->s_fc_lock protects the fast commit inodes queue and the fast commit
> + * dentry queue. ei->i_fc_lock protects the fast commit related info in a given
> + * inode. Most of the code avoids acquiring both the locks, but if one must do
> + * that then sbi->s_fc_lock must be acquired before ei->i_fc_lock.
> + *
>   * TODOs
>   * -----
>   *
> @@ -156,13 +170,7 @@
>   *    fast commit recovery even if that area is invalidated by later full
>   *    commits.
>   *
> - * 1) Fast commit's commit path locks the entire file system during fast
> - *    commit. This has significant performance penalty. Instead of that, we
> - *    should use ext4_fc_start/stop_update functions to start inode level
> - *    updates from ext4_journal_start/stop. Once we do that we can drop file
> - *    system locking during commit path.
> - *
> - * 2) Handle more ineligible cases.
> + * 1) Handle more ineligible cases.
>   */
>  
>  #include <trace/events/ext4.h>
> -- 
> 2.35.1.616.g0bdcbb4464-goog
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR