* [RESEND PATCH] base/memory: pass the base_section in add_memory_block
@ 2017-06-14 5:45 Wei Yang
2017-06-14 5:59 ` Michal Hocko
2017-06-14 6:05 ` Wei Yang
0 siblings, 2 replies; 6+ messages in thread
From: Wei Yang @ 2017-06-14 5:45 UTC (permalink / raw)
To: gregkh, mhocko; +Cc: linux-kernel, linux-mm, 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
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@gmail.com>
---
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
--
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH] base/memory: pass the base_section in add_memory_block
2017-06-14 5:45 [RESEND PATCH] base/memory: pass the base_section in add_memory_block Wei Yang
@ 2017-06-14 5:59 ` Michal Hocko
2017-06-14 6:19 ` Wei Yang
2017-06-14 6:05 ` Wei Yang
1 sibling, 1 reply; 6+ messages in thread
From: Michal Hocko @ 2017-06-14 5:59 UTC (permalink / raw)
To: Wei Yang; +Cc: gregkh, linux-kernel, linux-mm
On Wed 14-06-17 13:45:50, 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
I have already given you feedback
http://lkml.kernel.org/r/20170613114842.GI10819@dhcp22.suse.cz
and you seemed to ignore it completely.
> 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@gmail.com>
> ---
> 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
--
Michal Hocko
SUSE Labs
--
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH] base/memory: pass the base_section in add_memory_block
2017-06-14 5:45 [RESEND PATCH] base/memory: pass the base_section in add_memory_block Wei Yang
2017-06-14 5:59 ` Michal Hocko
@ 2017-06-14 6:05 ` Wei Yang
2017-06-14 6:30 ` Michal Hocko
1 sibling, 1 reply; 6+ messages in thread
From: Wei Yang @ 2017-06-14 6:05 UTC (permalink / raw)
To: Wei Yang; +Cc: gregkh, mhocko, linux-kernel, linux-mm
[-- Attachment #1: Type: text/plain, Size: 3680 bytes --]
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@gmail.com>
>---
> 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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH] base/memory: pass the base_section in add_memory_block
2017-06-14 5:59 ` Michal Hocko
@ 2017-06-14 6:19 ` Wei Yang
2017-06-14 6:22 ` Michal Hocko
0 siblings, 1 reply; 6+ messages in thread
From: Wei Yang @ 2017-06-14 6:19 UTC (permalink / raw)
To: Michal Hocko; +Cc: Wei Yang, gregkh, linux-kernel, linux-mm
[-- Attachment #1: Type: text/plain, Size: 2114 bytes --]
On Wed, Jun 14, 2017 at 07:59:25AM +0200, Michal Hocko wrote:
>On Wed 14-06-17 13:45:50, 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
>
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.
>I have already given you feedback
>http://lkml.kernel.org/r/20170613114842.GI10819@dhcp22.suse.cz
>and you seemed to ignore it completely.
>
>> 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@gmail.com>
>> ---
>> 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
>
>--
>Michal Hocko
>SUSE Labs
--
Wei Yang
Help you, Help me
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH] base/memory: pass the base_section in add_memory_block
2017-06-14 6:19 ` Wei Yang
@ 2017-06-14 6:22 ` Michal Hocko
0 siblings, 0 replies; 6+ messages in thread
From: Michal Hocko @ 2017-06-14 6:22 UTC (permalink / raw)
To: Wei Yang; +Cc: gregkh, linux-kernel, linux-mm
On Wed 14-06-17 14:19:59, Wei Yang wrote:
> On Wed, Jun 14, 2017 at 07:59:25AM +0200, Michal Hocko wrote:
> >On Wed 14-06-17 13:45:50, 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
> >
>
> 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.
You could add linux-mm to the cc in the response to that email
--
Michal Hocko
SUSE Labs
--
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH] base/memory: pass the base_section in add_memory_block
2017-06-14 6:05 ` Wei Yang
@ 2017-06-14 6:30 ` Michal Hocko
0 siblings, 0 replies; 6+ messages in thread
From: Michal Hocko @ 2017-06-14 6:30 UTC (permalink / raw)
To: Wei Yang; +Cc: gregkh, linux-kernel, linux-mm
On Wed 14-06-17 14:05:58, Wei Yang wrote:
> 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 code needs a larger rething rather than here and there small
changes I believe.
> >> 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.
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.
--
Michal Hocko
SUSE Labs
--
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-06-14 6:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-14 5:45 [RESEND PATCH] base/memory: pass the base_section in add_memory_block Wei Yang
2017-06-14 5:59 ` Michal Hocko
2017-06-14 6:19 ` Wei Yang
2017-06-14 6:22 ` Michal Hocko
2017-06-14 6:05 ` Wei Yang
2017-06-14 6:30 ` Michal Hocko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox