[PATCH 1/7] drivers: base: move mutex lock out of add_memory_section()

[PATCH 1/7] drivers: base: move mutex lock out of add_memory_section()

[Seth Jennings]

There is no point in releasing the mutex for each section that is added
during boot time.  Just hold it over the entire initialization loop.

Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
---
 drivers/base/memory.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 2b7813e..278bb3da 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -613,8 +613,6 @@ static int add_memory_section(int nid, struct mem_section *section,
 	int scn_nr = __section_nr(section);
 	int ret = 0;
 
-	mutex_lock(&mem_sysfs_mutex);
-
 	if (context == BOOT) {
 		/* same memory block ? */
 		if (mem_p && *mem_p)
@@ -643,7 +641,6 @@ static int add_memory_section(int nid, struct mem_section *section,
 			ret = register_mem_sect_under_node(mem, nid);
 	}
 
-	mutex_unlock(&mem_sysfs_mutex);
 	return ret;
 }
 
@@ -653,7 +650,13 @@ static int add_memory_section(int nid, struct mem_section *section,
  */
 int register_new_memory(int nid, struct mem_section *section)
 {
-	return add_memory_section(nid, section, NULL, MEM_OFFLINE, HOTPLUG);
+	int ret;
+
+	mutex_lock(&mem_sysfs_mutex);
+	ret = add_memory_section(nid, section, NULL, MEM_OFFLINE, HOTPLUG);
+	mutex_unlock(&mem_sysfs_mutex);
+
+	return ret;
 }
 
 #ifdef CONFIG_MEMORY_HOTREMOVE
@@ -746,6 +749,7 @@ int __init memory_dev_init(void)
 	 * Create entries for memory sections that were found
 	 * 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;
@@ -757,6 +761,7 @@ int __init memory_dev_init(void)
 		if (!ret)
 			ret = err;
 	}
+	mutex_unlock(&mem_sysfs_mutex);
 
 out:
 	if (ret)
-- 
1.8.3.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 2/7] drivers: base: remove unneeded variable

[Seth Jennings]

