2013-09-10 00:26:29

by Toshi Kani

[permalink] [raw]
Subject: [PATCH] cpu/mem hotplug: Add try_online_node() for cpu_up()

cpu_up() has #ifdef CONFIG_MEMORY_HOTPLUG code blocks, which
call mem_online_node() to put its node online if offlined and
then call build_all_zonelists() to initialize the zone list.
These steps are specific to memory hotplug, and should be
managed in mm/memory_hotplug.c. lock_memory_hotplug() should
also be held for the whole steps.

For this reason, this patch replaces mem_online_node() with
try_online_node(), which performs the whole steps with
lock_memory_hotplug() held. try_online_node() is named after
try_offline_node() as they have similar purpose.

There is no functional change in this patch.

Signed-off-by: Toshi Kani <[email protected]>
---
include/linux/memory_hotplug.h | 8 +++++++-
kernel/cpu.c | 29 +++--------------------------
mm/memory_hotplug.c | 15 +++++++++++++--
3 files changed, 23 insertions(+), 29 deletions(-)

diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index dd38e62..22203c2 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -94,6 +94,8 @@ extern void __online_page_set_limits(struct page *page);
extern void __online_page_increment_counters(struct page *page);
extern void __online_page_free(struct page *page);

+extern int try_online_node(int nid);
+
#ifdef CONFIG_MEMORY_HOTREMOVE
extern bool is_pageblock_removable_nolock(struct page *page);
extern int arch_remove_memory(u64 start, u64 size);
@@ -225,6 +227,11 @@ static inline void register_page_bootmem_info_node(struct pglist_data *pgdat)
{
}

+static inline int try_online_node(int nid)
+{
+ return 0;
+}
+
static inline void lock_memory_hotplug(void) {}
static inline void unlock_memory_hotplug(void) {}

@@ -256,7 +263,6 @@ static inline void remove_memory(int nid, u64 start, u64 size) {}

extern int walk_memory_range(unsigned long start_pfn, unsigned long end_pfn,
void *arg, int (*func)(struct memory_block *, void *));
-extern int mem_online_node(int nid);
extern int add_memory(int nid, u64 start, u64 size);
extern int arch_add_memory(int nid, u64 start, u64 size);
extern int offline_pages(unsigned long start_pfn, unsigned long nr_pages);
diff --git a/kernel/cpu.c b/kernel/cpu.c
index d7f07a2..c10b285 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -420,11 +420,6 @@ int cpu_up(unsigned int cpu)
{
int err = 0;

-#ifdef CONFIG_MEMORY_HOTPLUG
- int nid;
- pg_data_t *pgdat;
-#endif
-
if (!cpu_possible(cpu)) {
printk(KERN_ERR "can't online cpu %d because it is not "
"configured as may-hotadd at boot time\n", cpu);
@@ -435,27 +430,9 @@ int cpu_up(unsigned int cpu)
return -EINVAL;
}

-#ifdef CONFIG_MEMORY_HOTPLUG
- nid = cpu_to_node(cpu);
- if (!node_online(nid)) {
- err = mem_online_node(nid);
- if (err)
- return err;
- }
-
- pgdat = NODE_DATA(nid);
- if (!pgdat) {
- printk(KERN_ERR
- "Can't online cpu %d due to NULL pgdat\n", cpu);
- return -ENOMEM;
- }
-
- if (pgdat->node_zonelists->_zonerefs->zone == NULL) {
- mutex_lock(&zonelists_mutex);
- build_all_zonelists(NULL, NULL);
- mutex_unlock(&zonelists_mutex);
- }
-#endif
+ err = try_online_node(cpu_to_node(cpu));
+ if (err)
+ return err;

cpu_maps_update_begin();

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index ed85fe3..c326bdf 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1044,14 +1044,19 @@ static void rollback_node_hotadd(int nid, pg_data_t *pgdat)
}


