Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <6fe2dd45-d634-facc-bb8a-3052ab463abb@redhat.com>
Date:   Thu, 3 Aug 2023 16:21:41 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Content-Language: en-US
To:     Ryan Roberts <ryan.roberts@arm.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Matthew Wilcox <willy@infradead.org>,
        Yin Fengwei <fengwei.yin@intel.com>,
        Yu Zhao <yuzhao@google.com>, Yang Shi <shy828301@gmail.com>,
        "Huang, Ying" <ying.huang@intel.com>, Zi Yan <ziy@nvidia.com>,
        Nathan Chancellor <nathan@kernel.org>,
        Alexander Gordeev <agordeev@linux.ibm.com>,
        Gerald Schaefer <gerald.schaefer@linux.ibm.com>
Cc:     linux-kernel@vger.kernel.org, linux-mm@kvack.org
References: <20230727141837.3386072-1-ryan.roberts@arm.com>
 <20230727141837.3386072-4-ryan.roberts@arm.com>
 <6cda91b3-bb7a-4c4c-a618-2572b9c8bbf9@redhat.com>
 <4255e71a-63c9-b2f9-5e97-e46834f7837c@arm.com>
 <b54e7885-3e49-150f-cf8a-36a880e5dfc9@redhat.com>
 <bb871b52-526a-d4aa-5249-6105bc06aaba@arm.com>
From:   David Hildenbrand <david@redhat.com>
Organization: Red Hat
Subject: Re: [PATCH v4 3/3] mm: Batch-zap large anonymous folio PTE mappings
In-Reply-To: <bb871b52-526a-d4aa-5249-6105bc06aaba@arm.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 03.08.23 16:15, Ryan Roberts wrote:
> On 03/08/2023 15:10, David Hildenbrand wrote:
>>>>
>>>> With this patch, you'll might suddenly have mapcount > refcount for a folio, or
>>>> am I wrong?
>>>
>>> Yes you would. Does that break things?
>>>
>>
>> It is problematic whenever you want to check for additional page references that
>> are not from mappings (i.e., GUP refs/pins or anything else)
>>
>> One example lives in KSM code (!compound only):
>>
>> page_mapcount(page) + 1 + swapped != page_count(page)
>>
>> Another one in compaction code:
>>
>> if (!mapping && (folio_ref_count(folio) - 1) > folio_mapcount(folio))
>>
>> And another one in khugepaged (is_refcount_suitable)
>>
>> ... and in THP split can_split_folio() (although that can deal with false
>> positives and false negatives).
>>
>>
>> We want to avoid detecting "no other references" if there *are* other
>> references. Detecting "there are other references" although there are not is
>> usually better.
>>
>>
>> Assume you have mapcount > refcount for some time due to concurrent unmapping,
>> AND some unrelated reference. You would suddenly pass these checks (mapcount ==
>> refcount) and might not detect other references.
> 
> OK. I'll rework with the 2 loop approach, assuming I can calculate the number of
> free slots in the mmu_gather ahead of time.

Note that I think some of these checks might be racy in corner cases 
(and we should most probably make them more reliable by enforcing the 
memory order -- especially a single atomic total_mapcount might help).

But for now, at least the idea that seems to work is that

a) When you map the page, you first increment the refcount, then the 
mapcount

b) When you unmap a page, you first decrement the mapcount, then the 
refcount

So refcount >= mapcount should in theory always hold when taking a 
snapshot of both values. Although if the actual sides that check for 
these additional references might deserve some fine-tuning.

-- 
Cheers,

David / dhildenb