Re: [PATCH] x86: fix init_memory_mapping over boundary v2

["Yinghai Lu" <yhlu.kernel@gmail.com>]

On Sat, Jun 28, 2008 at 10:38 AM, Jeremy Fitzhardinge <jeremy@goop.org> 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 <yhlu.kernel@gmail.com>
>>
>> ---
>>  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<<PAGE_SHIFT) + PAGE_OFFSET);
>> +       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<<PAGE_SHIFT) + PAGE_OFFSET);
>> +               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...