The error variable is not needed.

Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
---
 drivers/base/memory.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 278bb3da..e771e2b 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -565,16 +565,13 @@ static const struct attribute_group *memory_memblk_attr_groups[] = {
 static
 int register_memory(struct memory_block *memory)
 {
-	int error;
-
 	memory->dev.bus = &memory_subsys;
 	memory->dev.id = memory->start_section_nr / sections_per_block;
 	memory->dev.release = memory_block_release;
 	memory->dev.groups = memory_memblk_attr_groups;
 	memory->dev.offline = memory->state == MEM_OFFLINE;
 
-	error = device_register(&memory->dev);
-	return error;
+	return device_register(&memory->dev);
 }
 
 static int init_memory_block(struct memory_block **memory,
-- 
1.8.3.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 3/7] drivers: base: use device get/put functions

[Seth Jennings]

Use the [get|put]_device functions for ref'ing the memory block device
rather than the kobject functions which should be hidden away by the
device layer.

Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
---
 drivers/base/memory.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index e771e2b..a77753c 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -616,14 +616,14 @@ static int add_memory_section(int nid, struct mem_section *section,
 			if (scn_nr >= (*mem_p)->start_section_nr &&
 			    scn_nr <= (*mem_p)->end_section_nr) {
 				mem = *mem_p;
-				kobject_get(&mem->dev.kobj);
+				get_device(&mem->dev);
 			}
 	} else
 		mem = find_memory_block(section);
 
 	if (mem) {
 		mem->section_count++;
-		kobject_put(&mem->dev.kobj);
+		put_device(&mem->dev);
 	} else {
 		ret = init_memory_block(&mem, section, state);
 		/* store memory_block pointer for next loop */
@@ -663,7 +663,7 @@ unregister_memory(struct memory_block *memory)
 	BUG_ON(memory->dev.bus != &memory_subsys);
 
 	/* drop the ref. we got in remove_memory_block() */
-	kobject_put(&memory->dev.kobj);
+	put_device(&memory->dev);
 	device_unregister(&memory->dev);
 }
 
@@ -680,7 +680,7 @@ static int remove_memory_block(unsigned long node_id,
 	if (mem->section_count == 0)
 		unregister_memory(mem);
 	else
-		kobject_put(&mem->dev.kobj);
+		put_device(&mem->dev);
 
 	mutex_unlock(&mem_sysfs_mutex);
 	return 0;
-- 
1.8.3.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 4/7] drivers: base: unshare add_memory_section() from hotplug

[Seth Jennings]

add_memory_section() is currently called from both boot time and run
time via hotplug and there is a lot of nastiness to allow for shared
code including an enum parameter to convey the calling context to
add_memory_section().

This patch is the first step in breaking up the messy code sharing by
pulling the hotplug path for add_memory_section() directly into
register_new_memory().

Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
---
 drivers/base/memory.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index a77753c..05a90ba 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -647,12 +647,25 @@ static int add_memory_section(int nid, struct mem_section *section,
  */
 int register_new_memory(int nid, struct mem_section *section)
 {
-	int ret;
+	int ret = 0;
+	struct memory_block *mem;
 
 	mutex_lock(&mem_sysfs_mutex);
-	ret = add_memory_section(nid, section, NULL, MEM_OFFLINE, HOTPLUG);
-	mutex_unlock(&mem_sysfs_mutex);
 
+	mem = find_memory_block(section);
+	if (mem) {
+		mem->section_count++;
+		put_device(&mem->dev);
+	} else {
+		ret = init_memory_block(&mem, section, MEM_OFFLINE);
+		if (ret)
+			goto out;
+	}
+
+	if (mem->section_count == sections_per_block)
+		ret = register_mem_sect_under_node(mem, nid);
+out:
+	mutex_unlock(&mem_sysfs_mutex);
 	return ret;
 }
 
-- 
1.8.3.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 5/7] drivers: base: reduce add_memory_section() for boot-time only

[Seth Jennings]

Now that add_memory_section() is only called from boot time, reduce
the logic and remove the enum.

Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
---
 drivers/base/memory.c  | 41 ++++++++++++++---------------------------
 include/linux/memory.h |  1 -
 2 files changed, 14 insertions(+), 28 deletions(-)

diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 05a90ba..a695164 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -602,40 +602,29 @@ static int init_memory_block(struct memory_block **memory,
 	return ret;
 }
 
-static int add_memory_section(int nid, struct mem_section *section,
-			struct memory_block **mem_p,
-			unsigned long state, enum mem_add_context context)
+static int add_memory_section(struct mem_section *section,
+			struct memory_block **mem_p)
 {
 	struct memory_block *mem = NULL;
 	int scn_nr = __section_nr(section);
 	int ret = 0;
 
-	if (context == BOOT) {
-		/* same memory block ? */
-		if (mem_p && *mem_p)
-			if (scn_nr >= (*mem_p)->start_section_nr &&
-			    scn_nr <= (*mem_p)->end_section_nr) {
-				mem = *mem_p;
-				get_device(&mem->dev);
-			}
-	} else
-		mem = find_memory_block(section);
+	if (mem_p && *mem_p) {
+		if (scn_nr >= (*mem_p)->start_section_nr &&
+		    scn_nr <= (*mem_p)->end_section_nr) {
+			mem = *mem_p;
+			get_device(&mem->dev);
+		}
+	}
 
 	if (mem) {
 		mem->section_count++;
 		put_device(&mem->dev);
 	} else {
-		ret = init_memory_block(&mem, section, state);
+		ret = init_memory_block(&mem, section, MEM_ONLINE);
 		/* store memory_block pointer for next loop */
-		if (!ret && context == BOOT)
-			if (mem_p)
-				*mem_p = mem;
-	}
-
-	if (!ret) {
-		if (context == HOTPLUG &&
-		    mem->section_count == sections_per_block)
-			ret = register_mem_sect_under_node(mem, nid);
+		if (!ret && mem_p)
+			*mem_p = mem;
 	}
 
 	return ret;
@@ -764,10 +753,8 @@ int __init memory_dev_init(void)
 		if (!present_section_nr(i))
 			continue;
 		/* don't need to reuse memory_block if only one per block */
-		err = add_memory_section(0, __nr_to_section(i),
-				 (sections_per_block == 1) ? NULL : &mem,
-					 MEM_ONLINE,
-					 BOOT);
+		err = add_memory_section(__nr_to_section(i),
+				 (sections_per_block == 1) ? NULL : &mem);
 		if (!ret)
 			ret = err;
 	}
diff --git a/include/linux/memory.h b/include/linux/memory.h
index 85c31a8..4c89fb0 100644
--- a/include/linux/memory.h
+++ b/include/linux/memory.h
@@ -125,7 +125,6 @@ extern struct memory_block *find_memory_block_hinted(struct mem_section *,
 							struct memory_block *);
 extern struct memory_block *find_memory_block(struct mem_section *);
 #define CONFIG_MEM_BLOCK_SIZE	(PAGES_PER_SECTION<<PAGE_SHIFT)
-enum mem_add_context { BOOT, HOTPLUG };
 #endif /* CONFIG_MEMORY_HOTPLUG_SPARSE */
 
 #ifdef CONFIG_MEMORY_HOTPLUG
-- 
1.8.3.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 6/7] drivers: base: remove improper get/put in add_memory_section()

