Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 9 Mar 2022 11:19:47 +0100
From:   Jan Kara <jack@suse.cz>
To:     Harshad Shirwadkar <harshadshirwadkar@gmail.com>
Cc:     linux-ext4@vger.kernel.org, riteshh@linux.ibm.com, jack@suse.cz,
        tytso@mit.edu
Subject: Re: [PATCH v2 5/5] ext4: update code documentation
Message-ID: <20220309101947.66pcot5qhxvtydha@quack3.lan>
References: <20220308163319.1183625-1-harshads@google.com>
 <20220308163319.1183625-6-harshads@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220308163319.1183625-6-harshads@google.com>
Precedence: bulk

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