base/memory: pass the base_section in add_memory_block
Subsystems (3)
PATCH RESEND 1/1 base/memory: pass the base_section in add_memory_block
Based on Greg's comment, cc it to mm list.
The original thread could be found https://lkml.org/lkml/2017/6/7/202
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
@@ -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 · 7 lines
2.11.0 -- 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>
Quoted 2 lines · Wei Yang
Based on Greg's comment, cc it to mm list.
The original thread could be found https://lkml.org/lkml/2017/6/7/202
I have already given you feedback
http://lkml.kernel.org/r/[email protected]
and you seemed to ignore it completely.
Quoted 43 lines
The second parameter of
init_memory_block()is used to calculate thestart_section_nrof this block, which means any section in the same block would get the samestart_section_nr.This patch passes the
base_sectiontoinit_memory_block(), so that to reduce a local variable and a check in every loop.Signed-off-by Wei Yang <[email protected]>--- drivers/base/memory.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)drivers/base/memory.c +3
-4@@ -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 · 8 lines
Michal Hocko SUSE Labs -- 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>
Quoted 4 lines · Michal Hocko
Quoted 2 lines · Wei Yang
Based on Greg's comment, cc it to mm list.
The original thread could be found https://lkml.org/lkml/2017/6/7/202
Wow, you are still working~ I just moved your response in this thread~
So that other audience would be convenient to see the whole story.
Quoted 51 lines
I have already given you feedback
http://lkml.kernel.org/r/[email protected]
and you seemed to ignore it completely.Quoted 43 lines
The second parameter of
init_memory_block()is used to calculate thestart_section_nrof this block, which means any section in the same block would get the samestart_section_nr.This patch passes the
base_sectiontoinit_memory_block(), so that to reduce a local variable and a check in every loop.Signed-off-by Wei Yang <[email protected]>--- drivers/base/memory.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)drivers/base/memory.c +3
-4@@ -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.0Signature · 2 lines
Michal Hocko SUSE Labs
Signature · 2 lines
Wei Yang Help you, Help me
Quoted 9 lines · Wei Yang
Quoted 4 lines · Michal Hocko
Quoted 2 lines · Wei Yang
Based on Greg's comment, cc it to mm list.
The original thread could be found https://lkml.org/lkml/2017/6/7/202Wow, you are still working~ I just moved your response in this thread~
So that other audience would be convenient to see the whole story.
You could add linux-mm to the cc in the response to that email
Signature · 8 lines
Michal Hocko SUSE Labs -- 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>
Hi, Michael
I copied your reply here:
Quoted 9 lines
[Sorry for a late response]
Quoted 3 lines · Wei Yang
The second parameter of
init_memory_block()is used to calculate thestart_section_nrof this block, which means any section in the same block would get the samestart_section_nr.Could you be more specific what is the problem here?
There is no problem in this code. I just find a unnecessary calculation and remove it in this patch.
Quoted 5 lines
Quoted 2 lines
This patch passes the
base_sectiontoinit_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
I don't see the situation you pointed here.
In add_memory_block(), section_nr is used to record the first section which is present. And this variable is used to calculate the section which is passed to init_memory_block().
In init_memory_block(), the section got from add_memory_block(), is used to calculate scn_nr, but finally transformed to "start_section_nr". That means in init_memory_block(), we just need the "start_section_nr" of a memory_block. We don't care about who is the first present section.
Quoted 6 lines
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.
There maybe other issues in memory_block, while for the code refine in this patch, the change is straight and not see side effects.
The field memory_block->start_section_nr records the section number of the first section in memory_block. No semantic change here and comply with the high level view of memory_block hierarchy.
Quoted 3 lines
Thanks
Quoted 46 lines · Wei Yang
Based on Greg's comment, cc it to mm list.
The original thread could be found https://lkml.org/lkml/2017/6/7/202The second parameter of
init_memory_block()is used to calculate thestart_section_nrof this block, which means any section in the same block would get the samestart_section_nr.This patch passes the
base_sectiontoinit_memory_block(), so that to reduce a local variable and a check in every loop.Signed-off-by Wei Yang <[email protected]>---
drivers/base/memory.c| 7 +++----1 file changed, 3 insertions(+), 4 deletions(-)drivers/base/memory.c +3
-4@@ -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
Wei Yang Help you, Help me
Quoted 11 lines · Wei Yang
Hi, Michael
I copied your reply here:
Quoted 7 lines
[Sorry for a late response]
Quoted 3 lines · Wei Yang
The second parameter of
init_memory_block()is used to calculate thestart_section_nrof this block, which means any section in the same block would get the samestart_section_nr.
Quoted 5 lines
Quoted 2 lines
Could you be more specific what is the problem here?
There is no problem in this code. I just find a unnecessary calculation and remove it in this patch.
This code needs a larger rething rather than here and there small changes I believe.
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
This patch passes the
base_sectiontoinit_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
I don't see the situation you pointed here.
Quoted 8 lines
In
add_memory_block(),section_nris used to record the first section which is present. And this variable is used to calculate the section which is passed toinit_memory_block().In
init_memory_block(), the section got fromadd_memory_block(), is used to calculatescn_nr, but finally transformed to "start_section_nr". That means ininit_memory_block(), we just need the "start_section_nr" of amemory_block. We don't care about who is the first present section.
You are right. The code is confusing as hell!
That being said, I am not opposing the patch but I would much rather appreciate a consistent cleanup in the whole memblock vs. sections area. That would be a larger project but the end result is really worth it.
Signature · 2 lines
Michal Hocko SUSE Labs