On Tue, 2015-05-05 at 13:19 +0200, Borislav Petkov wrote:
> On Tue, Mar 24, 2015 at 04:08:35PM -0600, Toshi Kani wrote:
> > Document the return values of KVA mapping functions,
>
> KVA?
> Please write it out.
Will expand it as Kernel Virtual Address.
> > pud_set_huge(), pmd_set_huge, pud_clear_huge() and
> > pmd_clear_huge().
> >
> > Simplify the conditions to select HAVE_ARCH_HUGE_VMAP
> > in the Kconfig, since X86_PAE depends on X86_32.
> >
> > There is no functional change in this patch.
> >
> > Signed-off-by: Toshi Kani <toshi.kani@xxxxxx>
> > ---
> > arch/x86/Kconfig | 2 +-
> > arch/x86/mm/pgtable.c | 36 ++++++++++++++++++++++++++++--------
> > 2 files changed, 29 insertions(+), 9 deletions(-)
> >
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index cb23206..2ea27da 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -99,7 +99,7 @@ config X86
> > select IRQ_FORCED_THREADING
> > select HAVE_BPF_JIT if X86_64
> > select HAVE_ARCH_TRANSPARENT_HUGEPAGE
> > - select HAVE_ARCH_HUGE_VMAP if X86_64 || (X86_32 && X86_PAE)
> > + select HAVE_ARCH_HUGE_VMAP if X86_64 || X86_PAE
> > select ARCH_HAS_SG_CHAIN
> > select CLKEVT_I8253
> > select ARCH_HAVE_NMI_SAFE_CMPXCHG
>
> This is an unrelated change, please carve it out in a separate patch.
Will do.
> > diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
> > index 0b97d2c..4891fa1 100644
> > --- a/arch/x86/mm/pgtable.c
> > +++ b/arch/x86/mm/pgtable.c
> > @@ -563,14 +563,19 @@ void native_set_fixmap(enum fixed_addresses idx, phys_addr_t phys,
> > }
> >
> > #ifdef CONFIG_HAVE_ARCH_HUGE_VMAP
> > +/**
> > + * pud_set_huge - setup kernel PUD mapping
> > + *
> > + * MTRR can override PAT memory types with 4KB granularity. Therefore,
> > + * it does not set up a huge page when the range is covered by a non-WB
>
> "it" is what exactly?
Will change to "this function".
> > + * type of MTRR. 0xFF indicates that MTRR are disabled.
>
> So this shows that this patch shouldn't be the first one in the series.
>
> IMO you want to start with cleaning up mtrr_type_lookup(), add the
> defines for its retval and *then* document its users. This way you won't
> have to touch the same place twice, the net-size of your patchset will
> go down and it will be easier for reviewiers.
Agreed. This patch-set was originally a small set of patches, but was
extended later with additional patches, which ended up with touching the
same place again. I will reorganize the patch-set.
> > + *
> > + * Return 1 on success, and 0 when no PUD was set.
>
> "Returns 1 on success and 0 on failure."
Will do.
> > + */
> > int pud_set_huge(pud_t *pud, phys_addr_t addr, pgprot_t prot)
> > {
> > u8 mtrr;
> >
> > - /*
> > - * Do not use a huge page when the range is covered by non-WB type
> > - * of MTRRs.
> > - */
> > mtrr = mtrr_type_lookup(addr, addr + PUD_SIZE);
> > if ((mtrr != MTRR_TYPE_WRBACK) && (mtrr != 0xFF))
> > return 0;
>
> Ditto for the rest.
Will do.
Thanks,
-Toshi
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>