[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <5215C9B3.4090608@jp.fujitsu.com>
Date: Thu, 22 Aug 2013 17:20:03 +0900
From: Yasuaki Ishimatsu <isimatu.yasuaki@...fujitsu.com>
To: Seth Jennings <sjenning@...ux.vnet.ibm.com>
CC: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Dave Hansen <dave@...1.net>,
Nathan Fontenot <nfont@...ux.vnet.ibm.com>,
Cody P Schafer <cody@...ux.vnet.ibm.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Lai Jiangshan <laijs@...fujitsu.com>,
"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
Yinghai Lu <yinghai@...nel.org>,
Wanpeng Li <liwanp@...ux.vnet.ibm.com>,
<linux-kernel@...r.kernel.org>, <linux-mm@...ck.org>,
<linuxppc-dev@...ts.ozlabs.org>
Subject: Re: [PATCH 7/7] drivers: base: refactor add_memory_section() to add_memory_block()
(2013/08/21 2:13), Seth Jennings wrote:
> Right now memory_dev_init() maintains the memory block pointer
> between iterations of add_memory_section(). This is nasty.
>
> This patch refactors add_memory_section() to become add_memory_block().
> The refactoring pulls the section scanning out of memory_dev_init()
> and simplifies the signature.
>
> Signed-off-by: Seth Jennings <sjenning@...ux.vnet.ibm.com>
> ---
> drivers/base/memory.c | 48 +++++++++++++++++++++---------------------------
> 1 file changed, 21 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index 7d9d3bc..021283a 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -602,32 +602,31 @@ static int init_memory_block(struct memory_block **memory,
> return ret;
> }
>
> -static int add_memory_section(struct mem_section *section,
> - struct memory_block **mem_p)
> +static int add_memory_block(int base_section_nr)
> {
> - struct memory_block *mem = NULL;
> - int scn_nr = __section_nr(section);
> - int ret = 0;
> -
> - if (mem_p && *mem_p) {
> - if (scn_nr >= (*mem_p)->start_section_nr &&
> - scn_nr <= (*mem_p)->end_section_nr) {
> - mem = *mem_p;
> - }
> - }
> + struct memory_block *mem;
> + int i, ret, section_count = 0, section_nr;
>
> - if (mem)
> - mem->section_count++;
> - else {
> - ret = init_memory_block(&mem, section, MEM_ONLINE);
> - /* store memory_block pointer for next loop */
> - if (!ret && mem_p)
> - *mem_p = mem;
> + for (i = base_section_nr;
> + (i < base_section_nr + sections_per_block) && i < NR_MEM_SECTIONS;
> + i++) {
> + if (!present_section_nr(i))
> + continue;
> + if (section_count == 0)
> + section_nr = i;
> + section_count++;
> }
>
> - return ret;
> + if (section_count == 0)
> + return 0;
> + ret = init_memory_block(&mem, __nr_to_section(section_nr), MEM_ONLINE);
> + if (ret)
> + return ret;
> + mem->section_count = section_count;
> + return 0;
> }
>
> +
> /*
> * need an interface for the VM to add new memory regions,
> * but without onlining it.
> @@ -733,7 +732,6 @@ int __init memory_dev_init(void)
> int ret;
> int err;
> unsigned long block_sz;
> - struct memory_block *mem = NULL;
>
> ret = subsys_system_register(&memory_subsys, memory_root_attr_groups);
> if (ret)
> @@ -747,12 +745,8 @@ int __init memory_dev_init(void)
> * during boot and have been initialized
> */
> mutex_lock(&mem_sysfs_mutex);
> - for (i = 0; i < NR_MEM_SECTIONS; i++) {
> - if (!present_section_nr(i))
> - continue;
> - /* don't need to reuse memory_block if only one per block */
> - err = add_memory_section(__nr_to_section(i),
> - (sections_per_block == 1) ? NULL : &mem);
> + for (i = 0; i < NR_MEM_SECTIONS; i += sections_per_block) {
Why do you remove present_setcion_nr() check?
> + err = add_memory_block(i);
> if (!ret)
Thanks,
Yasuaki Ishimatasu
> ret = err;
> }
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists