From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753865AbYF1UdW (ORCPT ); Sat, 28 Jun 2008 16:33:22 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751461AbYF1UdM (ORCPT ); Sat, 28 Jun 2008 16:33:12 -0400 Received: from rv-out-0506.google.com ([209.85.198.234]:9867 "EHLO rv-out-0506.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751456AbYF1UdL (ORCPT ); Sat, 28 Jun 2008 16:33:11 -0400 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:to:subject:cc:in-reply-to:mime-version :content-type:content-transfer-encoding:content-disposition :references; b=pv8KqFMSusY5sEqvs4wmOOdfeR9I25xQI5NH532DzGzcBeP5G74Y2p+iqQ/Lw7hYTB 5QcUIkgjMLBz0dRjcCy/6A966ZMuDimA0/p2/732ayvazIsqQQmH7U0gnYGY32jkaN3e oAUlde/Ibmj4/UX2UCY3Ixv2TjfL6Ht3zbNWs= Message-ID: <86802c440806281333t2bede7d0ld54cceda9a1cfb20@mail.gmail.com> Date: Sat, 28 Jun 2008 13:33:10 -0700 From: "Yinghai Lu" To: "Jeremy Fitzhardinge" Subject: Re: [PATCH] x86: fix init_memory_mapping over boundary v2 Cc: "Ingo Molnar" , "Thomas Gleixner" , "H. Peter Anvin" , "linux-kernel@vger.kernel.org" In-Reply-To: <486676FC.0@goop.org> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <200806251748.06743.yhlu.kernel@gmail.com> <200806252151.29028.yhlu.kernel@gmail.com> <200806271538.46829.yhlu.kernel@gmail.com> <200806272247.11971.yhlu.kernel@gmail.com> <486676FC.0@goop.org> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Jun 28, 2008 at 10:38 AM, Jeremy Fitzhardinge wrote: > Yinghai Lu wrote: >> >> some end boundary is only page alignment, instead of 2M alignment, >> so call ker_phycial_mapping_init three times. >> then don't overmap above the max_low_pfn >> >> v2: make init_memory_mapping more solid: start could be any value other >> than 0 >> >> Signed-off-by: Yinghai Lu >> >> --- >> arch/x86/mm/init_32.c | 86 >> ++++++++++++++++++++++++++++++++++++-------------- >> 1 file changed, 62 insertions(+), 24 deletions(-) >> >> Index: linux-2.6/arch/x86/mm/init_32.c >> =================================================================== >> --- linux-2.6.orig/arch/x86/mm/init_32.c >> +++ linux-2.6/arch/x86/mm/init_32.c >> @@ -184,8 +184,9 @@ static inline int is_kernel_text(unsigne >> * PAGE_OFFSET: >> */ >> static void __init kernel_physical_mapping_init(pgd_t *pgd_base, >> - unsigned long start, >> - unsigned long end) >> + unsigned long start_pfn, >> + unsigned long end_pfn, >> + int use_pse) >> > > Better to use bool for use_pse. > >> { >> int pgd_idx, pmd_idx, pte_ofs; >> unsigned long pfn; >> @@ -193,32 +194,30 @@ static void __init kernel_physical_mappi >> pmd_t *pmd; >> pte_t *pte; >> unsigned pages_2m = 0, pages_4k = 0; >> - unsigned limit_pfn = end >> PAGE_SHIFT; >> - pgd_idx = pgd_index(PAGE_OFFSET); >> - pgd = pgd_base + pgd_idx; >> - pfn = start >> PAGE_SHIFT; >> + if (!cpu_has_pse) >> + use_pse = 0; >> > > What are the semantics of the use_pse argument? Is it just a hint? If so, > then the caller below should just pass a constant 1/true. > >> + pfn = start_pfn; >> + pgd_idx = pgd_index((pfn<> + pgd = pgd_base + pgd_idx; >> for (; pgd_idx < PTRS_PER_PGD; pgd++, pgd_idx++) { >> pmd = one_md_table_init(pgd); >> - if (pfn >= limit_pfn) >> + >> + if (pfn >= end_pfn) >> continue; >> - for (pmd_idx = 0; >> - pmd_idx < PTRS_PER_PMD && pfn < limit_pfn; >> + pmd_idx = pmd_index((pfn<> + pmd += pmd_idx; >> + for (; pmd_idx < PTRS_PER_PMD && pfn < end_pfn; >> pmd++, pmd_idx++) { >> unsigned int addr = pfn * PAGE_SIZE + PAGE_OFFSET; >> /* >> * Map with big pages if possible, otherwise >> * create normal page tables: >> - * >> - * Don't use a large page for the first 2/4MB of >> memory >> - * because there are often fixed size MTRRs in >> there >> - * and overlapping MTRRs into large pages can >> cause >> - * slowdowns. >> */ >> - if (cpu_has_pse && !(pgd_idx == 0 && pmd_idx == >> 0)) { >> + if (use_pse) { >> unsigned int addr2; >> pgprot_t prot = PAGE_KERNEL_LARGE; >> @@ -233,13 +232,12 @@ static void __init kernel_physical_mappi >> set_pmd(pmd, pfn_pmd(pfn, prot)); >> pfn += PTRS_PER_PTE; >> - max_pfn_mapped = pfn; >> continue; >> } >> pte = one_page_table_init(pmd); >> for (pte_ofs = 0; >> - pte_ofs < PTRS_PER_PTE && pfn < max_low_pfn; >> + pte_ofs < PTRS_PER_PTE && pfn < end_pfn; >> pte++, pfn++, pte_ofs++, addr += PAGE_SIZE) { >> pgprot_t prot = PAGE_KERNEL; >> @@ -249,7 +247,6 @@ static void __init kernel_physical_mappi >> pages_4k++; >> set_pte(pte, pfn_pte(pfn, prot)); >> } >> - max_pfn_mapped = pfn; >> } >> } >> update_page_count(PG_LEVEL_2M, pages_2m); >> @@ -729,7 +726,7 @@ void __init setup_bootmem_allocator(void >> static void __init find_early_table_space(unsigned long end) >> { >> - unsigned long puds, pmds, tables, start; >> + unsigned long puds, pmds, ptes, tables, start; >> puds = (end + PUD_SIZE - 1) >> PUD_SHIFT; >> tables = PAGE_ALIGN(puds * sizeof(pud_t)); >> @@ -737,10 +734,15 @@ static void __init find_early_table_spac >> pmds = (end + PMD_SIZE - 1) >> PMD_SHIFT; >> tables += PAGE_ALIGN(pmds * sizeof(pmd_t)); >> - if (!cpu_has_pse) { >> - int ptes = (end + PAGE_SIZE - 1) >> PAGE_SHIFT; >> - tables += PAGE_ALIGN(ptes * sizeof(pte_t)); >> - } >> + if (cpu_has_pse) { >> + int extra; >> + extra = end - ((end>>21) << 21); >> > > That's a bit magic. Isn't that just "extra = end & ((1 << 21) - 1)"? And > what are you actually computing here anyway? > >> + extra += (2<<20); >> > > PMD_SIZE? The comment below says "2/4MB", which corresponds to a > PAE/non-PAE large PMD. > >> + ptes = (extra + PAGE_SIZE - 1) >> PAGE_SHIFT; >> + } else >> + ptes = (end + PAGE_SIZE - 1) >> PAGE_SHIFT; >> + >> + tables += PAGE_ALIGN(ptes * sizeof(pte_t)); >> /* >> * RED-PEN putting page tables only on node 0 could >> @@ -766,6 +768,8 @@ unsigned long __init_refok init_memory_m >> unsigned long end) >> { >> pgd_t *pgd_base = swapper_pg_dir; >> + unsigned long start_pfn, end_pfn; >> + unsigned long big_page_start; >> /* >> * Find space for the kernel direct mapping tables. >> @@ -790,7 +794,41 @@ unsigned long __init_refok init_memory_m >> __PAGE_KERNEL_EXEC |= _PAGE_GLOBAL; >> } >> - kernel_physical_mapping_init(pgd_base, start, end); >> + /* >> + * Don't use a large page for the first 2/4MB of memory >> + * because there are often fixed size MTRRs in there >> + * and overlapping MTRRs into large pages can cause >> + * slowdowns. >> + */ >> + big_page_start = 2UL<<20; >> > > PMD_SIZE again? Non-PAE big pages are 4MB. thanks, I will try to revive v2...