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 A8CC1EB64DA for ; Tue, 18 Jul 2023 10:19:15 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3BA756B0071; Tue, 18 Jul 2023 06:19:15 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3906A8D0002; Tue, 18 Jul 2023 06:19:15 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 27FEB8D0001; Tue, 18 Jul 2023 06:19:15 -0400 (EDT) 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 182856B0071 for ; Tue, 18 Jul 2023 06:19:15 -0400 (EDT) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id BCFF4B0B67 for ; Tue, 18 Jul 2023 10:19:14 +0000 (UTC) X-FDA: 81024335028.06.BCA87D6 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf30.hostedemail.com (Postfix) with ESMTP id D66A180010 for ; Tue, 18 Jul 2023 10:19:12 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf30.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=1689675553; 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=l4f1SGpNLS9Vah4kZfYBkhMHpCBA04Wx5UX8ZDxj6X0=; b=PEDGfj+v/BjDGZ/Jz/Y1/TRcw9TJ4CgF9vM2+3ktYFLrxygr6fB3TxTq7gcGxvMmkNLYaP Gn8fbOMKkooOvjTYMgXTrSFmhxJGMIMz1tjK3cVZIl2emVPVwUEqzCXqkCPMiH3PtoCWm7 DovEy5HHJ/MQS9l18LNz9RvDn7z16ds= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf30.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=1689675553; a=rsa-sha256; cv=none; b=JXRZzvaln3WETqWuLMVBG31bOGTVMzPZLSOG0DX+t75N6b3vf/DIlr+WUMa5sbBXExoLrf i8XaCDWdTpUvcud188Pl15XS+AA2ugbL8ySfNc9C2YzPUSwYjnXNJbmryMXFgrxE9Mxe+9 Hvd+tBJb5rF0UXUt5C1uOYbyNp+9H9Q= 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 859802F4; Tue, 18 Jul 2023 03:19:55 -0700 (PDT) Received: from [10.1.34.52] (C02Z41KALVDN.cambridge.arm.com [10.1.34.52]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A4E663F67D; Tue, 18 Jul 2023 03:19:10 -0700 (PDT) Message-ID: Date: Tue, 18 Jul 2023 11:19:08 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Subject: Re: [PATCH v1 3/3] mm: Batch-zap large anonymous folio PTE mappings To: Zi Yan Cc: Andrew Morton , Matthew Wilcox , Yin Fengwei , David Hildenbrand , Yu Zhao , Yang Shi , "Huang, Ying" , linux-kernel@vger.kernel.org, linux-mm@kvack.org References: <20230717143110.260162-1-ryan.roberts@arm.com> <20230717143110.260162-4-ryan.roberts@arm.com> <5A282984-F3AD-41E3-8EF2-BA0A77DD1A3A@nvidia.com> <980c4e1f-116b-0113-65ee-4e77fdd3e7b4@arm.com> From: Ryan Roberts In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: D66A180010 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: wwf79xi5cuhzzwhpfd97j4p3wh84dkog X-HE-Tag: 1689675552-970445 X-HE-Meta: U2FsdGVkX18Acb7pbL59M+PSzMA31Wn1a14gvdKp1Si1m+TR6bRjIGsKKHpXffLvMH4GRZgE5pmjjLPHL6SyqyLA5xjHAVYeXpotStRyK5tVkuLN+bQ2hhUHp/ymvfd60KG+bx4cRrwBZIpmVxd6iLAXGvT0xZBYmchRKScWDNYaH1WpQeHj/cLdliXk3xkzO2QpaRWsYN+VMSyymEeaew5h2HCQthJUwQskQFd6sCpi4xnvnTUJ5O26+LrwqP7tpg5Nn/zEnUlmXdqvfJrMZ8LZrQ6L7LCSpACLiB/LRvmOGYZS3+YBxOVKxskYl8amOulYX7+wCU7/4bOP87NgKop99ZuL1D2xWA4U9r3ONRs+5j+lIqzvCO8jSMfov3BMbM0WOLnnEgS4mpz0xKwCDc90h4go9PULwnV4EWg9YJ6b4ZlaDYUIqi5eQSAq17T4YyaoEwVC+QsHim6a+1F86Rpr4TDhBWRUhiIvnH2kGLPo3wX2YLeQ8fobd5c9r5TwZTm5wpmZS380MGH4FC3nrEqa8jMdkiO9Q2BiQhHBaJmTbSqHYrQEKxpBwh95/3lTZqRlA0cS+RDIi8W0lMzbjHoegdFzftxDXU7EsEYna8pl6ps8tuvAKaxI/6KbzQAELLdM7Qfi9oSTix6Swd5rRh3KtzQBH/uYIvzzeLStOtFl3uwjQqC3iO+bzQkyzwXfsgvSkOxnfH6TdohJ4C+r0yYs51bfwyZcyEzigNNy5cIU+9oCNl+tECO9ky9TSJp/sOTj/4VKGjiA6yZSSJ/14UQvQBZUQ/qLs3UMvmz1eoRYJW4eY8DRXmykC2GiOrwLACu+T9+vqqVRleMkqi+JUUpdsQqgmWc7wzzEVBQv5skHa9NZgqEEhtylnywcf3FeCqAk1xKs8XISMXwLq/FsQ9yAqWxXbACvk6RxRBs9sMAFjdBByxqytzD3Dt2hqNlKrvcEHNVFVfuHGQb+7KQ rs+cwtTq Q3nuFsPKV3XgXrRN/gloZBR45somgG4O2Ubf8WsKdNrh/UhxYiEfDTSDMYYxOGd8PesLqX085RDOmjpUpQPu5NtqnwOQWd4zKQE02eW8R+dSYVDqoE3XmS0H9WL42YIXCaIhEnbKPqKUDR3TjfnZMLs3q1mumHFNhHROfTh+btuqniUk= 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: On 17/07/2023 17:15, Zi Yan wrote: > On 17 Jul 2023, at 11:55, Ryan Roberts wrote: > >> On 17/07/2023 16:25, Zi Yan wrote: >>> On 17 Jul 2023, at 10:31, Ryan Roberts wrote: >>> >>>> This allows batching the rmap removal with folio_remove_rmap_range(), >>>> which means we avoid spuriously adding a partially unmapped folio to the >>>> deferrred split queue in the common case, which reduces split queue lock >>>> contention. >>>> >>>> Previously each page was removed from the rmap individually with >>>> page_remove_rmap(). If the first page belonged to a large folio, this >>>> would cause page_remove_rmap() to conclude that the folio was now >>>> partially mapped and add the folio to the deferred split queue. But >>>> subsequent calls would cause the folio to become fully unmapped, meaning >>>> there is no value to adding it to the split queue. >>>> >>>> Signed-off-by: Ryan Roberts >>>> --- >>>> mm/memory.c | 119 ++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>> 1 file changed, 119 insertions(+) >>>> >>>> diff --git a/mm/memory.c b/mm/memory.c >>>> index 01f39e8144ef..6facb8c8807a 100644 >>>> --- a/mm/memory.c >>>> +++ b/mm/memory.c >>>> @@ -1391,6 +1391,95 @@ zap_install_uffd_wp_if_needed(struct vm_area_struct *vma, >>>> pte_install_uffd_wp_if_needed(vma, addr, pte, pteval); >>>> } >>>> >>>> +static inline unsigned long page_addr(struct page *page, >>>> + struct page *anchor, unsigned long anchor_addr) >>>> +{ >>>> + unsigned long offset; >>>> + unsigned long addr; >>>> + >>>> + offset = (page_to_pfn(page) - page_to_pfn(anchor)) << PAGE_SHIFT; >>>> + addr = anchor_addr + offset; >>>> + >>>> + if (anchor > page) { >>>> + if (addr > anchor_addr) >>>> + return 0; >>>> + } else { >>>> + if (addr < anchor_addr) >>>> + return ULONG_MAX; >>>> + } >>>> + >>>> + return addr; >>>> +} >>>> + >>>> +static int calc_anon_folio_map_pgcount(struct folio *folio, >>>> + struct page *page, pte_t *pte, >>>> + unsigned long addr, unsigned long end) >>>> +{ >>>> + pte_t ptent; >>>> + int floops; >>>> + int i; >>>> + unsigned long pfn; >>>> + >>>> + end = min(page_addr(&folio->page + folio_nr_pages(folio), page, addr), >>>> + end); >>>> + floops = (end - addr) >> PAGE_SHIFT; >>>> + pfn = page_to_pfn(page); >>>> + pfn++; >>>> + pte++; >>>> + >>>> + for (i = 1; i < floops; i++) { >>>> + ptent = ptep_get(pte); >>>> + >>>> + if (!pte_present(ptent) || >>>> + pte_pfn(ptent) != pfn) { >>>> + return i; >>>> + } >>>> + >>>> + pfn++; >>>> + pte++; >>>> + } >>>> + >>>> + return floops; >>>> +} >>>> + >>>> +static unsigned long zap_anon_pte_range(struct mmu_gather *tlb, >>>> + struct vm_area_struct *vma, >>>> + struct page *page, pte_t *pte, >>>> + unsigned long addr, unsigned long end, >>>> + bool *full_out) >>>> +{ >>>> + struct folio *folio = page_folio(page); >>>> + struct mm_struct *mm = tlb->mm; >>>> + pte_t ptent; >>>> + int pgcount; >>>> + int i; >>>> + bool full; >>>> + >>>> + pgcount = calc_anon_folio_map_pgcount(folio, page, pte, addr, end); >>>> + >>>> + for (i = 0; i < pgcount;) { >>>> + ptent = ptep_get_and_clear_full(mm, addr, pte, tlb->fullmm); >>>> + tlb_remove_tlb_entry(tlb, pte, addr); >>>> + full = __tlb_remove_page(tlb, page, 0); >>>> + >>>> + if (unlikely(page_mapcount(page) < 1)) >>>> + print_bad_pte(vma, addr, ptent, page); >>>> + >>>> + i++; >>>> + page++; >>>> + pte++; >>>> + addr += PAGE_SIZE; >>>> + >>>> + if (unlikely(full)) >>>> + break; >>>> + } >>>> + >>>> + folio_remove_rmap_range(folio, page - i, i, vma); >>>> + >>>> + *full_out = full; >>>> + return i; >>>> +} >>>> + >>>> static unsigned long zap_pte_range(struct mmu_gather *tlb, >>>> struct vm_area_struct *vma, pmd_t *pmd, >>>> unsigned long addr, unsigned long end, >>>> @@ -1428,6 +1517,36 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb, >>>> page = vm_normal_page(vma, addr, ptent); >>>> if (unlikely(!should_zap_page(details, page))) >>>> continue; >>>> + >>>> + /* >>>> + * Batch zap large anonymous folio mappings. This allows >>>> + * batching the rmap removal, which means we avoid >>>> + * spuriously adding a partially unmapped folio to the >>>> + * deferrred split queue in the common case, which >>>> + * reduces split queue lock contention. Require the VMA >>>> + * to be anonymous to ensure that none of the PTEs in >>>> + * the range require zap_install_uffd_wp_if_needed(). >>>> + */ >>>> + if (page && PageAnon(page) && vma_is_anonymous(vma)) { >>>> + bool full; >>>> + int pgcount; >>>> + >>>> + pgcount = zap_anon_pte_range(tlb, vma, >>>> + page, pte, addr, end, &full); >>> >>> Are you trying to zap as many ptes as possible if all these ptes are >>> within a folio? >> >> Yes. >> >>> If so, why not calculate end before calling zap_anon_pte_range()? >>> That would make zap_anon_pte_range() simpler. >> >> I'm not sure I follow. That's currently done in calc_anon_folio_map_pgcount(). I >> could move it to here, but I'm not sure that makes things simpler, just puts >> more code in here and less in there? > > Otherwise your zap_anon_pte_range() is really zap_anon_pte_in_folio_range() or > some other more descriptive name. When I first look at the name, I thought > PTEs will be zapped until the end. But that is not the case when I look at the > code. And future users can easily be confused too and use it in a wrong way. OK I see your point. OK let me pull the page count calculation into here and pass it to zap_anon_pte_range(). Then I think we can keep the name as is? > > BTW, page_addr() needs a better name and is easily confused with existing > page_address(). Yeah... I'll try to think of something for v2. > >> >>> Also check if page is part of >>> a large folio first to make sure you can batch. >> >> Yeah that's fair. I'd be inclined to put that in zap_anon_pte_range() to short >> circuit calc_anon_folio_map_pgcount(). But ultimately zap_anon_pte_range() would >> still zap the single pte. >> >> >>> >>>> + >>>> + rss[mm_counter(page)] -= pgcount; >>>> + pgcount--; >>>> + pte += pgcount; >>>> + addr += pgcount << PAGE_SHIFT; >>>> + >>>> + if (unlikely(full)) { >>>> + force_flush = 1; >>>> + addr += PAGE_SIZE; >>>> + break; >>>> + } >>>> + continue; >>>> + } >>>> + >>>> ptent = ptep_get_and_clear_full(mm, addr, pte, >>>> tlb->fullmm); >>>> tlb_remove_tlb_entry(tlb, pte, addr); >>>> -- >>>> 2.25.1 >>> >>> >>> -- >>> Best Regards, >>> Yan, Zi > > > -- > Best Regards, > Yan, Zi