[PATCH] f2fs: optimize trace_f2fs_write_checkpoint with enums

YH Lin posted 1 patch 1 week, 6 days ago
There is a newer version of this series
fs/f2fs/checkpoint.c        |  6 +++---
include/trace/events/f2fs.h | 28 +++++++++++++++++++++++-----
2 files changed, 26 insertions(+), 8 deletions(-)
[PATCH] f2fs: optimize trace_f2fs_write_checkpoint with enums
Posted by YH Lin 1 week, 6 days ago
This patch optimizes the tracepoint by replacing these hardcoded strings
with a new enumeration f2fs_cp_phase.

1.Defines enum f2fs_cp_phase with values for each checkpoint phase.
2.Updates trace_f2fs_write_checkpoint to accept a u16 phase argument
instead of a string pointer.
3.Uses __print_symbolic in TP_printk to convert the enum values
back to their corresponding strings for human-readable trace output.

This change reduces the storage overhead for each trace event
by replacing a variable-length string with a 2-byte integer,
while maintaining the same readable output in ftrace.

Signed-off-by: YH Lin <yhli@google.com>
---
 fs/f2fs/checkpoint.c        |  6 +++---
 include/trace/events/f2fs.h | 28 +++++++++++++++++++++++-----
 2 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index bbe07e3a6c75..b04f82fdd143 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -1673,7 +1673,7 @@ int f2fs_write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 		goto out;
 	}
 
-	trace_f2fs_write_checkpoint(sbi->sb, cpc->reason, "start block_ops");
+	trace_f2fs_write_checkpoint(sbi->sb, cpc->reason, CP_PHASE_START_BLOCK_OPS);
 
 	err = block_operations(sbi);
 	if (err)
@@ -1681,7 +1681,7 @@ int f2fs_write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 
 	stat_cp_time(cpc, CP_TIME_OP_LOCK);
 
-	trace_f2fs_write_checkpoint(sbi->sb, cpc->reason, "finish block_ops");
+	trace_f2fs_write_checkpoint(sbi->sb, cpc->reason, CP_PHASE_FINISH_BLOCK_OPS);
 
 	f2fs_flush_merged_writes(sbi);
 
@@ -1747,7 +1747,7 @@ int f2fs_write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 
 	/* update CP_TIME to trigger checkpoint periodically */
 	f2fs_update_time(sbi, CP_TIME);
-	trace_f2fs_write_checkpoint(sbi->sb, cpc->reason, "finish checkpoint");
+	trace_f2fs_write_checkpoint(sbi->sb, cpc->reason, CP_PHASE_FINISH_CHECKPOINT);
 out:
 	if (cpc->reason != CP_RESIZE)
 		f2fs_up_write(&sbi->cp_global_sem);
diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
index e00611ead024..a87aab397457 100644
--- a/include/trace/events/f2fs.h
+++ b/include/trace/events/f2fs.h
@@ -11,6 +11,15 @@
 #define show_dev(dev)		MAJOR(dev), MINOR(dev)
 #define show_dev_ino(entry)	show_dev(entry->dev), (unsigned long)entry->ino
 
+#ifndef _TRACE_F2FS_ENUM_H
+#define _TRACE_F2FS_ENUM_H
+enum f2fs_cp_phase {
+	CP_PHASE_START_BLOCK_OPS,
+	CP_PHASE_FINISH_BLOCK_OPS,
+	CP_PHASE_FINISH_CHECKPOINT,
+};
+#endif /* _TRACE_F2FS_ENUM_H */
+
 TRACE_DEFINE_ENUM(NODE);
 TRACE_DEFINE_ENUM(DATA);
 TRACE_DEFINE_ENUM(META);
@@ -50,6 +59,9 @@ TRACE_DEFINE_ENUM(CP_PAUSE);
 TRACE_DEFINE_ENUM(CP_RESIZE);
 TRACE_DEFINE_ENUM(EX_READ);
 TRACE_DEFINE_ENUM(EX_BLOCK_AGE);
+TRACE_DEFINE_ENUM(CP_PHASE_START_BLOCK_OPS);
+TRACE_DEFINE_ENUM(CP_PHASE_FINISH_BLOCK_OPS);
+TRACE_DEFINE_ENUM(CP_PHASE_FINISH_CHECKPOINT);
 
 #define show_block_type(type)						\
 	__print_symbolic(type,						\
@@ -175,6 +187,12 @@ TRACE_DEFINE_ENUM(EX_BLOCK_AGE);
 #define S_ALL_PERM	(S_ISUID | S_ISGID | S_ISVTX |	\
 			S_IRWXU | S_IRWXG | S_IRWXO)
 
+#define show_cp_phase(phase)					\
+	__print_symbolic(phase,						\
+		{ CP_PHASE_START_BLOCK_OPS,		"start block_ops" },			\
+		{ CP_PHASE_FINISH_BLOCK_OPS,	"finish block_ops" },			\
+		{ CP_PHASE_FINISH_CHECKPOINT,	"finish checkpoint" })
+
 struct f2fs_sb_info;
 struct f2fs_io_info;
 struct extent_info;
@@ -1573,26 +1591,26 @@ TRACE_EVENT(f2fs_readpages,
 
 TRACE_EVENT(f2fs_write_checkpoint,
 
-	TP_PROTO(struct super_block *sb, int reason, const char *msg),
+	TP_PROTO(struct super_block *sb, int reason, u16 phase),
 
-	TP_ARGS(sb, reason, msg),
+	TP_ARGS(sb, reason, phase),
 
 	TP_STRUCT__entry(
 		__field(dev_t,	dev)
 		__field(int,	reason)
-		__string(dest_msg, msg)
+		__field(u16, phase)
 	),
 
 	TP_fast_assign(
 		__entry->dev		= sb->s_dev;
 		__entry->reason		= reason;
-		__assign_str(dest_msg);
+		__entry->phase		= phase;
 	),
 
 	TP_printk("dev = (%d,%d), checkpoint for %s, state = %s",
 		show_dev(__entry->dev),
 		show_cpreason(__entry->reason),
-		__get_str(dest_msg))
+		show_cp_phase(__entry->phase))
 );
 
 DECLARE_EVENT_CLASS(f2fs_discard,
-- 
2.52.0.rc1.455.g30608eb744-goog
Re: [PATCH] f2fs: optimize trace_f2fs_write_checkpoint with enums
Posted by Chao Yu 1 week, 5 days ago
On 11/18/25 14:21, YH Lin wrote:
> This patch optimizes the tracepoint by replacing these hardcoded strings
> with a new enumeration f2fs_cp_phase.
> 
> 1.Defines enum f2fs_cp_phase with values for each checkpoint phase.
> 2.Updates trace_f2fs_write_checkpoint to accept a u16 phase argument
> instead of a string pointer.
> 3.Uses __print_symbolic in TP_printk to convert the enum values
> back to their corresponding strings for human-readable trace output.
> 
> This change reduces the storage overhead for each trace event
> by replacing a variable-length string with a 2-byte integer,
> while maintaining the same readable output in ftrace.
> 
> Signed-off-by: YH Lin <yhli@google.com>
> ---
>  fs/f2fs/checkpoint.c        |  6 +++---
>  include/trace/events/f2fs.h | 28 +++++++++++++++++++++++-----
>  2 files changed, 26 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index bbe07e3a6c75..b04f82fdd143 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -1673,7 +1673,7 @@ int f2fs_write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>  		goto out;
>  	}
>  
> -	trace_f2fs_write_checkpoint(sbi->sb, cpc->reason, "start block_ops");
> +	trace_f2fs_write_checkpoint(sbi->sb, cpc->reason, CP_PHASE_START_BLOCK_OPS);
>  
>  	err = block_operations(sbi);
>  	if (err)
> @@ -1681,7 +1681,7 @@ int f2fs_write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>  
>  	stat_cp_time(cpc, CP_TIME_OP_LOCK);
>  
> -	trace_f2fs_write_checkpoint(sbi->sb, cpc->reason, "finish block_ops");
> +	trace_f2fs_write_checkpoint(sbi->sb, cpc->reason, CP_PHASE_FINISH_BLOCK_OPS);
>  
>  	f2fs_flush_merged_writes(sbi);
>  
> @@ -1747,7 +1747,7 @@ int f2fs_write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>  
>  	/* update CP_TIME to trigger checkpoint periodically */
>  	f2fs_update_time(sbi, CP_TIME);
> -	trace_f2fs_write_checkpoint(sbi->sb, cpc->reason, "finish checkpoint");
> +	trace_f2fs_write_checkpoint(sbi->sb, cpc->reason, CP_PHASE_FINISH_CHECKPOINT);
>  out:
>  	if (cpc->reason != CP_RESIZE)
>  		f2fs_up_write(&sbi->cp_global_sem);
> diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
> index e00611ead024..a87aab397457 100644
> --- a/include/trace/events/f2fs.h
> +++ b/include/trace/events/f2fs.h
> @@ -11,6 +11,15 @@
>  #define show_dev(dev)		MAJOR(dev), MINOR(dev)
>  #define show_dev_ino(entry)	show_dev(entry->dev), (unsigned long)entry->ino
>  
> +#ifndef _TRACE_F2FS_ENUM_H
> +#define _TRACE_F2FS_ENUM_H
> +enum f2fs_cp_phase {
> +	CP_PHASE_START_BLOCK_OPS,
> +	CP_PHASE_FINISH_BLOCK_OPS,
> +	CP_PHASE_FINISH_CHECKPOINT,
> +};

