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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <f77ac5df-c92c-321a-7624-35520b386f6f@kernel.org>
Date:   Thu, 12 Aug 2021 11:58:03 -0700
From:   Vineet Gupta <vgupta@...nel.org>
To:     Mike Rapoport <rppt@...nel.org>
Cc:     linux-snps-arc@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-mm@...ck.org, Anshuman Khandual <anshuman.khandual@....com>
Subject: Re: [PATCH 09/18] ARC: mm: non-functional code cleanup ahead of 3
 levels

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