Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
Message-ID: <ac9a63ab-e83b-41d5-8a59-c4a137cdebd4@intel.com>
Date:   Wed, 6 Dec 2023 09:22:16 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH RFC 02/39] mm/rmap: introduce and use
 hugetlb_remove_rmap()
Content-Language: en-US
To:     David Hildenbrand <david@redhat.com>,
        <linux-kernel@vger.kernel.org>
CC:     <linux-mm@kvack.org>, Andrew Morton <akpm@linux-foundation.org>,
        "Matthew Wilcox (Oracle)" <willy@infradead.org>,
        Hugh Dickins <hughd@google.com>,
        Ryan Roberts <ryan.roberts@arm.com>,
        Mike Kravetz <mike.kravetz@oracle.com>,
        Muchun Song <muchun.song@linux.dev>,
        Peter Xu <peterx@redhat.com>
References: <20231204142146.91437-1-david@redhat.com>
 <20231204142146.91437-3-david@redhat.com>
From:   Yin Fengwei <fengwei.yin@intel.com>
In-Reply-To: <20231204142146.91437-3-david@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?a09sVnZqbmltUWhDN01sMUhBMDVVdko2K2NzTnZmdDNKQUtJUUlLQU83SmVI?=
 =?utf-8?B?bUUrV0MzSXdzNCtBdkZLMnFSQ1Q5K1BNcmNGbyt2TmFiQ1ROamN2cSsrNUVS?=
 =?utf-8?B?Y2l6aVd2aURQUGVkcnorMThPZEtCcjVDS1lmS1BMWkFIUDFRWGI3VU8wUjRB?=
 =?utf-8?B?ZXg4VEh1TVl6Sm5NOHpSZmFlaUo3K3NPcncyNVRwQjlBdE5BdUhaSms2aXFG?=
 =?utf-8?B?QjBqbTA3OXNnNVpBTzg4UFFncU42VU1jL2krT0V6YkdxU05RS2FKajFZRkJC?=
 =?utf-8?B?OU53Y0xrR295TWtzWGgrTnZ5bTRQM0FqMmVUSm8wRk9xRHNjT1NsVEFOTmR4?=
 =?utf-8?B?ZEFtdUV3TnI2a094WGJITGdMeFkzclJpK0FjczNneFBHLzZzaytIZ0N6TnBi?=
 =?utf-8?B?d3VEOWVVdk81WVQ4Y0RkV0RPdzJTU2dLWGVQRFJNQUhseWFkc0UxVkV2TVl6?=
 =?utf-8?B?N0lHUnRKQ3BHbjRaM1d1VGlpc0ZLOXFxMmhhaDVzVmNZZWpEeDlvaGVjaDMz?=
 =?utf-8?B?VFFEb2pVeGI1QTI5d2lzelZ0K25xYTJtYTNjejFSaDZhNEhlZUNYZ2d3ZmFY?=
 =?utf-8?B?TXFYK3V5UmFBN0MySi9tUzNIQ0M5WnkwY0xaaFpJOHJHMWU4SzgyeDV5eU0v?=
 =?utf-8?B?cVBBYy9vTnJQc2JHRzIreGZDMzM2UmFTRVZ2TEZPY2k0YWlGeERyaXRHelFV?=
 =?utf-8?B?SGFRTUhMYXBydlN2OW5udUFMVmhCYk5Oa2lBTWhRTXBQZ2NISGl4aSt6djBo?=
 =?utf-8?B?T2xLdnpENkVQU29BL2d3QitPR0dWY0VMN1I1Rm0yOWJQa2w3VjRKWlpjTWpP?=
 =?utf-8?B?L0poTGdZY2VSUU1KU05meDF4K0YyS2ZRMGtDMDdTWVA1bFprZ3ZGY3Ixd01G?=
 =?utf-8?B?ai8ydzlPRDN4aWFHcnMwdEorMzJBbzd1VDB1RXZrNlFQYkdDSVREN3lzZHVB?=
 =?utf-8?B?S1B5UTEySVY1NzBzcS9SZFdVei82VGVGdUI0SDgrM1lBc1FFU0duT1RLVU40?=
 =?utf-8?B?cDdLQ2tvNWFNc0RIejBkUml0R1ZsRDRpbjg5eGd6SUpkUnA5S2ZsWS9lb2kv?=
 =?utf-8?B?RVcwQno4dUxUa1ZwQkNRZXFtZXByeFQ3NVdFSSttQ01OWXdlSWVEaTc5TE1r?=
 =?utf-8?B?MklVdSt2MTNQNUk5R25nZ1F0dE1ua0VVTnpwKy9ETHZPa1JTZTRBWmQxdWFO?=
 =?utf-8?B?aXJxbldJRjJnV1h2T0hCcHZDMWN6Nmp5L0dmc1ZLZUZRUmtKM3RmNkFta2Q3?=
 =?utf-8?B?dTVDdmFJRG1NdExRYkVqam1DOEtSeE55MTd4Zzl0M3FMV1F4RmVUL0tHRm1s?=
 =?utf-8?B?TVBhZlMzZU1xWkVDQnFwdGM2dGs1QllxQTVMNXRaSzI5WWZ5cTUyRHJ0YW1C?=
 =?utf-8?B?RWJDZEcvcFJEQm84cjlFckdRU2hUTHd0cUZ6M0ZQaXJ0MG1USjVkVXJBL0po?=
 =?utf-8?B?ZjBCZDNrUkJFeTB5aE5BUHZXbEliNllEVnlET2E1MEMzQWpjem91cUhSNVFp?=
 =?utf-8?B?T2pPYUgwT1RCTUtnSzZ6NVp0VkQvZXJuS1c4RmhqNkVvQ2xKSWNjZDMyVXpq?=
 =?utf-8?B?VTdCVFJBV0U4OW9IQWxSK2ZDaDBqanp3OUVPWm43YkpSNTF3UWxvYkZ3YlRk?=
 =?utf-8?B?ZlNtVTBhdWNWUjJuVUZkMlZueG1ocUdTVkN6MzVPMTZxVHE1YmtvRUJ2elVt?=
 =?utf-8?B?S3FkTWxWY3gvZFhnTWVwZ25yQVBpbi8vcWN4RnZNTFFucElIRjZXblJTdUFQ?=
 =?utf-8?B?Q2V2ZjRua3B6dVRTcUt1TlBhN0VlWGxwejVpc2NIWFd2aGdDYXh0dmloaGJz?=
 =?utf-8?B?Y1hRNS9NSVpRVk45V3BQY01qdGR2a0UwRnNsVlN4VE0wRklFMUNMS3gvWTVW?=
 =?utf-8?B?ejVOWUliU1FSRklaTk1ERmFkMlVHZHVYRk8vYldpOUxHMHZSUjRUekRKa0hq?=
 =?utf-8?B?NGZvcjBENGY1ZG5YSW0yaUR3TkY1bFY2MVZFWDRuRlRZdHFYa1lpaUI5WnN4?=
 =?utf-8?B?b1JDd0o2dS9EQm1HdkdPRDNJTVNvbkRYY0ZsUU5lNnFHdjRFYThYYmFEeHZD?=
 =?utf-8?B?aXBza3VZL2hlUFIzSU9TR0x3YTRnTWdtaUs0NTNVVFBkbzUzNHl6SWR1aWpM?=
 =?utf-8?B?SXFSM3I2RGVONy9OcnVHQmp4ZDVRR1ZTck9oc3FLMjhZRGNzc0tGTHFRL3hH?=
 =?utf-8?B?clE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 223151f6-48cf-4116-79ef-08dbf5fa18f4
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB4820.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Dec 2023 01:24:32.0481
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: I+LduDQe+RqYwjcUeLmnACPa3D07//YkRFFEiMQE17xXtRII9doetL3yxpLoqtAm9+fVBtJ6SeTy1wjRpgoWeg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR11MB6440
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Tue, 05 Dec 2023 17:25:36 -0800 (PST)


