linux-kernel - Re: [PATCH v2 3/3] madvise:madvise_free_pte_range(): don't use mapcount() against large folio for sharing check

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]

Message-ID: <ea478fa8-655b-90cc-0cd3-5f127e7e68b7@redhat.com>
Date:   Tue, 15 Aug 2023 15:41:26 +0200
From:   David Hildenbrand <david@...hat.com>
To:     Daniel Gomez <da.gomez@...sung.com>,
        Yin Fengwei <fengwei.yin@...el.com>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "willy@...radead.org" <willy@...radead.org>,
        "vishal.moola@...il.com" <vishal.moola@...il.com>,
        "wangkefeng.wang@...wei.com" <wangkefeng.wang@...wei.com>,
        "minchan@...nel.org" <minchan@...nel.org>,
        "yuzhao@...gle.com" <yuzhao@...gle.com>,
        "ryan.roberts@....com" <ryan.roberts@....com>,
        "shy828301@...il.com" <shy828301@...il.com>
Subject: Re: [PATCH v2 3/3] madvise:madvise_free_pte_range(): don't use
 mapcount() against large folio for sharing check

On 15.08.23 15:25, Daniel Gomez wrote:
> Hi Yin,
> On Tue, Aug 08, 2023 at 10:09:17AM +0800, Yin Fengwei wrote:
>> Commit 98b211d6415f ("madvise: convert madvise_free_pte_range() to use a
>> folio") replaced the page_mapcount() with folio_mapcount() to check
>> whether the folio is shared by other mapping.
>>
>> It's not correct for large folios. folio_mapcount() returns the total
>> mapcount of large folio which is not suitable to detect whether the folio
>> is shared.
>>
>> Use folio_estimated_sharers() which returns a estimated number of shares.
>> That means it's not 100% correct. It should be OK for madvise case here.
> 
> I'm trying to understand why it should be ok for madvise this change, so
> I hope it's okay to ask you few questions.
> 
> folio_mapcount() calculates the total maps for all the subpages of a
> folio. However, the folio_estimated_sharers does it only for the first
> subpage making it not true for large folios. Then, wouldn't this change
> drop support for large folios?

It's all a mess right now.

1) page_mapcount(page): how often it this page mapped

For a THP: entire mapcount of the THP (PMD-mapping) + mapcount of *this 
very subpage* (PTE-mapping) only


2) folio_mapcount(): how often is this folio mapped

For a THP: entire mapcount of the THP (PMD-mapping) + mapcount of *all* 
subpages (PTE-mapping) of the folio


3) folio_estimated_sharers(): how often is the first page mapped

For a THP: entire mapcount of the THP (PMD-mapping) + mapcount of *the 
first subpage* (PTE-mapping) only


For the time being, folio_estimated_sharers() is better then 
folio_mapcount(), because for a PTE-mapped THP folio_mapcount() > 1.

I'm looking into a replacement for folio_estimated_sharers() that is 
more precise ("folio_mapped_shared()"), but it's all a bit tricky. :)

-- 
Cheers,

David / dhildenb