| Message ID | 20200311011823.27740-1-bhe@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] x86/mm: Remove the redundant conditional check | expand |
On Wed 11-03-20 09:18:23, Baoquan He wrote: > In commit f70029bbaacb ("mm, memory_hotplug: drop CONFIG_MOVABLE_NODE"), > the dependency on CONFIG_MOVABLE_NODE was removed for N_MEMORY. Before > commit f70029bbaacb, CONFIG_HIGHMEM && !CONFIG_MOVABLE_NODE could make > (N_MEMORY == N_NORMAL_MEMORY) be true. After commit f70029bbaacb, N_MEMORY > doesn't have any chance to be equal to N_NORMAL_MEMORY. So the conditional > check in paging_init() doesn't make sense any more. Let's remove it. > > Signed-off-by: Baoquan He <bhe@redhat.com> Acked-by: Michal Hocko <mhocko@suse.com> Thanks! > --- > v1->v2: > Update patch log to make the description clearer per Michal's > suggestion. > > arch/x86/mm/init_64.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c > index abbdecb75fad..0a14711d3a93 100644 > --- a/arch/x86/mm/init_64.c > +++ b/arch/x86/mm/init_64.c > @@ -818,8 +818,7 @@ void __init paging_init(void) > * will not set it back. > */ > node_clear_state(0, N_MEMORY); > - if (N_MEMORY != N_NORMAL_MEMORY) > - node_clear_state(0, N_NORMAL_MEMORY); > + node_clear_state(0, N_NORMAL_MEMORY); > > zone_sizes_init(); > } > -- > 2.17.2
On Wed, Mar 11, 2020 at 09:18:23AM +0800, Baoquan He wrote: >In commit f70029bbaacb ("mm, memory_hotplug: drop CONFIG_MOVABLE_NODE"), >the dependency on CONFIG_MOVABLE_NODE was removed for N_MEMORY. Before >commit f70029bbaacb, CONFIG_HIGHMEM && !CONFIG_MOVABLE_NODE could make >(N_MEMORY == N_NORMAL_MEMORY) be true. After commit f70029bbaacb, N_MEMORY >doesn't have any chance to be equal to N_NORMAL_MEMORY. So the conditional >check in paging_init() doesn't make sense any more. Let's remove it. > >Signed-off-by: Baoquan He <bhe@redhat.com> The change looks good. While I have one question, we set default value for N_HIGH_MEMORY. Why we don't clear this too? Reviewed-by: Wei Yang <richard.weiyang@gmail.com> >--- >v1->v2: > Update patch log to make the description clearer per Michal's > suggestion. > > arch/x86/mm/init_64.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > >diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c >index abbdecb75fad..0a14711d3a93 100644 >--- a/arch/x86/mm/init_64.c >+++ b/arch/x86/mm/init_64.c >@@ -818,8 +818,7 @@ void __init paging_init(void) > * will not set it back. > */ > node_clear_state(0, N_MEMORY); >- if (N_MEMORY != N_NORMAL_MEMORY) >- node_clear_state(0, N_NORMAL_MEMORY); >+ node_clear_state(0, N_NORMAL_MEMORY); > > zone_sizes_init(); > } >-- >2.17.2
On 03/14/20 at 03:10pm, Wei Yang wrote: > On Wed, Mar 11, 2020 at 09:18:23AM +0800, Baoquan He wrote: > >In commit f70029bbaacb ("mm, memory_hotplug: drop CONFIG_MOVABLE_NODE"), > >the dependency on CONFIG_MOVABLE_NODE was removed for N_MEMORY. Before > >commit f70029bbaacb, CONFIG_HIGHMEM && !CONFIG_MOVABLE_NODE could make > >(N_MEMORY == N_NORMAL_MEMORY) be true. After commit f70029bbaacb, N_MEMORY > >doesn't have any chance to be equal to N_NORMAL_MEMORY. So the conditional > >check in paging_init() doesn't make sense any more. Let's remove it. > > > >Signed-off-by: Baoquan He <bhe@redhat.com> > > The change looks good. While I have one question, we set default value for > N_HIGH_MEMORY. Why we don't clear this too? This is for x86_64 only, there's no node_state for N_HIGH_MEMORY. > > Reviewed-by: Wei Yang <richard.weiyang@gmail.com> > > >--- > >v1->v2: > > Update patch log to make the description clearer per Michal's > > suggestion. > > > > arch/x86/mm/init_64.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > >diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c > >index abbdecb75fad..0a14711d3a93 100644 > >--- a/arch/x86/mm/init_64.c > >+++ b/arch/x86/mm/init_64.c > >@@ -818,8 +818,7 @@ void __init paging_init(void) > > * will not set it back. > > */ > > node_clear_state(0, N_MEMORY); > >- if (N_MEMORY != N_NORMAL_MEMORY) > >- node_clear_state(0, N_NORMAL_MEMORY); > >+ node_clear_state(0, N_NORMAL_MEMORY); > > > > zone_sizes_init(); > > } > >-- > >2.17.2 > > -- > Wei Yang > Help you, Help me >
On Sun, Mar 15, 2020 at 08:49:13PM +0800, Baoquan He wrote: >On 03/14/20 at 03:10pm, Wei Yang wrote: >> On Wed, Mar 11, 2020 at 09:18:23AM +0800, Baoquan He wrote: >> >In commit f70029bbaacb ("mm, memory_hotplug: drop CONFIG_MOVABLE_NODE"), >> >the dependency on CONFIG_MOVABLE_NODE was removed for N_MEMORY. Before >> >commit f70029bbaacb, CONFIG_HIGHMEM && !CONFIG_MOVABLE_NODE could make >> >(N_MEMORY == N_NORMAL_MEMORY) be true. After commit f70029bbaacb, N_MEMORY >> >doesn't have any chance to be equal to N_NORMAL_MEMORY. So the conditional >> >check in paging_init() doesn't make sense any more. Let's remove it. >> > >> >Signed-off-by: Baoquan He <bhe@redhat.com> >> >> The change looks good. While I have one question, we set default value for >> N_HIGH_MEMORY. Why we don't clear this too? > >This is for x86_64 only, there's no node_state for N_HIGH_MEMORY. > Thanks, I see it in arch/x86/Kconfig.
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c index abbdecb75fad..0a14711d3a93 100644 --- a/arch/x86/mm/init_64.c +++ b/arch/x86/mm/init_64.c @@ -818,8 +818,7 @@ void __init paging_init(void) * will not set it back. */ node_clear_state(0, N_MEMORY); - if (N_MEMORY != N_NORMAL_MEMORY) - node_clear_state(0, N_NORMAL_MEMORY); + node_clear_state(0, N_NORMAL_MEMORY); zone_sizes_init(); }
In commit f70029bbaacb ("mm, memory_hotplug: drop CONFIG_MOVABLE_NODE"), the dependency on CONFIG_MOVABLE_NODE was removed for N_MEMORY. Before commit f70029bbaacb, CONFIG_HIGHMEM && !CONFIG_MOVABLE_NODE could make (N_MEMORY == N_NORMAL_MEMORY) be true. After commit f70029bbaacb, N_MEMORY doesn't have any chance to be equal to N_NORMAL_MEMORY. So the conditional check in paging_init() doesn't make sense any more. Let's remove it. Signed-off-by: Baoquan He <bhe@redhat.com> --- v1->v2: Update patch log to make the description clearer per Michal's suggestion. arch/x86/mm/init_64.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)