| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <889a9ea4-bfdc-430d-95a9-e4b02b0f3625@kernel.org>
Date: Wed, 19 Nov 2025 14:46:07 +0800
From: Chao Yu <chao@...nel.org>
To: YH Lin <yhli@...gle.com>, jaegeuk@...nel.org,
linux-f2fs-devel@...ts.sourceforge.net, linux-kernel@...r.kernel.org
Cc: chao@...nel.org
Subject: Re: [PATCH] f2fs: optimize trace_f2fs_write_checkpoint with enums
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@...gle.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,
Powered by blists - more mailing lists