Re: [RFC V1 00/31] mm/mmap: Drop protection_map[] and platform's __SXXX/__PXXX requirements

[Anshuman Khandual <anshuman.khandual@arm.com>]

On 1/27/22 6:08 PM, Mike Rapoport wrote:
> Hi Anshuman,
> 
> On Mon, Jan 24, 2022 at 06:26:37PM +0530, Anshuman Khandual wrote:
>> protection_map[] is an array based construct that translates given vm_flags
>> combination. This array contains page protection map, which is populated by
>> the platform via [__S000 .. __S111] and [__P000 .. __P111] exported macros.
>> Primary usage for protection_map[] is for vm_get_page_prot(), which is used
>> to determine page protection value for a given vm_flags. vm_get_page_prot()
>> implementation, could again call platform overrides arch_vm_get_page_prot()
>> and arch_filter_pgprot(). Some platforms override protection_map[] that was
>> originally built with __SXXX/__PXXX with different runtime values.
>>
>> Currently there are multiple layers of abstraction i.e __SXXX/__PXXX macros
>> , protection_map[], arch_vm_get_page_prot() and arch_filter_pgprot() built
>> between the platform and generic MM, finally defining vm_get_page_prot().
>>
>> Hence this series proposes to drop all these abstraction levels and instead
>> just move the responsibility of defining vm_get_page_prot() to the platform
>> itself making it clean and simple.
>>
>> This first introduces ARCH_HAS_VM_GET_PAGE_PROT which enables the platforms
>> to define custom vm_get_page_prot(). This starts converting platforms that
>> either change protection_map[] or define the overrides arch_filter_pgprot()
>> or arch_vm_get_page_prot() which enables for those constructs to be dropped
>> off completely. This series then converts remaining platforms which enables
>> for __SXXX/__PXXX constructs to be dropped off completely. Finally it drops
>> the generic vm_get_page_prot() and then ARCH_HAS_VM_GET_PAGE_PROT as every
>> platform now defines their own vm_get_page_prot().
>
> I generally like the idea, I just think the conversion can be more straight
> forward. Rather than adding ARCH_HAS_VM_GET_PAGE_PROT and then dropping it,
> why won't me make the generic vm_get_page_prot() __weak, then add per-arch
> implementation and in the end drop the generic one?
>  

Is not the ARCH_HAS_ config based switch over a relatively better method ?
IIUC some existing platform overrides could have been implemented via __weak
identifier, although ARCH_HAS_ method was preferred. But I might be missing
something here.