[PATCH 09/18] ARC: mm: non-functional code cleanup ahead of 3 levels

[Vineet Gupta]

On 8/11/21 11:18 PM, Mike Rapoport wrote:
> On Wed, Aug 11, 2021 at 06:37:19PM -0700, Vineet Gupta wrote:
>> On 8/11/21 5:31 AM, Mike Rapoport wrote:
>>>> +/*
>>>> + * For ARC, pgtable_t is not struct page *, but pte_t * (to avoid
>>>> + * extraneous page_address() calculations) hence can't use
>>>> + * use asm-generic/pgalloc.h which assumes it being struct page *
>>>> + */
>>> Another reason to leave ARC without asm-generic/pgalloc.h was
>>> __get_order_pte() that other arches don't have.
>>> So this and pgtable_t aliased to pte_t * seemed to me too much to bother
>>> then, but probably it's worth reconsidering with addition of 3rd and 4th
>>> levels.
>> I agree that savings of not havign page_address() might not be huge. However
>> asm-generic/pgalloc.h only has pte allocation routines and all other
>> allocation levels come from arch file
> asm-generic/pgalloc.h has allocation routines up to PUD.
> There is also pgtable_pmd_page_ctor() and pgtable_pmd_page_dtor() called in
> the generic versions of PMD allocation, it seems they are not called in ARC
> implementation.

:-(

> So using asm-generic/pgalloc.h would probably save you some THP debugging ;-)
>
> We may even probably accommodate multi-page PTEs in asm-generic/pgalloc.h
> with something like
>
> #ifndef __HAVE_ARCH_PTE_GET_ORDER
> static inline int __pte_get_order(void)
> {
> 	return 0;
> }
> #endif

Not needed - those cases are unreal, esoteric at best. I'm working on 
switching back to canonical struct page based pgtable_t, will do that in v2.

>> Also for ARCv2, given the arbitrary address split and ensuing paging levels
>> (as well as support for different page sizes) we will need to make sure that
>> one page is enough to hold any level's paging using say BUILD_BUG_ON. In
>> fact that should also be done for 3rd and 4th levels for sanity.
> Right, these sanity checks would be useful, but they may live in one of .c
> files in arch/arc/mm.

Sure !

Thx,
-Vineet