linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
To: Israel Batista <linux@israelbatista.dev.br>
Cc: david@redhat.com, akpm@linux-foundation.org, linux-mm@kvack.org,
	osandov@osandov.com, linux-debuggers@vger.kernel.org
Subject: Re: [PATCH v2 3/3] mm: change type of parameter for memory_notify
Date: Thu, 30 Oct 2025 10:56:26 +0000	[thread overview]
Message-ID: <004bb148-a818-43d2-af04-2f2ed85cbd67@lucifer.local> (raw)
In-Reply-To: <20251029195617.2210700-4-linux@israelbatista.dev.br>

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

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
>


  reply	other threads:[~2025-10-30 10:56 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-29 19:56 [PATCH v2 0/3] mm: Convert memory block states (MEM_*) macros to Israel Batista
2025-10-29 19:56 ` [PATCH v2 1/3] mm: convert memory block states (MEM_*) macros to enum Israel Batista
2025-10-30 10:52   ` Lorenzo Stoakes
2025-10-30 11:31   ` David Hildenbrand
2025-10-29 19:56 ` [PATCH v2 2/3] mm: change type of state in struct memory_block Israel Batista
2025-10-29 20:59   ` Randy Dunlap
2025-10-30 11:00   ` Lorenzo Stoakes
2025-10-30 11:32   ` David Hildenbrand
2025-10-29 19:56 ` [PATCH v2 3/3] mm: change type of parameter for memory_notify Israel Batista
2025-10-30 10:56   ` Lorenzo Stoakes [this message]
2025-10-30 11:16     ` Israel Batista
2025-10-30 11:34       ` David Hildenbrand
2025-10-30 12:00         ` Israel Batista
2025-10-30 14:57 ` [PATCH v2 0/3] mm: Convert memory block states (MEM_*) macros to Mike Rapoport

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=004bb148-a818-43d2-af04-2f2ed85cbd67@lucifer.local \
    --to=lorenzo.stoakes@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@redhat.com \
    --cc=linux-debuggers@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux@israelbatista.dev.br \
    --cc=osandov@osandov.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox