2022-03-09 00:02:50

by harshad shirwadkar

[permalink] [raw]
Subject: [PATCH v2 5/5] ext4: update code documentation

From: Harshad Shirwadkar <[email protected]>

This patch updates code documentation to reflect the commit path changes
made in this series.

Signed-off-by: Harshad Shirwadkar <[email protected]>
---
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


2022-03-09 10:58:39

by Jan Kara

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

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

Looks good. Feel free to add:

Reviewed-by: Jan Kara <[email protected]>

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 <[email protected]>
SUSE Labs, CR