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 >