Re: [RFC PATCH 2/2] rmap: remove parameter 'compound' from foeio_add_file_rmap_range()

["Yin, Fengwei" <fengwei.yin@intel.com>]

On 2/7/2023 1:03 PM, Matthew Wilcox wrote:
> On Tue, Feb 07, 2023 at 10:44:10AM +0800, Yin, Fengwei wrote:
>> On 2/7/2023 12:50 AM, Matthew Wilcox wrote:
>>> On Mon, Feb 06, 2023 at 11:30:49PM +0800, Yin Fengwei wrote:
>>>> Remove parameter 'compound' from folio_add_file_rmap_range().
>>>>
>>>> The parameter nr_pages is checked whether same as folio page
>>>> numbers to know whether it's entire folio operation.
>>>
>>> We can't do this yet.  Even if a folio is large enough to be PMD mapped,
>>> it may have been mapped askew (or its PMD mapping may have been split).
>>> We still need the caller to tell us whether it's been mapped with
>>> a PMD or with PTEs.
>> OK. My understand is that the info about PMD mapped is only for
>> statistic update. Maybe move the PMD mapped statistic to caller.
> 
> Alas, no.  That 'compound' parameter really means "to be mapped by a
> PMD".  And we need to change all of add_file, add_anon and remove
> at the same time, otherwise we don't know which counter to inc/dec
> in page_remove_rmap().
OK. I thought that "compound" could mean "mapping the entire folio"
when I worked on this patch. Thanks a lot for clarification.

Regards
Yin, Fengwei

> 
>> Another thing I am not sure whether it's worthy:
>> What about maintaining total mapcount for folio? So we don't need to
>> query each page mapcount to know it. So we can use "total_mapoucnt > 
>> mapped" to know whether the folio has at least one page mapped more
>> than once.
>>
>> The payback is that we need update total mapcount when map/unmap
>> the folio.
> 
> Right, there are lots of mapcounts we _could_ maintain.  The important
> thing to understand is who wants to know what.  As I see it, there are
> three important things we need to know:
> 
> 1. Is any page in this folio mapped?
> (everywhere that calls folio_mapped() or page_mapped())
> 
> 2. Is any page in this folio mapped more than once?
> (some madvise calls, migration)
> 
> 3. How many refcounts does the mapcount account for?
> (splitting, compaction)
> 
> Some of the things we use mapcount for today I consider to be unimportant.
> For example, shrink_folio_list() checks to see if there are any PMD
> mappings and will split it if there are not.  This doesn't fit my vision
> of how the VM should work; I believe we should swap the entire folio
> out as a single unit.
> 
> So I don't really want to think about maintaining total_mapcount,
> I want the concept of total_mapcount to go away and just have a single
> mapcount for each folio, instead of this complex mismash of
> entire_mapcount, mapcount and total_mapcount.