Thanks for reporting this Steven and thanks Ritesh for fixing this.
Looks good.
Reviewed-by: Harshad Shirwadkar <harshadshirwadkar@xxxxxxxxx>
- Harshad
On Wed, 23 Feb 2022 at 01:54, Jan Kara <jack@xxxxxxx> wrote:
>
> On Wed 23-02-22 02:04:10, Ritesh Harjani wrote:
> > ftrace's __print_symbolic() requires that any enum values used in the
> > symbol to string translation table be wrapped in a TRACE_DEFINE_ENUM
> > so that the enum value can be encoded in the ftrace ring buffer.
> >
> > This patch also fixes few other problems found in this trace point.
> > e.g. dereferencing structures in TP_printk which should not be done
> > at any cost.
> >
> > Also to avoid checkpatch warnings, this patch removes those
> > whitespaces/tab stops issues.
> >
> > Fixes: commit aa75f4d3daae ("ext4: main fast-commit commit path")
> > Reported-by: Steven Rostedt <rostedt@xxxxxxxxxxx>
> > Signed-off-by: Ritesh Harjani <riteshh@xxxxxxxxxxxxx>
>
> Looks good (modulo Steven's nit). Feel free to add:
>
> Reviewed-by: Jan Kara <jack@xxxxxxx>
>
> Honza
>
>
> > ---
> > include/trace/events/ext4.h | 76 +++++++++++++++++++++++--------------
> > 1 file changed, 47 insertions(+), 29 deletions(-)
> >
> > diff --git a/include/trace/events/ext4.h b/include/trace/events/ext4.h
> > index 19e957b7f941..17fb9c506e8a 100644
> > --- a/include/trace/events/ext4.h
> > +++ b/include/trace/events/ext4.h
> > @@ -95,6 +95,16 @@ TRACE_DEFINE_ENUM(ES_REFERENCED_B);
> > { FALLOC_FL_COLLAPSE_RANGE, "COLLAPSE_RANGE"}, \
> > { FALLOC_FL_ZERO_RANGE, "ZERO_RANGE"})
> >
> > +TRACE_DEFINE_ENUM(EXT4_FC_REASON_XATTR);
> > +TRACE_DEFINE_ENUM(EXT4_FC_REASON_CROSS_RENAME);
> > +TRACE_DEFINE_ENUM(EXT4_FC_REASON_JOURNAL_FLAG_CHANGE);
> > +TRACE_DEFINE_ENUM(EXT4_FC_REASON_NOMEM);
> > +TRACE_DEFINE_ENUM(EXT4_FC_REASON_SWAP_BOOT);
> > +TRACE_DEFINE_ENUM(EXT4_FC_REASON_RESIZE);
> > +TRACE_DEFINE_ENUM(EXT4_FC_REASON_RENAME_DIR);
> > +TRACE_DEFINE_ENUM(EXT4_FC_REASON_FALLOC_RANGE);
> > +TRACE_DEFINE_ENUM(EXT4_FC_REASON_INODE_JOURNAL_DATA);
> > +
> > #define show_fc_reason(reason) \
> > __print_symbolic(reason, \
> > { EXT4_FC_REASON_XATTR, "XATTR"}, \
> > @@ -2723,41 +2733,49 @@ TRACE_EVENT(ext4_fc_commit_stop,
> >
> > #define FC_REASON_NAME_STAT(reason) \
> > show_fc_reason(reason), \
> > - __entry->sbi->s_fc_stats.fc_ineligible_reason_count[reason]
> > + __entry->fc_ineligible_rc[reason]
> >
> > TRACE_EVENT(ext4_fc_stats,
> > - TP_PROTO(struct super_block *sb),
> > -
> > - TP_ARGS(sb),
> > + TP_PROTO(struct super_block *sb),
> >
> > - TP_STRUCT__entry(
> > - __field(dev_t, dev)
> > - __field(struct ext4_sb_info *, sbi)
> > - __field(int, count)
> > - ),
> > + TP_ARGS(sb),
> >
> > - TP_fast_assign(
> > - __entry->dev = sb->s_dev;
> > - __entry->sbi = EXT4_SB(sb);
> > - ),
> > + TP_STRUCT__entry(
> > + __field(dev_t, dev)
> > + __array(unsigned int, fc_ineligible_rc, EXT4_FC_REASON_MAX)
> > + __field(unsigned long, fc_commits)
> > + __field(unsigned long, fc_ineligible_commits)
> > + __field(unsigned long, fc_numblks)
> > + ),
> >
> > - TP_printk("dev %d:%d fc ineligible reasons:\n"
> > - "%s:%d, %s:%d, %s:%d, %s:%d, %s:%d, %s:%d, %s:%d, %s:%d, %s:%d; "
> > - "num_commits:%ld, ineligible: %ld, numblks: %ld",
> > - MAJOR(__entry->dev), MINOR(__entry->dev),
> > - FC_REASON_NAME_STAT(EXT4_FC_REASON_XATTR),
> > - FC_REASON_NAME_STAT(EXT4_FC_REASON_CROSS_RENAME),
> > - FC_REASON_NAME_STAT(EXT4_FC_REASON_JOURNAL_FLAG_CHANGE),
> > - FC_REASON_NAME_STAT(EXT4_FC_REASON_NOMEM),
> > - FC_REASON_NAME_STAT(EXT4_FC_REASON_SWAP_BOOT),
> > - FC_REASON_NAME_STAT(EXT4_FC_REASON_RESIZE),
> > - FC_REASON_NAME_STAT(EXT4_FC_REASON_RENAME_DIR),
> > - FC_REASON_NAME_STAT(EXT4_FC_REASON_FALLOC_RANGE),
> > - FC_REASON_NAME_STAT(EXT4_FC_REASON_INODE_JOURNAL_DATA),
> > - __entry->sbi->s_fc_stats.fc_num_commits,
> > - __entry->sbi->s_fc_stats.fc_ineligible_commits,
> > - __entry->sbi->s_fc_stats.fc_numblks)
> > + TP_fast_assign(
> > + int i;
> >
> > + __entry->dev = sb->s_dev;
> > + for (i = 0; i < EXT4_FC_REASON_MAX; i++)
> > + __entry->fc_ineligible_rc[i] =
> > + EXT4_SB(sb)->s_fc_stats.fc_ineligible_reason_count[i];
> > + __entry->fc_commits = EXT4_SB(sb)->s_fc_stats.fc_num_commits;
> > + __entry->fc_ineligible_commits =
> > + EXT4_SB(sb)->s_fc_stats.fc_ineligible_commits;
> > + __entry->fc_numblks = EXT4_SB(sb)->s_fc_stats.fc_numblks;
> > + ),
> > +
> > + TP_printk("dev %d,%d fc ineligible reasons:\n"
> > + "%s:%u, %s:%u, %s:%u, %s:%u, %s:%u, %s:%u, %s:%u, %s:%u, %s:%u "
> > + "num_commits:%lu, ineligible: %lu, numblks: %lu",
> > + MAJOR(__entry->dev), MINOR(__entry->dev),
> > + FC_REASON_NAME_STAT(EXT4_FC_REASON_XATTR),
> > + FC_REASON_NAME_STAT(EXT4_FC_REASON_CROSS_RENAME),
> > + FC_REASON_NAME_STAT(EXT4_FC_REASON_JOURNAL_FLAG_CHANGE),
> > + FC_REASON_NAME_STAT(EXT4_FC_REASON_NOMEM),
> > + FC_REASON_NAME_STAT(EXT4_FC_REASON_SWAP_BOOT),
> > + FC_REASON_NAME_STAT(EXT4_FC_REASON_RESIZE),
> > + FC_REASON_NAME_STAT(EXT4_FC_REASON_RENAME_DIR),
> > + FC_REASON_NAME_STAT(EXT4_FC_REASON_FALLOC_RANGE),
> > + FC_REASON_NAME_STAT(EXT4_FC_REASON_INODE_JOURNAL_DATA),
> > + __entry->fc_commits, __entry->fc_ineligible_commits,
> > + __entry->fc_numblks)
> > );
> >
> > #define DEFINE_TRACE_DENTRY_EVENT(__type) \
> > --
> > 2.31.1
> >
> --
> Jan Kara <jack@xxxxxxxx>
> SUSE Labs, CR
