Messages in this thread | | | Date | Mon, 20 Jul 2009 11:07:29 +0800 | From | Zhaolei <> | Subject | Re: Re: [PATCH] Remove initialization of page_table in one_page_table_init() |
| |
Ingo Molnar wrote: > * Zhaolei <zhaolei@cn.fujitsu.com> wrote: > >> Only judge if (!page_table) when CONFIG_DEBUG_PAGEALLOC or CONFIG_KMEMCHECK >> are defined, then we don't need to set init value for page_table. >> >> Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com> >> --- >> arch/x86/mm/init_32.c | 4 ++-- >> 1 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c >> index 3cd7711..55da09b 100644 >> --- a/arch/x86/mm/init_32.c >> +++ b/arch/x86/mm/init_32.c >> @@ -108,13 +108,13 @@ static pmd_t * __init one_md_table_init(pgd_t *pgd) >> static pte_t * __init one_page_table_init(pmd_t *pmd) >> { >> if (!(pmd_val(*pmd) & _PAGE_PRESENT)) { >> - pte_t *page_table = NULL; >> + pte_t *page_table; >> >> if (after_bootmem) { >> #if defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_KMEMCHECK) >> page_table = (pte_t *) alloc_bootmem_pages(PAGE_SIZE); >> -#endif >> if (!page_table) >> +#endif >> page_table = >> (pte_t *)alloc_bootmem_low_pages(PAGE_SIZE); > > I think the original code was cleaner - putting an #ifdef straight > into the middle of a control block is quite fragile. > > Ingo >
Hello, Ingo
David Rientjes said that: Nack, gcc will already optimize the branch out for configs without CONFIG_DEBUG_PAGEALLOC and CONFIG_KMEMCHECK. The only thing this change does is make the code less readable.
I tested it with gcc -O2, and proved that David is correct. Initialization of page_table and additional "if (!page_table)" was removed by optimization. Please ignore this patch.
Thanks Zhaolei
| |