Re: [PATCH] mm: Convert memory block states (MEM_*) macros to enum

On Mon, Oct 27, 2025 at 10:29:15AM +0100, David Hildenbrand wrote:
> On 26.10.25 17:22, Israel Batista wrote:
> > The MEM_* constants indicating the state of the memory block are
> > currently defined as macros, meaning their definitions will be omitted
> > from the debuginfo on most kernel builds. This makes it harder for
> > debuggers to correctly map the block state at runtime, which can be
> > quite useful when analysing errors related to memory hot plugging and
> > unplugging with tools such as drgn and eBPF.
> > 
> > Converting the constants to an enum will ensure the correct information
> > is emitted by the compiler and available for the debugger, without needing
> > to hard-code them into the debugger and track their changes.
> > 
> > Signed-off-by: Israel Batista <[email protected]>
> > ---
> >   include/linux/memory.h | 16 +++++++++-------
> >   1 file changed, 9 insertions(+), 7 deletions(-)
> > 
> > diff --git a/include/linux/memory.h b/include/linux/memory.h
> > index ba1515160894..8feba3bfcd18 100644
> > --- a/include/linux/memory.h
> > +++ b/include/linux/memory.h
> > @@ -89,13 +89,15 @@ int arch_get_memory_phys_device(unsigned long start_pfn);
> >   unsigned long memory_block_size_bytes(void);
> >   int set_memory_block_size_order(unsigned int order);
> > -/* These states are exposed to userspace as text strings in sysfs */
> > -#define	MEM_ONLINE		(1<<0) /* exposed to userspace */
> > -#define	MEM_GOING_OFFLINE	(1<<1) /* exposed to userspace */
> > -#define	MEM_OFFLINE		(1<<2) /* exposed to userspace */
> > -#define	MEM_GOING_ONLINE	(1<<3)
> > -#define	MEM_CANCEL_ONLINE	(1<<4)
> > -#define	MEM_CANCEL_OFFLINE	(1<<5)
> > +enum mem_states {
> > +	/* These states are exposed to userspace as text strings in sysfs */
> > +	MEM_ONLINE		 = (1<<0), /* exposed to userspace */
> > +	MEM_GOING_OFFLINE	 = (1<<1), /* exposed to userspace */
> > +	MEM_OFFLINE		 = (1<<2), /* exposed to userspace */
> > +	MEM_GOING_ONLINE	 = (1<<3),
> > +	MEM_CANCEL_ONLINE	 = (1<<4),
> > +	MEM_CANCEL_OFFLINE	 = (1<<5),
> > +};
> >   struct memory_notify {
> >   	unsigned long start_pfn;
> 
> CCing Lorenzo, we recently had a discussion about such conversions.

Yeah, I've been asking people to send out these conversions as we
encounter them in drgn, but ONLY when the absence of a value in the
kernel debugging symbols causes actual problems for drgn. I want it to
be clear that we're not spamming these just to cause churn. This is an
unfortunate corner case of debug info that leaves us with no other
option.

> The states are mutually exclusive (so no flags), so I wonder if we can just
> drop the (1<< X) setting completely.

FWIW, putting my C standard committee hat on, there is nothing wrong
with combining flags in an enum. C11 is silent on the matter, but C23
made this explicit. Quoting 6.7.3.3, paragraph 16: "After possible
lvalue conversion a value of the enumerated type behaves the same as the
value with the underlying type, in particular with all aspects of
promotion, conversion, and arithmetic." Lorenzo may have been thinking
of the stricter rules in C++.

Of course, semantically, it makes more sense to use distinct values in
cases like this where the values are not actually flags.

> IIRC, these values are not exposed to
> user space, only the corresponding names are, see state_show().
> 
> 
> Won't the compiler now complain that e.g., kcore_callback() does snot handle
> all cases? (no default statement)

Only if the controlling expression of the switch statement actually has
the enum type. All existing code uses unsigned long, so the compiler
doesn't care.

> -- 
> Cheers
> 
> David / dhildenb
> 
>