Re: [PATCH 2/3] mm: thp: Fix the update_mmu_cache() last argumentpassing in mm/huge_memory.c

[Ralf Baechle]

On Wed, Sep 19, 2012 at 10:12:28AM +0100, Catalin Marinas wrote:

> >> 5) void update_mmu_cache(struct vm_area_struct *vma,
> >>                          unsigned long address, pte_t *ptep)
> >
> > Yes please.
> 
> Should we just use a generic (void *) for the last argument or force a
> cast in mm/huge_memory.c?
> 
> Ralf's point is that transparent huge page code calls update_mmu_cache
> with a (pmd_t *) as the last argument. This could make sense for THP
> as it assumes that huge pages can only be created at the pmd level.
> But that's unlike mm/hugetlb.c which casts huge page types to pte_t,
> even though on ARM they are implemented at the pmd level.
> 
> On ARM (with VIPT caches) update_mmu_cache() is empty like on x86,
> though a static inline rather than macro.

It's even worse - mm/huge_memory.c is passing a pmd_t, not a pointer so
changing the type of update_mmu_cache's 3rd argument alone won't cut it.

This went unnoticed so far because all existing architectures supporting
transparent huge pages implement update_mmu_cache() as do { } while (0).

MIPS uses update_mmu_cache() as the hook to deal with cache aliases and
pre-faulting a TLB entry.  But aliases don't affect small pages and having
a separate variant of update_mmu_cache for huge pages will allow some other
optimizations.  That's minor but it's the argument types that really need
to be fixed and because MIPS also implements huge pages at PMD level I'd
be happy if we settle on pte_t * for mm/huge_memory.c.

  Ralf
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/