linux-kernel - Re: [PATCH v1 07/39] mm/rmap: convert folio_add_file_rmap_range() into folio_add_file_rmap

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] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 13 Dec 2023 09:47:33 +0100
From:   David Hildenbrand <david@...hat.com>
To:     Yin Fengwei <fengwei.yin@...el.com>, linux-kernel@...r.kernel.org
Cc:     linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>,
        "Matthew Wilcox (Oracle)" <willy@...radead.org>,
        Hugh Dickins <hughd@...gle.com>,
        Ryan Roberts <ryan.roberts@....com>,
        Mike Kravetz <mike.kravetz@...cle.com>,
        Muchun Song <muchun.song@...ux.dev>,
        Peter Xu <peterx@...hat.com>
Subject: Re: [PATCH v1 07/39] mm/rmap: convert folio_add_file_rmap_range()
 into folio_add_file_rmap_[pte|ptes|pmd]()

On 13.12.23 06:33, Yin Fengwei wrote:
> 
> 
> On 2023/12/11 23:56, David Hildenbrand wrote:
>> Let's get rid of the compound parameter and instead define implicitly
>> which mappings we're adding. That is more future proof, easier to read
>> and harder to mess up.
>>
>> Use an enum to express the granularity internally. Make the compiler
>> always special-case on the granularity by using __always_inline. Replace
>> the "compound" check by a switch-case that will be removed by the
>> compiler completely.
>>
>> Add plenty of sanity checks with CONFIG_DEBUG_VM. Replace the
>> folio_test_pmd_mappable() check by a config check in the caller and
>> sanity checks. Convert the single user of folio_add_file_rmap_range().
>>
>> This function design can later easily be extended to PUDs and to batch
>> PMDs. Note that for now we don't support anything bigger than
>> PMD-sized folios (as we cleanly separated hugetlb handling). Sanity checks
>> will catch if that ever changes.
>>
>> Next up is removing page_remove_rmap() along with its "compound"
>> parameter and smilarly converting all other rmap functions.
>>
>> Signed-off-by: David Hildenbrand <david@...hat.com>
> Reviewed-by: Yin Fengwei <fengwei.yin@...el.com>
> 

Thanks!

> With one small comment.
> 
>> ---
>>    include/linux/rmap.h | 47 +++++++++++++++++++++++++--
>>    mm/memory.c          |  2 +-
>>    mm/rmap.c            | 75 +++++++++++++++++++++++++++++---------------
>>    3 files changed, 95 insertions(+), 29 deletions(-)
>>
>> diff --git a/include/linux/rmap.h b/include/linux/rmap.h
>> index e3857d26b944..1753900f4aed 100644
>> --- a/include/linux/rmap.h
>> +++ b/include/linux/rmap.h
>> @@ -191,6 +191,45 @@ typedef int __bitwise rmap_t;
>>     */
>>    #define RMAP_COMPOUND		((__force rmap_t)BIT(1))
>>    
>> +/*
>> + * Internally, we're using an enum to specify the granularity. Usually,
>> + * we make the compiler create specialized variants for the different
>> + * granularity.
>> + */
>> +enum rmap_mode {
>> +	RMAP_MODE_PTE = 0,
>> +	RMAP_MODE_PMD,
>> +};
> Maybe rmap_level for enum name? To me, PTE and PMD are level instead of
> mode.

Originally, I wanted to call this "enum rmap_granularity", but that 
turned out rather long. Agreed that "level" is better than "mode", 
something resembling "granularity" would be even better.

-- 
Cheers,

David / dhildenb