[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20251008101720.6c68c5cd@gandalf.local.home>
Date: Wed, 8 Oct 2025 10:17:20 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Jakub Acs <acsjakub@...zon.de>
Cc: <aliceryhl@...gle.com>, <djwong@...nel.org>, <jhubbard@...dia.com>,
<akpm@...ux-foundation.org>, <axelrasmussen@...gle.com>,
<chengming.zhou@...ux.dev>, <david@...hat.com>,
<linux-fsdevel@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<linux-mm@...ck.org>, <peterx@...hat.com>,
<rust-for-linux@...r.kernel.org>, <xu.xin16@....com.cn>
Subject: Re: [PATCH] mm: use enum for vm_flags
On Wed, 8 Oct 2025 12:54:27 +0000
Jakub Acs <acsjakub@...zon.de> wrote:
> Hi Alice,
>
> thanks for the patch, I squashed it in (should I add your signed-off-by
> too?) and added the TRACE_DEFINE_ENUM calls pointed out by Derrick.
>
> I have the following points to still address, though:
>
> - can the fact that we're not controlling the type of the values if
> using enum be a problem? (likely the indirect control we have through
> the highest value is good enough, but I'm not sure)
>
> - where do TRACE_DEFINE_ENUM calls belong?
It's probably best to put them in include/trace/events/mmflags.h
> I see them placed e.g. in include/trace/misc/nfs.h for nfs or
> arch/x86/kvm/mmu/mmutrace.h, but I don't see a corresponding file for
> mm.h - does this warrant creating a separate file for these
> definitions?
>
> - with the need for TRACE_DEFINE_ENUM calls, do we still deem this
> to be a good trade-off? - isn't fixing all of these in
> rust/bindings/bindings_helper.h better?
There's tricks to add a bunch of TRACE_DEFINE_ENUM()s at once. In fact,
look at how the macro TRACE_GFP_FLAGS_GENERAL is used in that mmflags.h
file.
The reason macros work but enums do not is because the pre-processor
converts macros to their original values but not enums. The TRACE_EVENT()
converts the "printf" part into a string. The macros are changes to their
values, but enums do not get changed.
The TRACE_DEFINE_ENUM() macro adds magic to map the name of the enum to its
value, and on boot up, the printf formats have the enums convert to the
values. Note, I'm working on having this performed at build time.
-- Steve
Powered by blists - more mailing lists