[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fba0b342-dbad-4a60-9b64-0e2b2824d741@redhat.com>
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
Powered by blists - more mailing lists