Hi, Michael I copied your reply here: >[Sorry for a late response] > >On Wed 07-06-17 16:52:12, Wei Yang wrote: >> 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? > There is no problem in this code. I just find a unnecessary calculation and remove it in this patch. >> 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 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. >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. > >Thanks > On Wed, Jun 14, 2017 at 01:45:50PM +0800, Wei Yang wrote: >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. > >Signed-off-by: Wei Yang <richard.weiyang@xxxxxxxxx> >--- > drivers/base/memory.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > >diff --git a/drivers/base/memory.c b/drivers/base/memory.c >index cc4f1d0cbffe..1e903aba2aa1 100644 >--- a/drivers/base/memory.c >+++ b/drivers/base/memory.c >@@ -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; >-- >2.11.0 -- Wei Yang Help you, Help me
Attachment:
signature.asc
Description: PGP signature