-/*
+/**
+ * try_online_node - online a node if offlined
+ *
* called by cpu_up() to online a node without onlined memory.
*/
-int mem_online_node(int nid)
+int try_online_node(int nid)
{
pg_data_t *pgdat;
int ret;

+ if (node_online(nid))
+ return 0;
+
lock_memory_hotplug();
pgdat = hotadd_new_pgdat(nid, 0);
if (!pgdat) {
@@ -1062,6 +1067,12 @@ int mem_online_node(int nid)
ret = register_one_node(nid);
BUG_ON(ret);

+ if (pgdat->node_zonelists->_zonerefs->zone == NULL) {
+ mutex_lock(&zonelists_mutex);
+ build_all_zonelists(NULL, NULL);
+ mutex_unlock(&zonelists_mutex);
+ }
+
out:
unlock_memory_hotplug();
return ret;


2013-09-10 03:32:17

by Yasuaki Ishimatsu

[permalink] [raw]
Subject: Re: [PATCH] cpu/mem hotplug: Add try_online_node() for cpu_up()

(2013/09/10 9:24), Toshi Kani wrote:
> cpu_up() has #ifdef CONFIG_MEMORY_HOTPLUG code blocks, which
> call mem_online_node() to put its node online if offlined and
> then call build_all_zonelists() to initialize the zone list.
> These steps are specific to memory hotplug, and should be
> managed in mm/memory_hotplug.c. lock_memory_hotplug() should
> also be held for the whole steps.
>
> For this reason, this patch replaces mem_online_node() with
> try_online_node(), which performs the whole steps with
> lock_memory_hotplug() held. try_online_node() is named after
> try_offline_node() as they have similar purpose.
>
> There is no functional change in this patch.
>
> Signed-off-by: Toshi Kani <[email protected]>
> ---
> include/linux/memory_hotplug.h | 8 +++++++-
> kernel/cpu.c | 29 +++--------------------------
> mm/memory_hotplug.c | 15 +++++++++++++--
> 3 files changed, 23 insertions(+), 29 deletions(-)
>
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index dd38e62..22203c2 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -94,6 +94,8 @@ extern void __online_page_set_limits(struct page *page);
> extern void __online_page_increment_counters(struct page *page);
> extern void __online_page_free(struct page *page);
>
> +extern int try_online_node(int nid);
> +
> #ifdef CONFIG_MEMORY_HOTREMOVE
> extern bool is_pageblock_removable_nolock(struct page *page);
> extern int arch_remove_memory(u64 start, u64 size);
> @@ -225,6 +227,11 @@ static inline void register_page_bootmem_info_node(struct pglist_data *pgdat)
> {
> }
>
> +static inline int try_online_node(int nid)
> +{
> + return 0;
> +}
> +
> static inline void lock_memory_hotplug(void) {}
> static inline void unlock_memory_hotplug(void) {}
>
> @@ -256,7 +263,6 @@ static inline void remove_memory(int nid, u64 start, u64 size) {}
>
> extern int walk_memory_range(unsigned long start_pfn, unsigned long end_pfn,
> void *arg, int (*func)(struct memory_block *, void *));
> -extern int mem_online_node(int nid);
> extern int add_memory(int nid, u64 start, u64 size);
> extern int arch_add_memory(int nid, u64 start, u64 size);
> extern int offline_pages(unsigned long start_pfn, unsigned long nr_pages);
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index d7f07a2..c10b285 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -420,11 +420,6 @@ int cpu_up(unsigned int cpu)
> {
> int err = 0;
>
> -#ifdef CONFIG_MEMORY_HOTPLUG
> - int nid;
> - pg_data_t *pgdat;
> -#endif
> -
> if (!cpu_possible(cpu)) {
> printk(KERN_ERR "can't online cpu %d because it is not "
> "configured as may-hotadd at boot time\n", cpu);
> @@ -435,27 +430,9 @@ int cpu_up(unsigned int cpu)
> return -EINVAL;
> }
>
> -#ifdef CONFIG_MEMORY_HOTPLUG
> - nid = cpu_to_node(cpu);
> - if (!node_online(nid)) {
> - err = mem_online_node(nid);
> - if (err)
> - return err;
> - }
> -
> - pgdat = NODE_DATA(nid);
> - if (!pgdat) {

> - printk(KERN_ERR
> - "Can't online cpu %d due to NULL pgdat\n", cpu);

Please move this comments into try_online_node() too.

Thanks,
Yasuaki Ishimatsu

> - return -ENOMEM;
> - }
> -
> - if (pgdat->node_zonelists->_zonerefs->zone == NULL) {
> - mutex_lock(&zonelists_mutex);
> - build_all_zonelists(NULL, NULL);
> - mutex_unlock(&zonelists_mutex);
> - }
> -#endif
> + err = try_online_node(cpu_to_node(cpu));
> + if (err)
> + return err;
>
> cpu_maps_update_begin();
>
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index ed85fe3..c326bdf 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1044,14 +1044,19 @@ static void rollback_node_hotadd(int nid, pg_data_t *pgdat)
> }
>
>
> -/*
> +/**
> + * try_online_node - online a node if offlined
> + *
> * called by cpu_up() to online a node without onlined memory.
> */
> -int mem_online_node(int nid)
> +int try_online_node(int nid)
> {
> pg_data_t *pgdat;
> int ret;
>
> + if (node_online(nid))
> + return 0;
> +
> lock_memory_hotplug();
> pgdat = hotadd_new_pgdat(nid, 0);
> if (!pgdat) {
> @@ -1062,6 +1067,12 @@ int mem_online_node(int nid)
> ret = register_one_node(nid);
> BUG_ON(ret);
>
> + if (pgdat->node_zonelists->_zonerefs->zone == NULL) {
> + mutex_lock(&zonelists_mutex);
> + build_all_zonelists(NULL, NULL);
> + mutex_unlock(&zonelists_mutex);
> + }
> +
> out:
> unlock_memory_hotplug();
> return ret;
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to [email protected]. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
>

2013-09-10 03:35:06

by Yasuaki Ishimatsu

[permalink] [raw]
Subject: Re: [PATCH] cpu/mem hotplug: Add try_online_node() for cpu_up()

(2013/09/10 12:31), Yasuaki Ishimatsu wrote:
> (2013/09/10 9:24), Toshi Kani wrote:
>> cpu_up() has #ifdef CONFIG_MEMORY_HOTPLUG code blocks, which
>> call mem_online_node() to put its node online if offlined and
>> then call build_all_zonelists() to initialize the zone list.
>> These steps are specific to memory hotplug, and should be
>> managed in mm/memory_hotplug.c. lock_memory_hotplug() should
>> also be held for the whole steps.
>>
>> For this reason, this patch replaces mem_online_node() with
>> try_online_node(), which performs the whole steps with
>> lock_memory_hotplug() held. try_online_node() is named after
>> try_offline_node() as they have similar purpose.
>>
>> There is no functional change in this patch.
>>
>> Signed-off-by: Toshi Kani <[email protected]>
>> ---
>> include/linux/memory_hotplug.h | 8 +++++++-
>> kernel/cpu.c | 29 +++--------------------------
>> mm/memory_hotplug.c | 15 +++++++++++++--
>> 3 files changed, 23 insertions(+), 29 deletions(-)
>>
>> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
>> index dd38e62..22203c2 100644
>> --- a/include/linux/memory_hotplug.h
>> +++ b/include/linux/memory_hotplug.h
>> @@ -94,6 +94,8 @@ extern void __online_page_set_limits(struct page *page);
>> extern void __online_page_increment_counters(struct page *page);
>> extern void __online_page_free(struct page *page);
>>
>> +extern int try_online_node(int nid);
>> +
>> #ifdef CONFIG_MEMORY_HOTREMOVE
>> extern bool is_pageblock_removable_nolock(struct page *page);
>> extern int arch_remove_memory(u64 start, u64 size);
>> @@ -225,6 +227,11 @@ static inline void register_page_bootmem_info_node(struct pglist_data *pgdat)
>> {
>> }
>>
>> +static inline int try_online_node(int nid)
>> +{
>> + return 0;
>> +}
>> +
>> static inline void lock_memory_hotplug(void) {}
>> static inline void unlock_memory_hotplug(void) {}
>>
>> @@ -256,7 +263,6 @@ static inline void remove_memory(int nid, u64 start, u64 size) {}
>>
>> extern int walk_memory_range(unsigned long start_pfn, unsigned long end_pfn,
>> void *arg, int (*func)(struct memory_block *, void *));
>> -extern int mem_online_node(int nid);
>> extern int add_memory(int nid, u64 start, u64 size);
>> extern int arch_add_memory(int nid, u64 start, u64 size);
>> extern int offline_pages(unsigned long start_pfn, unsigned long nr_pages);
>> diff --git a/kernel/cpu.c b/kernel/cpu.c
>> index d7f07a2..c10b285 100644
>> --- a/kernel/cpu.c
>> +++ b/kernel/cpu.c
>> @@ -420,11 +420,6 @@ int cpu_up(unsigned int cpu)
>> {
>> int err = 0;
>>
>> -#ifdef CONFIG_MEMORY_HOTPLUG
>> - int nid;
>> - pg_data_t *pgdat;
>> -#endif
>> -
>> if (!cpu_possible(cpu)) {
>> printk(KERN_ERR "can't online cpu %d because it is not "
>> "configured as may-hotadd at boot time\n", cpu);
>> @@ -435,27 +430,9 @@ int cpu_up(unsigned int cpu)
>> return -EINVAL;
>> }
>>
>> -#ifdef CONFIG_MEMORY_HOTPLUG
>> - nid = cpu_to_node(cpu);
>> - if (!node_online(nid)) {
>> - err = mem_online_node(nid);
>> - if (err)
>> - return err;
>> - }
>> -
>> - pgdat = NODE_DATA(nid);
>> - if (!pgdat) {
>
>> - printk(KERN_ERR
>> - "Can't online cpu %d due to NULL pgdat\n", cpu);
>
> Please move this comments into try_online_node() too.

In this case, please use pr_err() instead of printk().

Thanks,
Yasuaki Ishimatsu

>
> Thanks,
> Yasuaki Ishimatsu
>
>> - return -ENOMEM;
>> - }
>> -
>> - if (pgdat->node_zonelists->_zonerefs->zone == NULL) {
>> - mutex_lock(&zonelists_mutex);
>> - build_all_zonelists(NULL, NULL);
>> - mutex_unlock(&zonelists_mutex);
>> - }
>> -#endif
>> + err = try_online_node(cpu_to_node(cpu));
>> + if (err)
>> + return err;
>>
>> cpu_maps_update_begin();
>>
>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>> index ed85fe3..c326bdf 100644
>> --- a/mm/memory_hotplug.c
>> +++ b/mm/memory_hotplug.c
>> @@ -1044,14 +1044,19 @@ static void rollback_node_hotadd(int nid, pg_data_t *pgdat)
>> }
>>
>>
>> -/*
>> +/**
>> + * try_online_node - online a node if offlined
>> + *
>> * called by cpu_up() to online a node without onlined memory.
>> */
>> -int mem_online_node(int nid)
>> +int try_online_node(int nid)
>> {
>> pg_data_t *pgdat;
>> int ret;
>>
>> + if (node_online(nid))
>> + return 0;
>> +
>> lock_memory_hotplug();
>> pgdat = hotadd_new_pgdat(nid, 0);
>> if (!pgdat) {
>> @@ -1062,6 +1067,12 @@ int mem_online_node(int nid)
>> ret = register_one_node(nid);
>> BUG_ON(ret);
>>
>> + if (pgdat->node_zonelists->_zonerefs->zone == NULL) {
>> + mutex_lock(&zonelists_mutex);
>> + build_all_zonelists(NULL, NULL);
>> + mutex_unlock(&zonelists_mutex);
>> + }
>> +
>> out:
>> unlock_memory_hotplug();
>> return ret;
>>
>> --
>> To unsubscribe, send a message with 'unsubscribe linux-mm' in
>> the body to [email protected]. For more info on Linux MM,
>> see: http://www.linux-mm.org/ .
>> Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
>>
>
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to [email protected]. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
>

2013-09-10 13:57:23

by Toshi Kani

[permalink] [raw]
Subject: Re: [PATCH] cpu/mem hotplug: Add try_online_node() for cpu_up()

On Tue, 2013-09-10 at 12:31 +0900, Yasuaki Ishimatsu wrote:
> (2013/09/10 9:24), Toshi Kani wrote:
:
> > diff --git a/kernel/cpu.c b/kernel/cpu.c
> > index d7f07a2..c10b285 100644
> > --- a/kernel/cpu.c
> > +++ b/kernel/cpu.c
> > @@ -420,11 +420,6 @@ int cpu_up(unsigned int cpu)
> > {
> > int err = 0;
> >
> > -#ifdef CONFIG_MEMORY_HOTPLUG
> > - int nid;
> > - pg_data_t *pgdat;
> > -#endif
> > -
> > if (!cpu_possible(cpu)) {
> > printk(KERN_ERR "can't online cpu %d because it is not "
> > "configured as may-hotadd at boot time\n", cpu);
> > @@ -435,27 +430,9 @@ int cpu_up(unsigned int cpu)
> > return -EINVAL;
> > }
> >
> > -#ifdef CONFIG_MEMORY_HOTPLUG
> > - nid = cpu_to_node(cpu);
> > - if (!node_online(nid)) {
> > - err = mem_online_node(nid);
> > - if (err)
> > - return err;
> > - }
> > -
> > - pgdat = NODE_DATA(nid);
> > - if (!pgdat) {
>
> > - printk(KERN_ERR
> > - "Can't online cpu %d due to NULL pgdat\n", cpu);
>
> Please move this comments into try_online_node() too.

This code block is no longer necessary, but I will add a pr_err() when
hotadd_new_pgdat() returns NULL in try_oline_node() below.

> > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> > index ed85fe3..c326bdf 100644
> > --- a/mm/memory_hotplug.c
> > +++ b/mm/memory_hotplug.c
> > @@ -1044,14 +1044,19 @@ static void rollback_node_hotadd(int nid, pg_data_t *pgdat)
> > }
> >
> >
> > -/*
> > +/**
> > + * try_online_node - online a node if offlined
> > + *
> > * called by cpu_up() to online a node without onlined memory.
> > */
> > -int mem_online_node(int nid)
> > +int try_online_node(int nid)
> > {
> > pg_data_t *pgdat;
> > int ret;
> >
> > + if (node_online(nid))
> > + return 0;
> > +
> > lock_memory_hotplug();
> > pgdat = hotadd_new_pgdat(nid, 0);
> > if (!pgdat) {

+ pr_err("Cannot online node %d due to NULL pgdat\n",
nid);

Thanks!
-Toshi