Messages in this thread | | | Date | Mon, 20 Jul 2009 13:50:03 +0200 | From | Ingo Molnar <> | Subject | Re: Re: [PATCH] Remove initialization of page_table in one_page_table_init() |
| |
* Zhaolei <zhaolei@cn.fujitsu.com> wrote:
> 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.
Yeah - and note that even if GCC _didnt_ optimize it out, the patch was not acceptable in this form.
Ingo
| |