[Seth Jennings]

The path through add_memory_section() when the memory block already
exists uses flawed refcounting logic.  A get_device() is done on a
memory block using a pointer that might not be valid as we dropped
our previous reference and didn't obtain a new reference in the
proper way.

Lets stop pretending and just remove the get/put.  The
mem_sysfs_mutex, which we hold over the entire init loop now, will
prevent the memory blocks from disappearing from under us.

Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
---
 drivers/base/memory.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index a695164..7d9d3bc 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -613,14 +613,12 @@ static int add_memory_section(struct mem_section *section,
 		if (scn_nr >= (*mem_p)->start_section_nr &&
 		    scn_nr <= (*mem_p)->end_section_nr) {
 			mem = *mem_p;
-			get_device(&mem->dev);
 		}
 	}
 
-	if (mem) {
+	if (mem)
 		mem->section_count++;
-		put_device(&mem->dev);
-	} else {
+	else {
 		ret = init_memory_block(&mem, section, MEM_ONLINE);
 		/* store memory_block pointer for next loop */
 		if (!ret && mem_p)
-- 
1.8.3.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 7/7] drivers: base: refactor add_memory_section() to add_memory_block()

[Seth Jennings]

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@linux.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) {
+		err = add_memory_block(i);
 		if (!ret)
 			ret = err;
 	}
-- 
1.8.3.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/7] drivers: base: move mutex lock out of add_memory_section()

[Seth Jennings]

Gah! Forgot the cover letter.

This patchset just seeks to clean up and refactor some things in
memory.c for better understanding and possibly better performance due do
a decrease in mutex acquisitions and refcount churn at boot time.  No
functional change is intended by this set!

Seth

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/7] drivers: base: move mutex lock out of add_memory_section()

[Greg Kroah-Hartman]

On Tue, Aug 20, 2013 at 12:24:45PM -0500, Seth Jennings wrote:
> Gah! Forgot the cover letter.

No worries, I barely read them anyway :)

> This patchset just seeks to clean up and refactor some things in
> memory.c for better understanding and possibly better performance due do
> a decrease in mutex acquisitions and refcount churn at boot time.  No
> functional change is intended by this set!

All looks good, thanks for breaking it up into reviewable patches.  Now
applied.

greg k-h

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 7/7] drivers: base: refactor add_memory_section() to add_memory_block()

[Yasuaki Ishimatsu]

(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@linux.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, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 7/7] drivers: base: refactor add_memory_section() to add_memory_block()

[Yasuaki Ishimatsu]

(2013/08/22 17:20), Yasuaki Ishimatsu wrote:
> (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@linux.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?

Sorry for the noise. I understood.
The check was moved into add_memory_section(). So it was removed.

Thanks,
Yasuaki Ishimatsu

> 
>> +		err = add_memory_block(i);
>>    		if (!ret)
> 
> Thanks,
> Yasuaki Ishimatasu
> 
>>    			ret = err;
>>    	}
>>
> 
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
> 


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/7] drivers: base: move mutex lock out of add_memory_section()

[Yasuaki Ishimatsu]

(2013/08/21 2:24), Seth Jennings wrote:
> Gah! Forgot the cover letter.
>
> This patchset just seeks to clean up and refactor some things in
> memory.c for better understanding and possibly better performance due do
> a decrease in mutex acquisitions and refcount churn at boot time.  No
> functional change is intended by this set!

All patches were
Reviewed-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
Tested-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>

Thanks,
Yasuaki Ishimatsu

>
> Seth
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 7/7] drivers: base: refactor add_memory_section() to add_memory_block()

[Seth Jennings]

On Thu, Aug 22, 2013 at 05:20:03PM +0900, Yasuaki Ishimatsu wrote:
> (2013/08/21 2:13), Seth Jennings wrote:
> > -	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?

The previous logic was that if any section was present in the memory
block that the memory block is created.  If you do the
present_setcion_nr() check here, if the first section isn't
present, it skips the whole memory block, even though there may have
been other present sections in that block, which isn't what we want.

Seth

> 
> > +		err = add_memory_block(i);
> >   		if (!ret)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>