... adding Kirill

On 8/7/20 1:40 AM, Joerg Roedel wrote:
> +             lvl = "p4d";
> +             p4d = p4d_alloc(&init_mm, pgd, addr);
> +             if (!p4d)
> +                     goto failed;
>  
> +             /*
> +              * With 5-level paging the P4D level is not folded. So the PGDs
> +              * are now populated and there is no need to walk down to the
> +              * PUD level.
> +              */
>               if (pgtable_l5_enabled())
>                       continue;

It's early and I'm a coffee or two short of awake, but I had to stare at
the comment for a but to make sense of it.

It feels wrong, I think, because the 5-level code usually ends up doing
*more* allocations and in this case, it is _appearing_ to do fewer.
Would something like this make sense?

                /*
                 * The goal here is to allocate all possibly required
                 * hardware page tables pointed to by the top hardware
                 * level.
                 *
                 * On 4-level systems, the p4d layer is folded away and
                 * the above code does no preallocation.  Below, go down
                 * to the pud _software_ level to ensure the second
                 * hardware level is allocated.
                 */


> -             pud = pud_offset(p4d, addr);
> -             if (pud_none(*pud)) {
> -                     /* Ends up here only with 4-level paging */
> -                     pud = pud_alloc(&init_mm, p4d, addr);
> -                     if (!pud) {
> -                             lvl = "pud";
> -                             goto failed;
> -                     }
> -             }
> +             lvl = "pud";
> +             pud = pud_alloc(&init_mm, p4d, addr);
> +             if (!pud)
> +                     goto failed;
>       }

Reply via email to