Messages in this thread | | | Subject | Re: [PATCH 09/18] ARC: mm: non-functional code cleanup ahead of 3 levels | From | Vineet Gupta <> | Date | Thu, 12 Aug 2021 11:58:03 -0700 |
| |
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
| |