From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 725662D9484 for ; Wed, 19 Nov 2025 06:46:13 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763534773; cv=none; b=GKz6IP0NA1J5EQTd8dz8gsZbB3SiIpIEAktHJr4a4Rzb6vQXfoSDkLNyllf8HkWbAqo3NiCk0kG++X/HGes1O0xIc4opy9bWDF2cqcGZAvswYWwTLnK+XlVdQl8Y83rwwlE+jc7BxfLzyCXI8/0O0pl4hqIibuf7GBm1JN2o0v8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763534773; c=relaxed/simple; bh=p2fE6wVS9TbTJkA8qgux4UegzFHrWuo8BAPcmvjWX5A=; h=Message-ID:Date:MIME-Version:Cc:Subject:To:References:From: In-Reply-To:Content-Type; b=EN/4I/SdZDF71ZYNj37Wxvvur8queom/IolnxylOI0EIsT0rMOehbKLhCYdF2nqR5rS+x4mwUVIKrbyzh3Dra7dACESV0KK9ObAaNpC1mK4/RaG+65ujg6NxNwVgN/Sa6k/pfp+G+e/I3idZm3WhRePIn+Kpl/ur8MzR6/shWLE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=klPpQ9mG; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="klPpQ9mG" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 00E31C16AAE; Wed, 19 Nov 2025 06:46:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1763534772; bh=p2fE6wVS9TbTJkA8qgux4UegzFHrWuo8BAPcmvjWX5A=; h=Date:Cc:Subject:To:References:From:In-Reply-To:From; b=klPpQ9mGVWPb3p+Bv6txZ7X0JNz0//HJYKaM4Vhq6He4pw1rQMLj4VuU//hMVtt7h NJopwO+VhpluIVmv2Ty2m8K1bOuqTsV67Ar+OHOblc9y3jRJv0iCUDEM89ILBUEr2e i3RPfbtIIxvt38yMpZzmxn9gS0/Ij7G/f5dELKdBn1NaADnyr8INTv1nrkqALRCHdc uvUfcGlfL9/yFhR65CxsDz8WkihsURa8hVeLbvDubSKnmuTK/gPYmYrpTFIBPhnLUm 82/TCmjH22xuAB4f8PNtpJdk/M/CutN1KEnmXASOoekd73wvIey5zHbh80Be3QofPv SqTVLUQ5i9UUQ== Message-ID: <889a9ea4-bfdc-430d-95a9-e4b02b0f3625@kernel.org> Date: Wed, 19 Nov 2025 14:46:07 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Cc: chao@kernel.org Subject: Re: [PATCH] f2fs: optimize trace_f2fs_write_checkpoint with enums To: YH Lin , jaegeuk@kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org References: <20251118062132.678025-1-yhli@google.com> Content-Language: en-US From: Chao Yu In-Reply-To: <20251118062132.678025-1-yhli@google.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit 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 > --- > 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,