On 12/4/23 22:21, David Hildenbrand wrote:
> hugetlb rmap handling differs quite a lot from "ordinary" rmap code.
> For example, hugetlb currently only supports entire mappings, and treats
> any mapping as mapped using a single "logical PTE". Let's move it out
> of the way so we can overhaul our "ordinary" rmap.
> implementation/interface.
> 
> Let's introduce and use hugetlb_remove_rmap() and remove the hugetlb
> code from page_remove_rmap(). This effectively removes one check on the
> small-folio path as well.
> 
> Note: all possible candidates that need care are page_remove_rmap() that
>       pass compound=true.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>


> ---
>  include/linux/rmap.h |  5 +++++
>  mm/hugetlb.c         |  4 ++--
>  mm/rmap.c            | 17 ++++++++---------
>  3 files changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/rmap.h b/include/linux/rmap.h
> index 4c5bfeb054636..e8d1dc1d5361f 100644
> --- a/include/linux/rmap.h
> +++ b/include/linux/rmap.h
> @@ -208,6 +208,11 @@ void hugetlb_add_anon_rmap(struct folio *, struct vm_area_struct *,
>  void hugetlb_add_new_anon_rmap(struct folio *, struct vm_area_struct *,
>  		unsigned long address);
>  
> +static inline void hugetlb_remove_rmap(struct folio *folio)
> +{
> +	atomic_dec(&folio->_entire_mapcount);
> +}
> +
>  static inline void __page_dup_rmap(struct page *page, bool compound)
>  {
>  	if (compound) {
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 4cfa0679661e2..d17bb53b19ff2 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -5669,7 +5669,7 @@ void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma,
>  					make_pte_marker(PTE_MARKER_UFFD_WP),
>  					sz);
>  		hugetlb_count_sub(pages_per_huge_page(h), mm);
> -		page_remove_rmap(page, vma, true);
> +		hugetlb_remove_rmap(page_folio(page));
>  
>  		spin_unlock(ptl);
>  		tlb_remove_page_size(tlb, page, huge_page_size(h));
> @@ -5980,7 +5980,7 @@ static vm_fault_t hugetlb_wp(struct mm_struct *mm, struct vm_area_struct *vma,
>  
>  		/* Break COW or unshare */
>  		huge_ptep_clear_flush(vma, haddr, ptep);
> -		page_remove_rmap(&old_folio->page, vma, true);
> +		hugetlb_remove_rmap(old_folio);
>  		hugetlb_add_new_anon_rmap(new_folio, vma, haddr);
>  		if (huge_pte_uffd_wp(pte))
>  			newpte = huge_pte_mkuffd_wp(newpte);
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 112467c30b2c9..5037581b79ec6 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -1440,13 +1440,6 @@ void page_remove_rmap(struct page *page, struct vm_area_struct *vma,
>  
>  	VM_BUG_ON_PAGE(compound && !PageHead(page), page);
>  
> -	/* Hugetlb pages are not counted in NR_*MAPPED */
> -	if (unlikely(folio_test_hugetlb(folio))) {
> -		/* hugetlb pages are always mapped with pmds */
> -		atomic_dec(&folio->_entire_mapcount);
> -		return;
> -	}
> -
>  	/* Is page being unmapped by PTE? Is this its last map to be removed? */
>  	if (likely(!compound)) {
>  		last = atomic_add_negative(-1, &page->_mapcount);
> @@ -1804,7 +1797,10 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>  			dec_mm_counter(mm, mm_counter_file(&folio->page));
>  		}
>  discard:
> -		page_remove_rmap(subpage, vma, folio_test_hugetlb(folio));
> +		if (unlikely(folio_test_hugetlb(folio)))
> +			hugetlb_remove_rmap(folio);
> +		else
> +			page_remove_rmap(subpage, vma, false);
>  		if (vma->vm_flags & VM_LOCKED)
>  			mlock_drain_local();
>  		folio_put(folio);
> @@ -2157,7 +2153,10 @@ static bool try_to_migrate_one(struct folio *folio, struct vm_area_struct *vma,
>  			 */
>  		}
>  
> -		page_remove_rmap(subpage, vma, folio_test_hugetlb(folio));
> +		if (unlikely(folio_test_hugetlb(folio)))
> +			hugetlb_remove_rmap(folio);
> +		else
> +			page_remove_rmap(subpage, vma, false);
>  		if (vma->vm_flags & VM_LOCKED)
>  			mlock_drain_local();
>  		folio_put(folio);