linux-kernel - Re: [PATCH RFC 00/39] mm/rmap: interface overhaul

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:   Tue, 5 Dec 2023 14:55:31 +0100
From:   David Hildenbrand <david@...hat.com>
To:     Ryan Roberts <ryan.roberts@....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>,
        Yin Fengwei <fengwei.yin@...el.com>,
        Mike Kravetz <mike.kravetz@...cle.com>,
        Muchun Song <muchun.song@...ux.dev>,
        Peter Xu <peterx@...hat.com>
Subject: Re: [PATCH RFC 00/39] mm/rmap: interface overhaul


>>>>>>>
>>>> Regarding the contpte, I think it needs more work. Especially, as raised, to not
>>>> degrade
>>>> order-0 performance. Maybe we won't make the next merge window (and you already
>>>> predicated
>>>> that in some cover letter :P ). Let's see.
>>>
>>> Yeah that's ok. I'll do the work to fix the order-0 perf. And also do the same
>>> for patch 2 in that series - would also be really helpful if you had a chance to
>>> look at patch 2 - its new for v3.
>>
>> I only skimmed over it, but it seems to go into the direction we'll need.
>> Keeping order-0 performance unharmed should have highest priority. Hopefully my
>> microbenchmarks are helpful.
> 
> Yes absolutely - are you able to share them??

I shared them in the reply to your patchset. Let me know if you can't 
find them.

[...]

>>> Having now reviewed your series, I have a less strong opinion, perhaps it's
>>> actually best with your original names; "folio" is actually the subject after
>>> all; it's the thing being operated on.
>>>
>>
>> I think having "folio" in there looks cleaner and more consistent to other
>> functions.
>>
>> I tend to like "rmap_dup_file_folio_[pte|ptes|pmd]()", because then we have
>> "file folio" and "anon folio" as one word.
>>
>> But then I wonder about the hugetlb part. Maybe simply
>> "hugetlb_rmap_remove_folio()" etc.
>>
>> Having the "hugetlb_" prefix at the beginning feels like the right thing to do,
>> looking at orher hugetlb special-handlings.
>>
>> But I'll wait a bit until I go crazy on renaming :)
> 
> I suspect we could argue in multiple directions for hours :)

:)

> 
> Let's see if others have opinions.
> 
> FWIW, I've looked through all the patches; I like what I see! This is a really
> nice clean up and will definitely help with the various patch sets I've been
> working on. Apart from the comments I've already raised, looks in pretty good
> shape to me.

Thanks!

-- 
Cheers,

David / dhildenb