YH,

What do you think of relocating this enumeration to f2fs.h, and remove
_TRACE_F2FS_ENUM_H for cleanup? something like this?

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 5f104518c414..67c42aa27d32 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -319,6 +319,12 @@ struct cp_control {
        struct cp_stats stats;
 };

+enum f2fs_cp_phase {
+       CP_PHASE_START_BLOCK_OPS,
+       CP_PHASE_FINISH_BLOCK_OPS,
+       CP_PHASE_FINISH_CHECKPOINT,
+};
+

Thanks,

> +#endif /* _TRACE_F2FS_ENUM_H */
> +
>  TRACE_DEFINE_ENUM(NODE);
>  TRACE_DEFINE_ENUM(DATA);
>  TRACE_DEFINE_ENUM(META);
> @@ -50,6 +59,9 @@ TRACE_DEFINE_ENUM(CP_PAUSE);
>  TRACE_DEFINE_ENUM(CP_RESIZE);
>  TRACE_DEFINE_ENUM(EX_READ);
>  TRACE_DEFINE_ENUM(EX_BLOCK_AGE);
> +TRACE_DEFINE_ENUM(CP_PHASE_START_BLOCK_OPS);
> +TRACE_DEFINE_ENUM(CP_PHASE_FINISH_BLOCK_OPS);
> +TRACE_DEFINE_ENUM(CP_PHASE_FINISH_CHECKPOINT);
>  
>  #define show_block_type(type)						\
>  	__print_symbolic(type,						\
> @@ -175,6 +187,12 @@ TRACE_DEFINE_ENUM(EX_BLOCK_AGE);
>  #define S_ALL_PERM	(S_ISUID | S_ISGID | S_ISVTX |	\
>  			S_IRWXU | S_IRWXG | S_IRWXO)
>  
> +#define show_cp_phase(phase)					\
> +	__print_symbolic(phase,						\
> +		{ CP_PHASE_START_BLOCK_OPS,		"start block_ops" },			\
> +		{ CP_PHASE_FINISH_BLOCK_OPS,	"finish block_ops" },			\
> +		{ CP_PHASE_FINISH_CHECKPOINT,	"finish checkpoint" })
> +
>  struct f2fs_sb_info;
>  struct f2fs_io_info;
>  struct extent_info;
> @@ -1573,26 +1591,26 @@ TRACE_EVENT(f2fs_readpages,
>  
>  TRACE_EVENT(f2fs_write_checkpoint,
>  
> -	TP_PROTO(struct super_block *sb, int reason, const char *msg),
> +	TP_PROTO(struct super_block *sb, int reason, u16 phase),
>  
> -	TP_ARGS(sb, reason, msg),
> +	TP_ARGS(sb, reason, phase),
>  
>  	TP_STRUCT__entry(
>  		__field(dev_t,	dev)
>  		__field(int,	reason)
> -		__string(dest_msg, msg)
> +		__field(u16, phase)
>  	),
>  
>  	TP_fast_assign(
>  		__entry->dev		= sb->s_dev;
>  		__entry->reason		= reason;
> -		__assign_str(dest_msg);
> +		__entry->phase		= phase;
>  	),
>  
>  	TP_printk("dev = (%d,%d), checkpoint for %s, state = %s",
>  		show_dev(__entry->dev),
>  		show_cpreason(__entry->reason),
> -		__get_str(dest_msg))
> +		show_cp_phase(__entry->phase))
>  );
>  
>  DECLARE_EVENT_CLASS(f2fs_discard,