From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id A725CC4167B for ; Tue, 5 Dec 2023 12:30:08 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 25CDE6B0082; Tue, 5 Dec 2023 07:30:08 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 20CCB6B0085; Tue, 5 Dec 2023 07:30:08 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0FB9C6B0088; Tue, 5 Dec 2023 07:30:08 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 0036A6B0082 for ; Tue, 5 Dec 2023 07:30:07 -0500 (EST) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id C3E30120165 for ; Tue, 5 Dec 2023 12:30:07 +0000 (UTC) X-FDA: 81532696854.11.F218965 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf05.hostedemail.com (Postfix) with ESMTP id DF872100006 for ; Tue, 5 Dec 2023 12:30:05 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf05.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1701779406; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=mxvthjoSD4fgEtXjY2FBuTHTIxKzN+8U6GSm4iQmJH8=; b=k4ECc6BjYMAR2Lj4K9a7jFA8hvz6xvng1XENSXcuHScrfDfEI884oMe9MzQJz+P7q4OXBb dAiRndd5T5SSyoZrchh2jBNjV85Qos4MzqUMFnxlb230d9uAlWKxVnLMAPJ7zZA3ghOwIF Ti7xBk1kMVSNWPLAfNlM6NHmx7dW0pI= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf05.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1701779406; a=rsa-sha256; cv=none; b=8jZlAFufI9OP9/0yb6y7U9we83/4RkZPIRJ+YR1rFCaFf3qnF7pV5SjX4tvoBu+/YIDUCT HXiuD+8Tts7W6IMRLNz9Te7/YZR/acFRub5qa/9/s+Q/6BcDD+SMJIAMyNa9oQ+UlrVhQj sHT5o6zL9wz/7zKcghT4B27931Eknik= Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id C4F91139F; Tue, 5 Dec 2023 04:30:51 -0800 (PST) Received: from [10.57.73.130] (unknown [10.57.73.130]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 1EC213F5A1; Tue, 5 Dec 2023 04:30:01 -0800 (PST) Message-ID: <33fb5a78-1432-4b43-af62-afeac84cf1ca@arm.com> Date: Tue, 5 Dec 2023 12:29:59 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH RFC 00/39] mm/rmap: interface overhaul Content-Language: en-GB From: Ryan Roberts To: David Hildenbrand , linux-kernel@vger.kernel.org Cc: linux-mm@kvack.org, Andrew Morton , "Matthew Wilcox (Oracle)" , Hugh Dickins , Yin Fengwei , Mike Kravetz , Muchun Song , Peter Xu References: <20231204142146.91437-1-david@redhat.com> <993ea322-8cdb-4ab1-84d3-0a1cb40049c9@arm.com> In-Reply-To: <993ea322-8cdb-4ab1-84d3-0a1cb40049c9@arm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: DF872100006 X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: utrx5ai7hu15rnioxbebtd9tx5suj8rb X-HE-Tag: 1701779405-529226 X-HE-Meta: U2FsdGVkX1/8jerAD8lzU4XsgRrlws9+nO/hdAtdQnZ7dUDNtBUr/3Diir//qSFFEVDOUW6gtCI5thDVQpNpOpfp30ADeRXKB3Kxi1csDzG1hsgQZ4CoIbqgAeET+YttOETvSS06gY7etR+HSGelEQwSUGpHNmWspPJS3CSjbnoPTouxjTN2UtxMp/4oP+wTfdlgM5tFqYRMtkY1oTvu2nBKaYCGteYeUSGx9FaanVS0uYflbcg9DYjKr2weslrlSJ+WFaKgVczU9aPUrR/jflhSVDEdLzl2cxAZsRNj3I1HOvKxpKPL7wqNxiY9ezj0cz7wF0rh517iwrvjiJk3qY91Y6IzNGACRbKJUzJ1C+5VVJCl5yCBgFfjyZKeeeEvrSSOFFaNi0zFpb2ZZevjndp/1vBp6AqiR7de2Ez88vyNDRRzQN1qyAKAaHDAuGY1Tqyda2qgZXViL/3SUY9fe8f7M5riRfOhKtYC8jpXmdOtDAUcAG/4vtp0S6mq3LcX3jVrEwClJCb4pRwOZFR6mYmtYVKyO65Ve5lmIXDPhf2XJ66AoEWav/J3+EPmCyfHvRT8RLH0ec3/plpddMR6zGa8/cqw43t0jWN9ATnrGYWjuCACO17FhLBENawDeYOqsvJ5Z5csFhEW7k4PMZ8z50f9U3PZpy8WvjmF7q/Tf+KbchJsZXgJtyMk1bpNRpV68xV4glagxyeTwFjLSm6Mfloxuj2Pa62eNC4JTKBIvEdba5qfi3G7R0lXcduH/4oK9H3yJOFxOycK3jRT7/R7o2vonsdhg4wwOBfZAbCW+kZpPSGx6IGBFmhtus+WjFMJsR5B+HVpAtB1MP+0mQokjjDEqFEF48mGBOKdSxvdK84b6YuJHXtEOYieUAbM/XxtDmKZ7qRAKy5V+x9sTGw082k1+3vpabtRlAQWgqaX+Pb7WN6LGFsOw6qAcyOE7zhvA57K2xYe0D6w6hwLzsK 0XHBPs1M KkFDPfyrRMBDQxC1w5wH8nMtGZ0GK4vnr8ztMCFpUyPUhpA9ZIIKNPiv7esQ34POWl+R43T4G7EmjtN8NM0ZynaPuzHDaS/w0qloonjPNynVTYbzcSjP7y3c7lA82QT+O0pQzJorbJTXgPzx4qwa4Sbxki40xib1ckc0dkbqFWrbuL65Bppp7g7VlcB/OLPE4Tl+amA/Ltx/YBo5EJSS1hALWNoGaeYvuIl2FKuWEqdbWpqTOAo1T7Qb16D91D4zQyNZSwnKtzRu1WiH6mGGeQWaaP5J9yjCmnTrW0CnkQ8ak+MA2YT6nwSeSMZzpyQci1EEd2gXYxDNwkM2KKAc79Nodxwku0tSNXlN/VSdgE4dMPB4z2c1rkZ5dC8jfUOHu7EEsxkHYiskRyd4p+E3eRe0lGPd80N3nTfoViUhBXlQX/ONj0g8fzWazEExteRU9+vVDVCrBeXfjWkOuNEb44JEQZugjcoQbK2bs X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On 04/12/2023 19:53, Ryan Roberts wrote: > On 04/12/2023 14:21, David Hildenbrand wrote: >> Baed on mm-stable from a couple of days. >> >> This series proposes an overhaul to our rmap interface, to get rid of the >> "bool compound" / RMAP_COMPOUND parameter with the goal of making the >> interface less error prone, more future proof, and more natural to extend >> to "batching". Also, this converts the interface to always consume >> folio+subpage, which speeds up operations on large folios. >> >> Further, this series adds PTE-batching variants for 4 rmap functions, >> whereby only folio_add_anon_rmap_ptes() is used for batching in this series >> when PTE-remapping a PMD-mapped THP. > > I certainly support the objective you have here; making the interfaces clearer, > more consistent and more amenable to batching. I'll try to find some time this > week to review. > >> >> Ryan has series where we would make use of folio_remove_rmap_ptes() [1] >> -- he carries his own batching variant right now -- and >> folio_try_dup_anon_rmap_ptes()/folio_dup_file_rmap_ptes() [2]. > > Note that the contpte series at [2] has a new patch in v3 (patch 2), which could > benefit from folio_remove_rmap_ptes() or equivalent. My plan was to revive [1] > on top of [2] once it is merged. > >> >> There is some overlap with both series (and some other work, like >> multi-size THP [3]), so that will need some coordination, and likely a >> stepwise inclusion. > > Selfishly, I'd really like to get my stuff merged as soon as there is no > technical reason not to. I'd prefer not to add this as a dependency if we can > help it. > >> >> I got that started [4], but it made sense to show the whole picture. The >> patches of [4] are contained in here, with one additional patch added >> ("mm/rmap: introduce and use hugetlb_try_share_anon_rmap()") and some >> slight patch description changes. >> >> In general, RMAP batching is an important optimization for PTE-mapped >> THP, especially once we want to move towards a total mapcount or further, >> as shown with my WIP patches on "mapped shared vs. mapped exclusively" [5]. >> >> The rmap batching part of [5] is also contained here in a slightly reworked >> fork [and I found a bug du to the "compound" parameter handling in these >> patches that should be fixed here :) ]. >> >> This series performs a lot of folio conversion, that could be separated >> if there is a good reason. Most of the added LOC in the diff are only due >> to documentation. >> >> As we're moving to a pte/pmd interface where we clearly express the >> mapping granularity we are dealing with, we first get the remainder of >> hugetlb out of the way, as it is special and expected to remain special: it >> treats everything as a "single logical PTE" and only currently allows >> entire mappings. >> >> Even if we'd ever support partial mappings, I strongly >> assume the interface and implementation will still differ heavily: >> hopefull we can avoid working on subpages/subpage mapcounts completely and >> only add a "count" parameter for them to enable batching. >> >> >> New (extended) hugetlb interface that operate on entire folio: >> * hugetlb_add_new_anon_rmap() -> Already existed >> * hugetlb_add_anon_rmap() -> Already existed >> * hugetlb_try_dup_anon_rmap() >> * hugetlb_try_share_anon_rmap() >> * hugetlb_add_file_rmap() >> * hugetlb_remove_rmap() >> >> New "ordinary" interface for small folios / THP:: >> * folio_add_new_anon_rmap() -> Already existed >> * folio_add_anon_rmap_[pte|ptes|pmd]() >> * folio_try_dup_anon_rmap_[pte|ptes|pmd]() >> * folio_try_share_anon_rmap_[pte|pmd]() >> * folio_add_file_rmap_[pte|ptes|pmd]() >> * folio_dup_file_rmap_[pte|ptes|pmd]() >> * folio_remove_rmap_[pte|ptes|pmd]() > > I'm not sure if there are official guidelines, but personally if we are > reworking the API, I'd take the opportunity to move "rmap" to the front of the > name, rather than having it burried in the middle as it is for some of these: > > rmap_hugetlb_*() > > rmap_folio_*() In fact, I'd be inclined to drop the "folio" to shorten the name; everything is a folio, so its not telling us much. e.g.: New (extended) hugetlb interface that operate on entire folio: * rmap_hugetlb_add_new_anon() -> Already existed * rmap_hugetlb_add_anon() -> Already existed * rmap_hugetlb_try_dup_anon() * rmap_hugetlb_try_share_anon() * rmap_hugetlb_add_file() * rmap_hugetlb_remove() New "ordinary" interface for small folios / THP:: * rmap_add_new_anon() -> Already existed * rmap_add_anon_[pte|ptes|pmd]() * rmap_try_dup_anon_[pte|ptes|pmd]() * rmap_try_share_anon_[pte|pmd]() * rmap_add_file_[pte|ptes|pmd]() * rmap_dup_file_[pte|ptes|pmd]() * rmap_remove_[pte|ptes|pmd]() > > I guess reading the patches will tell me, but what's the point of "ptes"? Surely > you're either mapping at pte or pmd level, and the number of pages is determined > by the folio size? (or presumably nr param passed in) > > Thanks, > Ryan > >> >> folio_add_new_anon_rmap() will always map at the biggest granularity >> possible (currently, a single PMD to cover a PMD-sized THP). Could be >> extended if ever required. >> >> In the future, we might want "_pud" variants and eventually "_pmds" variants >> for batching. Further, if hugepd is ever a thing outside hugetlb code, >> we might want some variants for that. All stuff for the distant future. >> >> >> I ran some simple microbenchmarks from [5] on an Intel(R) Xeon(R) Silver >> 4210R: munmap(), fork(), cow, MADV_DONTNEED on each PTE ... and PTE >> remapping PMD-mapped THPs on 1 GiB of memory. >> >> For small folios, there is barely a change (< 1 % performance improvement), >> whereby fork() still stands out with 0.74% performance improvement, but >> it might be just some noise. Folio optimizations don't help that much >> with small folios. >> >> For PTE-mapped THP: >> * PTE-remapping a PMD-mapped THP is more than 10% faster. >> -> RMAP batching >> * fork() is more than 4% faster. >> -> folio conversion >> * MADV_DONTNEED is 2% faster >> -> folio conversion >> * COW by writing only a single byte on a COW-shared PTE >> -> folio conversion >> * munmap() is only slightly faster (< 1%). >> >> [1] https://lkml.kernel.org/r/20230810103332.3062143-1-ryan.roberts@arm.com >> [2] https://lkml.kernel.org/r/20231204105440.61448-1-ryan.roberts@arm.com >> [3] https://lkml.kernel.org/r/20231204102027.57185-1-ryan.roberts@arm.com >> [4] https://lkml.kernel.org/r/20231128145205.215026-1-david@redhat.com >> [5] https://lkml.kernel.org/r/20231124132626.235350-1-david@redhat.com >> >> Cc: Andrew Morton >> Cc: "Matthew Wilcox (Oracle)" >> Cc: Hugh Dickins >> Cc: Ryan Roberts >> Cc: Yin Fengwei >> Cc: Mike Kravetz >> Cc: Muchun Song >> Cc: Peter Xu >> >> David Hildenbrand (39): >> mm/rmap: rename hugepage_add* to hugetlb_add* >> mm/rmap: introduce and use hugetlb_remove_rmap() >> mm/rmap: introduce and use hugetlb_add_file_rmap() >> mm/rmap: introduce and use hugetlb_try_dup_anon_rmap() >> mm/rmap: introduce and use hugetlb_try_share_anon_rmap() >> mm/rmap: add hugetlb sanity checks >> mm/rmap: convert folio_add_file_rmap_range() into >> folio_add_file_rmap_[pte|ptes|pmd]() >> mm/memory: page_add_file_rmap() -> folio_add_file_rmap_[pte|pmd]() >> mm/huge_memory: page_add_file_rmap() -> folio_add_file_rmap_pmd() >> mm/migrate: page_add_file_rmap() -> folio_add_file_rmap_pte() >> mm/userfaultfd: page_add_file_rmap() -> folio_add_file_rmap_pte() >> mm/rmap: remove page_add_file_rmap() >> mm/rmap: factor out adding folio mappings into __folio_add_rmap() >> mm/rmap: introduce folio_add_anon_rmap_[pte|ptes|pmd]() >> mm/huge_memory: batch rmap operations in __split_huge_pmd_locked() >> mm/huge_memory: page_add_anon_rmap() -> folio_add_anon_rmap_pmd() >> mm/migrate: page_add_anon_rmap() -> folio_add_anon_rmap_pte() >> mm/ksm: page_add_anon_rmap() -> folio_add_anon_rmap_pte() >> mm/swapfile: page_add_anon_rmap() -> folio_add_anon_rmap_pte() >> mm/memory: page_add_anon_rmap() -> folio_add_anon_rmap_pte() >> mm/rmap: remove page_add_anon_rmap() >> mm/rmap: remove RMAP_COMPOUND >> mm/rmap: introduce folio_remove_rmap_[pte|ptes|pmd]() >> kernel/events/uprobes: page_remove_rmap() -> folio_remove_rmap_pte() >> mm/huge_memory: page_remove_rmap() -> folio_remove_rmap_pmd() >> mm/khugepaged: page_remove_rmap() -> folio_remove_rmap_pte() >> mm/ksm: page_remove_rmap() -> folio_remove_rmap_pte() >> mm/memory: page_remove_rmap() -> folio_remove_rmap_pte() >> mm/migrate_device: page_remove_rmap() -> folio_remove_rmap_pte() >> mm/rmap: page_remove_rmap() -> folio_remove_rmap_pte() >> Documentation: stop referring to page_remove_rmap() >> mm/rmap: remove page_remove_rmap() >> mm/rmap: convert page_dup_file_rmap() to >> folio_dup_file_rmap_[pte|ptes|pmd]() >> mm/rmap: introduce folio_try_dup_anon_rmap_[pte|ptes|pmd]() >> mm/huge_memory: page_try_dup_anon_rmap() -> >> folio_try_dup_anon_rmap_pmd() >> mm/memory: page_try_dup_anon_rmap() -> folio_try_dup_anon_rmap_pte() >> mm/rmap: remove page_try_dup_anon_rmap() >> mm: convert page_try_share_anon_rmap() to >> folio_try_share_anon_rmap_[pte|pmd]() >> mm/rmap: rename COMPOUND_MAPPED to ENTIRELY_MAPPED >> >> Documentation/mm/transhuge.rst | 4 +- >> Documentation/mm/unevictable-lru.rst | 4 +- >> include/linux/mm.h | 6 +- >> include/linux/rmap.h | 380 +++++++++++++++++++----- >> kernel/events/uprobes.c | 2 +- >> mm/gup.c | 2 +- >> mm/huge_memory.c | 85 +++--- >> mm/hugetlb.c | 21 +- >> mm/internal.h | 12 +- >> mm/khugepaged.c | 17 +- >> mm/ksm.c | 15 +- >> mm/memory-failure.c | 4 +- >> mm/memory.c | 60 ++-- >> mm/migrate.c | 12 +- >> mm/migrate_device.c | 41 +-- >> mm/mmu_gather.c | 2 +- >> mm/rmap.c | 422 ++++++++++++++++----------- >> mm/swapfile.c | 2 +- >> mm/userfaultfd.c | 2 +- >> 19 files changed, 709 insertions(+), 384 deletions(-) >> >