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

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

[Israel Batista]

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 <linux@israelbatista.dev.br>
---
 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;
-- 
2.51.0

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

[David Hildenbrand]

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 <linux@israelbatista.dev.br>
> ---
>   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.

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

-- 
Cheers

David / dhildenb

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

[Omar Sandoval]

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 <linux@israelbatista.dev.br>
> > ---
> >   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
> 
> 

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

[David Hildenbrand]

On 27.10.25 19:15, Omar Sandoval wrote:
> 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 <linux@israelbatista.dev.br>
>>> ---
>>>    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.

Well, I think we use the opportunity to clean all this further up, so 
all good :)

> 
>> 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. 

Right.

> 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++.

Right, it was around semantics.

For example, instead of defining the flags as an enum, define the actual 
bits in an enum.

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

Right. And that's the case here, we just made it look like flags for no 
apparent reason.

So I hope we can clean this up in the same go (same patch or addon patch).

> 
>> 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.

Ah, indeed, inherited from the notifier_call implementation. Which 
brings us to another topic: likely we should see where we can then 
actually used this named enum.

struct memory_block -> state looks like *the* candidate ;)

-- 
Cheers

David / dhildenb

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

[Lorenzo Stoakes]

On Mon, Oct 27, 2025 at 11:15:35AM -0700, Omar Sandoval wrote:
> 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 <linux@israelbatista.dev.br>
> > > ---
> > >   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 {

Why are we naming a type we never use...

> > > +	/* 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),
> > > +};

If it has to be named, can we just expose the bits as an enum and the values as
BIT(...)?

> > >   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.

Right. That really sucks, but I like drgn so if reasonable I do want us to
make life easier there... :)

>
> > 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++.

I don't really understand the argument being made there.

That's just saying the enum behaves as if it's the underlying type? I'm not
arguing otherwise.

Consider:

enum some_name {
	X,
	Y
};

void some_func(enum some_name val)
{
	switch (val) {
	case X:
		...
	case Y:
		...
	}

	// compiler doesn't warn about missing cases.
}

This is already giving some sense as to the intuition that enums specify
all the values as declared specific enumeration values.

But intuitively, with the enum as a _named_ type, it's _weird_ for there to
be possible values for it that are not listed.

The problem here for me is the type being _named_.

If it's unnamed, then it doesn't really matter, it's just another way of
declaring the values.

If drgn needs it named, then just name bit values and use BIT(...).

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

enum's are kinda defined by being distinct values...

>
> > 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.

So why are we naming the type... does drgn require it?

Thanks, Lorenzo

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

[Omar Sandoval]

On Mon, Oct 27, 2025 at 07:46:43PM +0000, Lorenzo Stoakes wrote:
> On Mon, Oct 27, 2025 at 11:15:35AM -0700, Omar Sandoval wrote:
> > 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 <linux@israelbatista.dev.br>
> > > > ---
> > > >   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 {
> 
> Why are we naming a type we never use...

As David suggested, naming it means that we can then use it in a way
that enables compiler warnings and documents the code better.

> > > > +	/* 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),
> > > > +};
> 
> If it has to be named, can we just expose the bits as an enum and the values as
> BIT(...)?
> 
> > > >   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.
> 
> Right. That really sucks, but I like drgn so if reasonable I do want us to
> make life easier there... :)
> 
> >
> > > 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++.
> 
> I don't really understand the argument being made there.
> 
> That's just saying the enum behaves as if it's the underlying type? I'm not
> arguing otherwise.
> 
> Consider:
> 
> enum some_name {
> 	X,
> 	Y
> };
> 
> void some_func(enum some_name val)
> {
> 	switch (val) {
> 	case X:
> 		...
> 	case Y:
> 		...
> 	}
> 
> 	// compiler doesn't warn about missing cases.
> }
> 
> This is already giving some sense as to the intuition that enums specify
> all the values as declared specific enumeration values.
> 
> But intuitively, with the enum as a _named_ type, it's _weird_ for there to
> be possible values for it that are not listed.
> 
> The problem here for me is the type being _named_.
> 
> If it's unnamed, then it doesn't really matter, it's just another way of
> declaring the values.

I read
https://lore.kernel.org/all/809f552d-3282-4746-ba49-066d2bd8d44f@lucifer.local/
("it's not valid to have flag values as an enum") as a claim that this
was invalid at the language level, but it sounds like your objection is
more of a personal style preference. Which is totally fine, the MM
subsystem can have whatever rules it wants.

To play devil's advocate, using a named enum for flags makes it easy to
document what flags are used for a given field. I don't know about you,
but I'm often annoyed when I come across an undocumented `unsigned long
flags` and I have to track down what set of flags it uses. And switching
on a flags value doesn't make sense in the first place regardless of
whether it's a macro or enum, so I don't expect that concern to matter
much in practice.

> If drgn needs it named, then just name bit values and use BIT(...).
> 
> >
> > Of course, semantically, it makes more sense to use distinct values in
> > cases like this where the values are not actually flags.
> 
> enum's are kinda defined by being distinct values...
> 
> >
> > > 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.
> 
> So why are we naming the type... does drgn require it?

Nope, drgn can work with anonymous enum types just fine. As long as
we're all on the same page that naming it/not naming it and using bit
numbers/flag values is a style choice and not a language requirement,
I'm happy with whatever approach makes the maintainers happy.

> Thanks, Lorenzo

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

[David Hildenbrand]

> I read
> https://lore.kernel.org/all/809f552d-3282-4746-ba49-066d2bd8d44f@lucifer.local/
> ("it's not valid to have flag values as an enum") as a claim that this
> was invalid at the language level, but it sounds like your objection is
> more of a personal style preference. Which is totally fine, the MM
> subsystem can have whatever rules it wants.
> 
> To play devil's advocate, using a named enum for flags makes it easy to
> document what flags are used for a given field

A current best practice for flags is to use a __bitwise typedef. That 
way, sparse will be able to make sure that people are not using other 
random values.

We don't make use of that everywhere yet, of course. FOLL_ flags are 
still wrapped in an enum, for example.

-- 
Cheers

David / dhildenb

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

[Lorenzo Stoakes]

On Mon, Oct 27, 2025 at 04:34:29PM -0700, Omar Sandoval wrote:
> On Mon, Oct 27, 2025 at 07:46:43PM +0000, Lorenzo Stoakes wrote:
> > On Mon, Oct 27, 2025 at 11:15:35AM -0700, Omar Sandoval wrote:
> > > 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 <linux@israelbatista.dev.br>
> > > > > ---
> > > > >   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 {
> >
> > Why are we naming a type we never use...
>
> As David suggested, naming it means that we can then use it in a way
> that enables compiler warnings and documents the code better.

Which compiler warnings?

enum foo {
	X = 1UL << 40,
};

static void blah(enum foo foo)
{
}

...
blah(3);

Doesn't generate any.

Nor does W=1 or W=2.

Nor with clang, etiher.

The better solution is to do something like __bitwise with sparse.

Also re: type, I'm not sure it is all that safe, or you certainly lose
control somewhat as the size of the integer you're passing around is
'whatever fits the values'. Technically it should be restricted to a signed
integer (e.g. 32 bit) for C11 but I think gnu c11 is using some compiler
extension stuff to just make it adapt it to the correct sizing.

Anyway, this is all probably moot as I believe David suggested these
_aren't used as flags_.

In which case I'm fine with it.

I'm just confused as to why we're still debating the flag stuff?

>
> > > > > +	/* 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),
> > > > > +};
> >
> > If it has to be named, can we just expose the bits as an enum and the values as
> > BIT(...)?
> >
> > > > >   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.
> >
> > Right. That really sucks, but I like drgn so if reasonable I do want us to
> > make life easier there... :)
> >
> > >
> > > > 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++.
> >
> > I don't really understand the argument being made there.
> >
> > That's just saying the enum behaves as if it's the underlying type? I'm not
> > arguing otherwise.
> >
> > Consider:
> >
> > enum some_name {
> > 	X,
> > 	Y
> > };
> >
> > void some_func(enum some_name val)
> > {
> > 	switch (val) {
> > 	case X:
> > 		...
> > 	case Y:
> > 		...
> > 	}
> >
> > 	// compiler doesn't warn about missing cases.
> > }
> >
> > This is already giving some sense as to the intuition that enums specify
> > all the values as declared specific enumeration values.
> >
> > But intuitively, with the enum as a _named_ type, it's _weird_ for there to
> > be possible values for it that are not listed.
> >
> > The problem here for me is the type being _named_.
> >
> > If it's unnamed, then it doesn't really matter, it's just another way of
> > declaring the values.
>
> I read
> https://lore.kernel.org/all/809f552d-3282-4746-ba49-066d2bd8d44f@lucifer.local/
> ("it's not valid to have flag values as an enum") as a claim that this
> was invalid at the language level, but it sounds like your objection is
> more of a personal style preference. Which is totally fine, the MM
> subsystem can have whatever rules it wants.

Why are you ignoring the points I've made above and referring instead to
another reply of mine? I'm very confused.

Can we keep focused on the discussion here?

Also I'm not overly impressed witht the suggestion that this is a personal
style preference. In fact it's something _somebody else_ reviewed me on a
while ago telling me not to do this... it simply made me think, and
therefore I am making rational arguments in favour of it.

But another point if those arguments (that you don't seem to have responded
to) don't suffice - I don't believe (correct me if I'm wrong) that we,
anywhere in mm, use a _named_ enum type passed around as that type to
specify flags.

We _do_ use anonymous enum's. We do also (bizarrely) name enum types but
then don't use that type.

Again, if I've missed a case then let me know.

So this is (AFAICT) _mm's_ style preference. And again, the switch()
statement point is a strong one... the compiler certainly seems to think
it's an exhaustive list there.

I simply don't agree that your reference to the standard says what you
think it says, or rather as I said that it really makes a difference to the
points made. And linux uses C11 which as you say remains silent on
underlying types.

However it's gnu c11, and you only get a warning on large values in the
enum (which technically is limited to int apparently) if -pedantic is set
specifying a larger value.

The fact that the compiler will treat a switch statement of all enum values
as exhaustive is already a strong argument against this.

Note that you can in fact have type safety even with these kinds of values
using sparse, a pattern I've used many times myself, and am using in the
upcoming VMA flag series (with an anon enum of bit values).

>
> To play devil's advocate, using a named enum for flags makes it easy to
> document what flags are used for a given field. I don't know about you,
> but I'm often annoyed when I come across an undocumented `unsigned long
> flags` and I have to track down what set of flags it uses. And switching
> on a flags value doesn't make sense in the first place regardless of
> whether it's a macro or enum, so I don't expect that concern to matter
> much in practice.

Yes agreed I prefer a named type, and it's irritating to track down
possible values.

But you can achieve this with sparse without having to put the values in an
enum for flags.

>
> > If drgn needs it named, then just name bit values and use BIT(...).
> >
> > >
> > > Of course, semantically, it makes more sense to use distinct values in
> > > cases like this where the values are not actually flags.
> >
> > enum's are kinda defined by being distinct values...
> >
> > >
> > > > 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.
> >
> > So why are we naming the type... does drgn require it?
>
> Nope, drgn can work with anonymous enum types just fine. As long as
> we're all on the same page that naming it/not naming it and using bit
> numbers/flag values is a style choice and not a language requirement,
> I'm happy with whatever approach makes the maintainers happy.

We're not on the same page. But I'm again confused as to why you're
labouring this point in a situation where it's not relevant afaict.

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

[Omar Sandoval]

On Tue, Oct 28, 2025 at 04:33:39PM +0000, Lorenzo Stoakes wrote:
> On Mon, Oct 27, 2025 at 04:34:29PM -0700, Omar Sandoval wrote:
> > On Mon, Oct 27, 2025 at 07:46:43PM +0000, Lorenzo Stoakes wrote:
> > > On Mon, Oct 27, 2025 at 11:15:35AM -0700, Omar Sandoval wrote:
> > > > 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 <linux@israelbatista.dev.br>
> > > > > > ---
> > > > > >   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 {
> > >
> > > Why are we naming a type we never use...
> >
> > As David suggested, naming it means that we can then use it in a way
> > that enables compiler warnings and documents the code better.
> 
> Which compiler warnings?

I was referring to a situation that David suggested where:

1. enum memory_block_state contains distinct values, not flags, and
2. We convert struct memory_block->state to enum memory_block_state.

Then, everywhere that does switch (memory_block->state) would get
warnings for forgotten cases. Maybe desirable, maybe not, but those are
the warnings I was referring to.

> enum foo {
> 	X = 1UL << 40,
> };
> 
> static void blah(enum foo foo)
> {
> }
> 
> ...
> blah(3);
> 
> Doesn't generate any.
> 
> Nor does W=1 or W=2.
> 
> Nor with clang, etiher.
> 
> The better solution is to do something like __bitwise with sparse.
> 
> Also re: type, I'm not sure it is all that safe, or you certainly lose
> control somewhat as the size of the integer you're passing around is
> 'whatever fits the values'. Technically it should be restricted to a signed
> integer (e.g. 32 bit) for C11 but I think gnu c11 is using some compiler
> extension stuff to just make it adapt it to the correct sizing.
> 
> Anyway, this is all probably moot as I believe David suggested these
> _aren't used as flags_.
> 
> In which case I'm fine with it.
> 
> I'm just confused as to why we're still debating the flag stuff?

Right, it's irrelevant in this case. I mainly wanted to know for the
future what MM's preference was for cases that really do need flags. My
bad for not making that clear. It sounds like the answer is a __bitwise
typedef with bit numbers in an anonymous enum, which is fine with me.

[snipping the rest as my question has been answered]

Thanks,
Omar

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

[Israel Batista]

On 10/27/25 16:46, Lorenzo Stoakes wrote:

> 
> So why are we naming the type... does drgn require it?
> 

It doesn't need to be named, but as David pointed out, we could find
where these values are being used and replace the type with the proper
enum. I quickly grepped the codebase and it seems doable, I'm probably
adding these changes to the next version of this patch since there are
some things to fix anyway.

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

[Lorenzo Stoakes]

On Mon, Oct 27, 2025 at 11:53:57PM +0000, Israel Batista wrote:
>
>
> On 10/27/25 16:46, Lorenzo Stoakes wrote:
>
> >
> > So why are we naming the type... does drgn require it?
> >
>
> It doesn't need to be named, but as David pointed out, we could find
> where these values are being used and replace the type with the proper
> enum. I quickly grepped the codebase and it seems doable, I'm probably
> adding these changes to the next version of this patch since there are
> some things to fix anyway.

I mean we're getting a little out of scope here but fine, if these are not
in fact used as flags, I don't mind, just remove the silly 1 << x values
while you do it.

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

[Israel Batista]

On 10/28/25 13:34, Lorenzo Stoakes wrote:
> On Mon, Oct 27, 2025 at 11:53:57PM +0000, Israel Batista wrote:
>>
>>
>> On 10/27/25 16:46, Lorenzo Stoakes wrote:
>>
>>>
>>> So why are we naming the type... does drgn require it?
>>>
>>
>> It doesn't need to be named, but as David pointed out, we could find
>> where these values are being used and replace the type with the proper
>> enum. I quickly grepped the codebase and it seems doable, I'm probably
>> adding these changes to the next version of this patch since there are
>> some things to fix anyway.
> 
> I mean we're getting a little out of scope here but fine, if these are not
> in fact used as flags, I don't mind, just remove the silly 1 << x values
> while you do it.

Could you also point me to what tree is preferred to base the patches
for the mm subsystem, please? There are differences between linux-next
and the current version of mainline regarding these values.
(MEM_PREPARE_OFFLINE and MEM_FINISH_OFFLINE are being removed on
linux-next).

So I would appreciate if you could indicate what tree is the best to
base my changes, as I'm not that familiar with this subsystem.

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

[Lorenzo Stoakes]

On Tue, Oct 28, 2025 at 07:06:00PM +0000, Israel Batista wrote:
>
>
> On 10/28/25 13:34, Lorenzo Stoakes wrote:
> > On Mon, Oct 27, 2025 at 11:53:57PM +0000, Israel Batista wrote:
> > >
> > >
> > > On 10/27/25 16:46, Lorenzo Stoakes wrote:
> > >
> > > >
> > > > So why are we naming the type... does drgn require it?
> > > >
> > >
> > > It doesn't need to be named, but as David pointed out, we could find
> > > where these values are being used and replace the type with the proper
> > > enum. I quickly grepped the codebase and it seems doable, I'm probably
> > > adding these changes to the next version of this patch since there are
> > > some things to fix anyway.
> >
> > I mean we're getting a little out of scope here but fine, if these are not
> > in fact used as flags, I don't mind, just remove the silly 1 << x values
> > while you do it.
>
> Could you also point me to what tree is preferred to base the patches
> for the mm subsystem, please? There are differences between linux-next
> and the current version of mainline regarding these values.
> (MEM_PREPARE_OFFLINE and MEM_FINISH_OFFLINE are being removed on
> linux-next).
>
> So I would appreciate if you could indicate what tree is the best to
> base my changes, as I'm not that familiar with this subsystem.

Sure it's not at all obvious atm (mea culpa mm :)

So the tree is at https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git/

The best branch is mm-new, however this is the 'crazy' branch where _every
ongoing series_ that isn't found to be broken is taken as a testing ground and
base point for development to avoid conflicts.

I tend to base all my series on mm-new, as series must apply against it to be
taken there of course (though Andrew may resolve simpler conflicts).

The more stable ground (despite the name) is mm-unstable, which is everything
that has sat in mm-new long enough to be likely to be sent to Linus for the next
release, and it's mm-unstable that gets set to linux-next.

_Generally speaking_ your series should apply against both. If it doesn't, I'd
base it on mm-new.

So in general TL;DR mm-new is the way to go.

Cheers, Lorenzo

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

[Omar Sandoval]

On Sun, Oct 26, 2025 at 04:22:05PM +0000, 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 <linux@israelbatista.dev.br>
> ---
>  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)

What kernel version is this patch based on? It doesn't apply on mainline
because it is missing a couple of definitions added in 6.9 by commit
c5f1e2d18909 ("mm/memory_hotplug: introduce
MEM_PREPARE_ONLINE/MEM_FINISH_OFFLINE notifiers").

> +enum mem_states {

mem_state is very vague. enum memory_block_state might be a more
appropriate name.

Thanks for sending this!

Omar

> +	/* 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;
> -- 
> 2.51.0
> 
> 

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

[David Hildenbrand]

On 27.10.25 19:18, Omar Sandoval wrote:
> On Sun, Oct 26, 2025 at 04:22:05PM +0000, 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 <linux@israelbatista.dev.br>
>> ---
>>   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)
> 
> What kernel version is this patch based on? It doesn't apply on mainline
> because it is missing a couple of definitions added in 6.9 by commit
> c5f1e2d18909 ("mm/memory_hotplug: introduce
> MEM_PREPARE_ONLINE/MEM_FINISH_OFFLINE notifiers").
> 
>> +enum mem_states {
> 
> mem_state is very vague. enum memory_block_state might be a more
> appropriate name.

Agreed. That then nicely fits "struct memory_block".

-- 
Cheers

David / dhildenb

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

[Israel Batista]

On 10/27/25 15:18, Omar Sandoval wrote:
> 
> What kernel version is this patch based on? It doesn't apply on mainline
> because it is missing a couple of definitions added in 6.9 by commit
> c5f1e2d18909 ("mm/memory_hotplug: introduce
> MEM_PREPARE_ONLINE/MEM_FINISH_OFFLINE notifiers").
> 

Sorry, my bad. I mixed-up my local branches and created the patch from
linux-next rather than Linus's tree. I will create a new patch with the
proper changes.

>> +enum mem_states {
> 
> mem_state is very vague. enum memory_block_state might be a more
> appropriate name.
> 

Agreed, I will change it on the next version of the patch.

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

[Omar Sandoval]

On Mon, Oct 27, 2025 at 11:41:47PM +0000, Israel Batista wrote:
> 
> 
> On 10/27/25 15:18, Omar Sandoval wrote:
> > 
> > What kernel version is this patch based on? It doesn't apply on mainline
> > because it is missing a couple of definitions added in 6.9 by commit
> > c5f1e2d18909 ("mm/memory_hotplug: introduce
> > MEM_PREPARE_ONLINE/MEM_FINISH_OFFLINE notifiers").
> > 
> 
> Sorry, my bad. I mixed-up my local branches and created the patch from
> linux-next rather than Linus's tree. I will create a new patch with the
> proper changes.

Oh, linux-next is usually fine, too (different subsystems have different
preferences, and I don't know mm's preference). Regardless of what tree
you base it on, it's nice if you mention the base in the patch email
under the ---.

Thanks,
Omar