| Message ID | 60daf324eec64f2be0b9ce0e240294d36411037c.1645558375.git.riteshh@linux.ibm.com |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | ext4: Improve FC trace events and discuss one FC failure | expand |
On Wed 23-02-22 02:04:13, Ritesh Harjani wrote: > This adds commit_tid argument in ext4_fc_update_stats() > so that we can add this information too in jbd_debug logs. > This is also required in a later patch to pass the commit_tid info in > ext4_fc_commit_start/stop() trace events. > > Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com> Looks good. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > fs/ext4/fast_commit.c | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c > index 7fb1eceef30c..ee32aac0cbbf 100644 > --- a/fs/ext4/fast_commit.c > +++ b/fs/ext4/fast_commit.c > @@ -1127,11 +1127,12 @@ static int ext4_fc_perform_commit(journal_t *journal) > } > > static void ext4_fc_update_stats(struct super_block *sb, int status, > - u64 commit_time, int nblks) > + u64 commit_time, int nblks, tid_t commit_tid) > { > struct ext4_fc_stats *stats = &EXT4_SB(sb)->s_fc_stats; > > - jbd_debug(1, "Fast commit ended with status = %d", status); > + jbd_debug(1, "Fast commit ended with status = %d for tid %u", > + status, commit_tid); > if (status == EXT4_FC_STATUS_OK) { > stats->fc_num_commits++; > stats->fc_numblks += nblks; > @@ -1181,14 +1182,16 @@ int ext4_fc_commit(journal_t *journal, tid_t commit_tid) > if (atomic_read(&sbi->s_fc_subtid) <= subtid && > commit_tid > journal->j_commit_sequence) > goto restart_fc; > - ext4_fc_update_stats(sb, EXT4_FC_STATUS_SKIPPED, 0, 0); > + ext4_fc_update_stats(sb, EXT4_FC_STATUS_SKIPPED, 0, 0, > + commit_tid); > return 0; > } else if (ret) { > /* > * Commit couldn't start. Just update stats and perform a > * full commit. > */ > - ext4_fc_update_stats(sb, EXT4_FC_STATUS_FAILED, 0, 0); > + ext4_fc_update_stats(sb, EXT4_FC_STATUS_FAILED, 0, 0, > + commit_tid); > return jbd2_complete_transaction(journal, commit_tid); > } > > @@ -1220,12 +1223,12 @@ int ext4_fc_commit(journal_t *journal, tid_t commit_tid) > * don't react too strongly to vast changes in the commit time > */ > commit_time = ktime_to_ns(ktime_sub(ktime_get(), start_time)); > - ext4_fc_update_stats(sb, status, commit_time, nblks); > + ext4_fc_update_stats(sb, status, commit_time, nblks, commit_tid); > return ret; > > fallback: > ret = jbd2_fc_end_commit_fallback(journal); > - ext4_fc_update_stats(sb, status, 0, 0); > + ext4_fc_update_stats(sb, status, 0, 0, commit_tid); > return ret; > } > > -- > 2.31.1 >
Looks good. Reviewed-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com> - Harshad On Wed, 23 Feb 2022 at 01:42, Jan Kara <jack@suse.cz> wrote: > > On Wed 23-02-22 02:04:13, Ritesh Harjani wrote: > > This adds commit_tid argument in ext4_fc_update_stats() > > so that we can add this information too in jbd_debug logs. > > This is also required in a later patch to pass the commit_tid info in > > ext4_fc_commit_start/stop() trace events. > > > > Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com> > > Looks good. Feel free to add: > > Reviewed-by: Jan Kara <jack@suse.cz> > > Honza > > > --- > > fs/ext4/fast_commit.c | 15 +++++++++------ > > 1 file changed, 9 insertions(+), 6 deletions(-) > > > > diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c > > index 7fb1eceef30c..ee32aac0cbbf 100644 > > --- a/fs/ext4/fast_commit.c > > +++ b/fs/ext4/fast_commit.c > > @@ -1127,11 +1127,12 @@ static int ext4_fc_perform_commit(journal_t *journal) > > } > > > > static void ext4_fc_update_stats(struct super_block *sb, int status, > > - u64 commit_time, int nblks) > > + u64 commit_time, int nblks, tid_t commit_tid) > > { > > struct ext4_fc_stats *stats = &EXT4_SB(sb)->s_fc_stats; > > > > - jbd_debug(1, "Fast commit ended with status = %d", status); > > + jbd_debug(1, "Fast commit ended with status = %d for tid %u", > > + status, commit_tid); > > if (status == EXT4_FC_STATUS_OK) { > > stats->fc_num_commits++; > > stats->fc_numblks += nblks; > > @@ -1181,14 +1182,16 @@ int ext4_fc_commit(journal_t *journal, tid_t commit_tid) > > if (atomic_read(&sbi->s_fc_subtid) <= subtid && > > commit_tid > journal->j_commit_sequence) > > goto restart_fc; > > - ext4_fc_update_stats(sb, EXT4_FC_STATUS_SKIPPED, 0, 0); > > + ext4_fc_update_stats(sb, EXT4_FC_STATUS_SKIPPED, 0, 0, > > + commit_tid); > > return 0; > > } else if (ret) { > > /* > > * Commit couldn't start. Just update stats and perform a > > * full commit. > > */ > > - ext4_fc_update_stats(sb, EXT4_FC_STATUS_FAILED, 0, 0); > > + ext4_fc_update_stats(sb, EXT4_FC_STATUS_FAILED, 0, 0, > > + commit_tid); > > return jbd2_complete_transaction(journal, commit_tid); > > } > > > > @@ -1220,12 +1223,12 @@ int ext4_fc_commit(journal_t *journal, tid_t commit_tid) > > * don't react too strongly to vast changes in the commit time > > */ > > commit_time = ktime_to_ns(ktime_sub(ktime_get(), start_time)); > > - ext4_fc_update_stats(sb, status, commit_time, nblks); > > + ext4_fc_update_stats(sb, status, commit_time, nblks, commit_tid); > > return ret; > > > > fallback: > > ret = jbd2_fc_end_commit_fallback(journal); > > - ext4_fc_update_stats(sb, status, 0, 0); > > + ext4_fc_update_stats(sb, status, 0, 0, commit_tid); > > return ret; > > } > > > > -- > > 2.31.1 > > > -- > Jan Kara <jack@suse.com> > SUSE Labs, CR
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c index 7fb1eceef30c..ee32aac0cbbf 100644 --- a/fs/ext4/fast_commit.c +++ b/fs/ext4/fast_commit.c @@ -1127,11 +1127,12 @@ static int ext4_fc_perform_commit(journal_t *journal) } static void ext4_fc_update_stats(struct super_block *sb, int status, - u64 commit_time, int nblks) + u64 commit_time, int nblks, tid_t commit_tid) { struct ext4_fc_stats *stats = &EXT4_SB(sb)->s_fc_stats; - jbd_debug(1, "Fast commit ended with status = %d", status); + jbd_debug(1, "Fast commit ended with status = %d for tid %u", + status, commit_tid); if (status == EXT4_FC_STATUS_OK) { stats->fc_num_commits++; stats->fc_numblks += nblks; @@ -1181,14 +1182,16 @@ int ext4_fc_commit(journal_t *journal, tid_t commit_tid) if (atomic_read(&sbi->s_fc_subtid) <= subtid && commit_tid > journal->j_commit_sequence) goto restart_fc; - ext4_fc_update_stats(sb, EXT4_FC_STATUS_SKIPPED, 0, 0); + ext4_fc_update_stats(sb, EXT4_FC_STATUS_SKIPPED, 0, 0, + commit_tid); return 0; } else if (ret) { /* * Commit couldn't start. Just update stats and perform a * full commit. */ - ext4_fc_update_stats(sb, EXT4_FC_STATUS_FAILED, 0, 0); + ext4_fc_update_stats(sb, EXT4_FC_STATUS_FAILED, 0, 0, + commit_tid); return jbd2_complete_transaction(journal, commit_tid); } @@ -1220,12 +1223,12 @@ int ext4_fc_commit(journal_t *journal, tid_t commit_tid) * don't react too strongly to vast changes in the commit time */ commit_time = ktime_to_ns(ktime_sub(ktime_get(), start_time)); - ext4_fc_update_stats(sb, status, commit_time, nblks); + ext4_fc_update_stats(sb, status, commit_time, nblks, commit_tid); return ret; fallback: ret = jbd2_fc_end_commit_fallback(journal); - ext4_fc_update_stats(sb, status, 0, 0); + ext4_fc_update_stats(sb, status, 0, 0, commit_tid); return ret; }
This adds commit_tid argument in ext4_fc_update_stats() so that we can add this information too in jbd_debug logs. This is also required in a later patch to pass the commit_tid info in ext4_fc_commit_start/stop() trace events. Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com> --- fs/ext4/fast_commit.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)