Re: [PATCH v2 3/3] mm: change type of parameter for memory_notify

Lorenzo Stoakes <[email protected]> Thu, 30 Oct 2025 10:56:26 +0000
Newsgroups org.kernel.vger.linux-debuggers,org.kvack.linux-mm
Message-ID <[email protected]>
On Wed, Oct 29, 2025 at 07:56:32PM +0000, Israel Batista wrote:
> The memory_notify function is responsible for sending events related to
> memory hotplugging to a notification queue. Since all the events must
> match one of the values from the enum memory_block_state, it is
> appropriate to change the function parameter type to make this
> condition explicit at compile time.
>
> Signed-off-by: Israel Batista <[email protected]>

This seems fine, but I can see a whole bunch of others like:

kcore_callback()
mm_compute_batch_notifier()
page_ext_callback()
reserve_mem_notiifer()
etc.

So I think worth chasing all of these down?

Some of the switches which don't cover all cases will need to be adjusted to
insert a no-op default:

> ---
>  drivers/base/memory.c  | 4 ++--
>  include/linux/memory.h | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index 3d17dd774947..c03f3b5e5e6f 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -204,9 +204,9 @@ static ssize_t state_show(struct device *dev, struct device_attribute *attr,
>  	return sysfs_emit(buf, "%s\n", output);
>  }
>
> -int memory_notify(unsigned long val, void *v)
> +int memory_notify(enum memory_block_state state, void *v)
>  {
> -	return blocking_notifier_call_chain(&memory_chain, val, v);
> +	return blocking_notifier_call_chain(&memory_chain, state, v);
>  }
>
>  #if defined(CONFIG_MEMORY_FAILURE) && defined(CONFIG_MEMORY_HOTPLUG)
> diff --git a/include/linux/memory.h b/include/linux/memory.h
> index 36d733283329..6a2456686bf4 100644
> --- a/include/linux/memory.h
> +++ b/include/linux/memory.h
> @@ -141,7 +141,7 @@ static inline int register_memory_notifier(struct notifier_block *nb)
>  static inline void unregister_memory_notifier(struct notifier_block *nb)
>  {
>  }
> -static inline int memory_notify(unsigned long val, void *v)
> +static inline int memory_notify(enum memory_block_state state, void *v)
>  {
>  	return 0;
>  }
> @@ -165,7 +165,7 @@ int create_memory_block_devices(unsigned long start, unsigned long size,
>  				struct memory_group *group);
>  void remove_memory_block_devices(unsigned long start, unsigned long size);
>  extern void memory_dev_init(void);
> -extern int memory_notify(unsigned long val, void *v);
> +extern int memory_notify(enum memory_block_state state, void *v);

No reason you'd know, but we have a pattern of removing extraneous extern's like
this when we otherwise change a line.

>  extern struct memory_block *find_memory_block(unsigned long section_nr);
>  typedef int (*walk_memory_blocks_func_t)(struct memory_block *, void *);
>  extern int walk_memory_blocks(unsigned long start, unsigned long size,
> --
> 2.51.0
>