lkml.org 
[lkml]   [2011]   [Nov]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH v8 13/16] ARM: LPAE: Add identity mapping support for the 3-level page table format
    On Mon, Nov 07, 2011 at 04:16:55PM +0000, Catalin Marinas wrote:
    > With LPAE, the pgd is a separate page table with entries pointing to the
    > pmd. The identity_mapping_add() function needs to ensure that the pgd is
    > populated before populating the pmd level. The do..while blocks now loop
    > over the pmd in order to have the same implementation for the two page
    > table formats. The pmd_addr_end() definition has been removed and the
    > generic one used instead. The pmd clean-up is done in the pgd_free()
    > function.
    >
    > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
    > ---
    > arch/arm/mm/idmap.c | 36 ++++++++++++++++++++++++++++++++++--
    > 1 files changed, 34 insertions(+), 2 deletions(-)
    >
    > diff --git a/arch/arm/mm/idmap.c b/arch/arm/mm/idmap.c
    > index 2be9139..24e0655 100644
    > --- a/arch/arm/mm/idmap.c
    > +++ b/arch/arm/mm/idmap.c
    > @@ -1,9 +1,36 @@
    > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
    > +

    I don't like this kind of thing in core files, it doesn't really provide
    much additional inforamtion above the message given.

    > #include <linux/kernel.h>
    >
    > #include <asm/cputype.h>
    > #include <asm/pgalloc.h>
    > #include <asm/pgtable.h>
    >
    > +#ifdef CONFIG_ARM_LPAE
    > +static void idmap_add_pmd(pud_t *pud, unsigned long addr, unsigned long end,
    > + unsigned long prot)
    > +{
    > + pmd_t *pmd;
    > + unsigned long next;
    > +
    > + if (pud_none_or_clear_bad(pud) || (pud_val(*pud) & L_PGD_SWAPPER)) {
    > + pmd = pmd_alloc_one(NULL, addr);

    This should be &init_mm, not NULL.

    > + if (!pmd) {
    > + pr_warning("Failed to allocate identity pmd.\n");

    This message implies its from idmap - "identity pmd".

    > + return;
    > + }
    > + pud_populate(NULL, pud, pmd);

    This should be &init_mm, not NULL.

    > + pmd += pmd_index(addr);
    > + } else
    > + pmd = pmd_offset(pud, addr);
    > +
    > + do {
    > + next = pmd_addr_end(addr, end);
    > + *pmd = __pmd((addr & PMD_MASK) | prot);
    > + flush_pmd_entry(pmd);
    > + } while (pmd++, addr = next, addr != end);
    > +}
    > +#else /* !CONFIG_ARM_LPAE */

    I'm not convinced about the wiseness of this. One of the places where
    this code is called is from the reboot paths, which can happen in atomic
    context. Trying to allocate memory in such a context is going to lead
    to problems, especially if support for panic'ing on OOM, and rebooting
    on panic'ing are both enabled.
    Therefore, I think this patch depends on the work Will has been doing to
    re-structure the kexec and restart support.


    \
     
     \ /
      Last update: 2011-11-10 23:57    [W:4.336 / U:1.188 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site