x86/mm: Remove the redundant conditional check
Subsystems (3)
PATCH 1/1 x86/mm: Remove the redundant conditional check
In commit f70029bbaacbfa8f0 ("mm, memory_hotplug: drop CONFIG_MOVABLE_NODE"), the dependency on CONFIG_MOVABLE_NODE was removed for N_MEMORY, so the conditional check in paging_init() doesn't make any sense any more. Remove it.
--- arch/x86/mm/init_64.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
arch/x86/mm/init_64.c +1 -2
@@ -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 (); }
Signature · 1 line
2.17.2
Quoted 4 lines · Baoquan He
In commit
f70029bbaacbfa8f0("mm,memory_hotplug: dropCONFIG_MOVABLE_NODE"), the dependency onCONFIG_MOVABLE_NODEwas removed forN_MEMORY, so the conditional check inpaging_init()doesn't make any sense any more. Remove it.
Please expand more. I would really have to refresh the intention of the code but from a quick look at the code CONFIG_HIGHMEM still makes N_MEMORY != N_NORMAL_MEMORY. So what what does this change mean for that config?
Quoted 22 lines
Signed-off-by Baoquan He <[email protected]>--- arch/x86/mm/init_64.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)arch/x86/mm/init_64.c +1
-2@@ -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 (); }Signature · 1 line
2.17.2
Signature · 2 lines
Michal Hocko SUSE Labs
Quoted 10 lines · Michal Hocko
Quoted 4 lines · Baoquan He
In commit
f70029bbaacbfa8f0("mm,memory_hotplug: dropCONFIG_MOVABLE_NODE"), the dependency onCONFIG_MOVABLE_NODEwas removed forN_MEMORY, so the conditional check inpaging_init()doesn't make any sense any more. Remove it.Please expand more. I would really have to refresh the intention of the code but from a quick look at the code
CONFIG_HIGHMEMstill makesN_MEMORY!=N_NORMAL_MEMORY. So what what does this change mean for that config?
Thanks for looking into this. I was trying to explain that CONFIG_MOVABLE_NODE made N_MEMORY have chance to take different enum value.
Do you think the below saying is OK to you?
~~~
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 any sense any more. Let's remove it.
~~~
Thanks
Baoquan
Quoted 18 lines · Baoquan He
Quoted 10 lines · Michal Hocko
Quoted 4 lines · Baoquan He
In commit
f70029bbaacbfa8f0("mm,memory_hotplug: dropCONFIG_MOVABLE_NODE"), the dependency onCONFIG_MOVABLE_NODEwas removed forN_MEMORY, so the conditional check inpaging_init()doesn't make any sense any more. Remove it.Please expand more. I would really have to refresh the intention of the code but from a quick look at the code
CONFIG_HIGHMEMstill makesN_MEMORY!=N_NORMAL_MEMORY. So what what does this change mean for that config?Thanks for looking into this. I was trying to explain that
CONFIG_MOVABLE_NODEmadeN_MEMORYhave chance to take different enum value.Do you think the below saying is OK to you?
Quoted 7 lines
~~~
In commitf70029bbaacb("mm,memory_hotplug: dropCONFIG_MOVABLE_NODE"), the dependency onCONFIG_MOVABLE_NODEwas removed forN_MEMORY. Before commitf70029bbaacb,CONFIG_HIGHMEM&& !CONFIG_MOVABLE_NODE could make (N_MEMORY==N_NORMAL_MEMORY) be true. After commitf70029bbaacb,N_MEMORYdoesn't have any chance to be equal toN_NORMAL_MEMORY. So the conditional check inpaging_init()doesn't make any sense any more. Let's remove it.
Yes this describes the matter much better. I have obviously misread the code when looking at it this morning. Being explicit in the changelog would have helped at least me. Thanks!
Signature · 2 lines
Michal Hocko SUSE Labs
Quoted 27 lines · Michal Hocko
Quoted 18 lines · Baoquan He
Quoted 10 lines · Michal Hocko
Quoted 4 lines · Baoquan He
In commit
f70029bbaacbfa8f0("mm,memory_hotplug: dropCONFIG_MOVABLE_NODE"), the dependency onCONFIG_MOVABLE_NODEwas removed forN_MEMORY, so the conditional check inpaging_init()doesn't make any sense any more. Remove it.Please expand more. I would really have to refresh the intention of the code but from a quick look at the code
CONFIG_HIGHMEMstill makesN_MEMORY!=N_NORMAL_MEMORY. So what what does this change mean for that config?Thanks for looking into this. I was trying to explain that
CONFIG_MOVABLE_NODEmadeN_MEMORYhave chance to take different enum value.Do you think the below saying is OK to you?
Quoted 7 lines
~~~
In commitf70029bbaacb("mm,memory_hotplug: dropCONFIG_MOVABLE_NODE"), the dependency onCONFIG_MOVABLE_NODEwas removed forN_MEMORY. Before commitf70029bbaacb,CONFIG_HIGHMEM&& !CONFIG_MOVABLE_NODE could make (N_MEMORY==N_NORMAL_MEMORY) be true. After commitf70029bbaacb,N_MEMORYdoesn't have any chance to be equal toN_NORMAL_MEMORY. So the conditional check inpaging_init()doesn't make any sense any more. Let's remove it.
Quoted 3 lines
Yes this describes the matter much better. I have obviously misread the code when looking at it this morning. Being explicit in the changelog would have helped at least me. Thanks!
Will update log and repost, thanks.