Skip to contentSkip to conversation

base/memory: pass the base_section in add_memory_block

PATCH Superseded Wei Yang 3 participants 3 messages Jun 7, 2017 – Jun 13, 2017

Subsystems (2)
lore
Share & tools
lore

Wei Yang

author
#0 lore raw

PATCH 1/1 base/memory: pass the base_section in add_memory_block

The second parameter of init_memory_block() is used to calculate the start_section_nr of this block, which means any section in the same block would get the same start_section_nr.

This patch passes the base_section to init_memory_block(), so that to reduce a local variable and a check in every loop.

---
 drivers/base/memory.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/base/memory.c +3 -4
index cc4f1d0cbffe..1e903aba2aa1 100644
@@ -664,21 +664,20 @@ static int init_memory_block(struct memory_block **memory,
 static int add_memory_block(int base_section_nr)
 {
 	struct memory_block *mem;
-	int i, ret, section_count = 0, section_nr;
+	int i, ret, section_count = 0;
 
 	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++;
 	}
 
 	if (section_count == 0)
 		return 0;
-	ret = init_memory_block(&mem, __nr_to_section(section_nr), MEM_ONLINE);
+	ret = init_memory_block(&mem, __nr_to_section(base_section_nr),
+				MEM_ONLINE);
 	if (ret)
 		return ret;
 	mem->section_count = section_count;
Signature · 1 line
2.11.0
#1 lore raw

[Sorry for a late response]

Quoted 3 lines · Wei Yang

The second parameter of init_memory_block() is used to calculate the start_section_nr of this block, which means any section in the same block would get the same start_section_nr.

Could you be more specific what is the problem here?

Quoted 2 lines

This patch passes the base_section to init_memory_block(), so that to reduce a local variable and a check in every loop.

But then you are not handling a memblock which starts with a !present section. The code is quite hairy but I do not see why your change is any more correct. This needs much better justification than what the above gives us. Maybe the whole thing about incomplete memblock is just overengineered piece of code, who knows this area is full of stuff that makes only little sense but again the changelog should be pretty verbose about all the consequences and focus on the high level rather than particular issues here and there.

Thanks

Quoted 36 lines
---
 drivers/base/memory.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/base/memory.c +3 -4
index cc4f1d0cbffe..1e903aba2aa1 100644
@@ -664,21 +664,20 @@ static int init_memory_block(struct memory_block **memory,
 static int add_memory_block(int base_section_nr)
 {
 	struct memory_block *mem;
-	int i, ret, section_count = 0, section_nr;
+	int i, ret, section_count = 0;
 
 	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++;
 	}
 
 	if (section_count == 0)
 		return 0;
-	ret = init_memory_block(&mem, __nr_to_section(section_nr), MEM_ONLINE);
+	ret = init_memory_block(&mem, __nr_to_section(base_section_nr),
+				MEM_ONLINE);
 	if (ret)
 		return ret;
 	mem->section_count = section_count;
Signature · 1 line
2.11.0
Signature · 2 lines
Michal Hocko
SUSE Labs
maintainer
#2 lore raw
In reply to Wei Yang
Quoted 11 lines · Wei Yang

The second parameter of init_memory_block() is used to calculate the start_section_nr of this block, which means any section in the same block would get the same start_section_nr.

This patch passes the base_section to init_memory_block(), so that to reduce a local variable and a check in every loop.

---
 drivers/base/memory.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Can you resend this and cc: linux-mm? I think the developers there need to review this.

thanks,

greg k-h