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 8397AC25B75 for ; Tue, 14 May 2024 08:36:59 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C6EC36B0307; Tue, 14 May 2024 04:36:58 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C206A6B0308; Tue, 14 May 2024 04:36:58 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B0D696B0309; Tue, 14 May 2024 04:36:58 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 93C066B0307 for ; Tue, 14 May 2024 04:36:58 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 16893A1F4C for ; Tue, 14 May 2024 08:36:58 +0000 (UTC) X-FDA: 82116346116.13.040E595 Received: from mail-ed1-f49.google.com (mail-ed1-f49.google.com [209.85.208.49]) by imf16.hostedemail.com (Postfix) with ESMTP id 2D8D1180007 for ; Tue, 14 May 2024 08:36:54 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=Y2NRBluV; spf=pass (imf16.hostedemail.com: domain of ioworker0@gmail.com designates 209.85.208.49 as permitted sender) smtp.mailfrom=ioworker0@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1715675815; 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:dkim-signature; bh=OJ/eGTHdJFdkG3VlMLzx72gixE8FVhh6dK5QLaqRFCY=; b=WBFtyflPQATtBfTwdAoTNmXDa6TJKPrdLuUJb6kDr3Y5zC8hTFpeC2MbDu7DKbQbd3t3Kq ClTUkELsBqizFxq0o6anLpShQk4RAAekoXUQ6Jk8Kcx5k+ZGw6dRRNBCX9ebb9wtYojwRa XQNitUIq02olfyHZBblDT35X49g49zk= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=Y2NRBluV; spf=pass (imf16.hostedemail.com: domain of ioworker0@gmail.com designates 209.85.208.49 as permitted sender) smtp.mailfrom=ioworker0@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1715675815; a=rsa-sha256; cv=none; b=NDkx2cqS+e/DxK2UL67kfh7nHCu9gW15uOEklVeF4BUBA2FnViJ6eAHx2gctFPc4wqlKfJ y9p5vd8WHtLRo70mlSYPZjUDDMFPo8o+W/+wxqPLzXbGrfWWtR8sSUy0z446GhbhU71l2G Xq74XeW/we3nYcdhI42+aBu4YgHV6T4= Received: by mail-ed1-f49.google.com with SMTP id 4fb4d7f45d1cf-5731ffcc905so9829a12.2 for ; Tue, 14 May 2024 01:36:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1715675813; x=1716280613; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=OJ/eGTHdJFdkG3VlMLzx72gixE8FVhh6dK5QLaqRFCY=; b=Y2NRBluVeCQciBxU5AvRR+e7MVTzP8oycqSe39yJHvSl7M5m8XP5NTpNd0Xpt7YXzy od0PbxiqUUpcwNaQHUcH1FQKd/mfpJFcTgGrJlLzDUEhJBCRKIQF+jHJ5ZswazpR3dje FCz3cAa4iwPl/Ufwn4aBDx7d8Txuly/22GaG/4vdyrBWfAMmhZwJUMgneKjp5TeJVK0H 7D6bfQB+auSiZKYra+9fycx3VWsfra82KoFwi4mRLn/zhiWiAxvr43KFX3icWbH4UZ/q P87XwUHcBn+nVqW+EQVXjDIEU7RczOGwye2X8WNQnQ+nvukHVhOSF268lZLOgdlB7AgL RI9g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1715675813; x=1716280613; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=OJ/eGTHdJFdkG3VlMLzx72gixE8FVhh6dK5QLaqRFCY=; b=Q3v1rnZHdUR9AJOOKAFDLo/jp0rlen0n9ilzGNZk+Wtnh/z4rTAZozS25a9O8tfR2V CynRS0BQZcoELFtWVtFGrUcslmbmBdNYLKthjFekxAwYEN0WJ4a4/oZHzi6R5imOqKd8 QqmfUn+WVyiNiCo74EU2HwUhiIMyT0I2e8HiMl7yemBNqK9O5Q+QX1nVgzF1nXBAvf05 ef6x2zJEXbPdJcibnpzeC2nAzqcbCxk8UBfD8Klji1UJZ3P0wh6L/Jg8Scax8mfMtDKh mvGgJx1/Wr0BeABU/51+Nb7soYemugeFkvBPHhvYJ+U2Ab/T35dzQ6yM9Wr5kh4FCF1c 6faw== X-Forwarded-Encrypted: i=1; AJvYcCVDAWSam0PcQgUWc6T9qIYpxHPVM/i810yeRAfTDec04+i3xoCFUu2BQyw02qhvIhqFY+lXsg8w1ieqqWs4LVD89Zo= X-Gm-Message-State: AOJu0Yx94qHc0LY00kHkxKI2Mh6lC8rk4RjeUNsWQPSD0xPauY1ZVpuL yu6yYfaNoViO1adXwPpR3R7gtF9Bn6JY7X7Fs9qEJw/hqtkLlXk0tKy1LcfQAE4LpxUTTidm3H8 vv0loJKBsEaWV0f8226w0gEYwfso= X-Google-Smtp-Source: AGHT+IFL6LmTzPmyUZ4HKJocx8V2LR8Ri2r66jvZdMmjAdm1Xlmf5hGSCM7pHC7zxxtjb5RDklqNdjEGNv/V8HFG9FM= X-Received: by 2002:a50:8adb:0:b0:572:a22b:12db with SMTP id 4fb4d7f45d1cf-5734d67f00fmr8930278a12.28.1715675813507; Tue, 14 May 2024 01:36:53 -0700 (PDT) MIME-Version: 1.0 References: <20240513074712.7608-1-ioworker0@gmail.com> <20240513074712.7608-5-ioworker0@gmail.com> In-Reply-To: From: Lance Yang Date: Tue, 14 May 2024 16:36:42 +0800 Message-ID: Subject: Re: [PATCH RESEND v5 4/4] mm/vmscan: avoid split lazyfree THP during shrink_folio_list() To: Baolin Wang Cc: akpm@linux-foundation.org, willy@infradead.org, sj@kernel.org, maskray@google.com, ziy@nvidia.com, ryan.roberts@arm.com, david@redhat.com, 21cnbao@gmail.com, mhocko@suse.com, fengwei.yin@intel.com, zokeefe@google.com, shy828301@gmail.com, xiehuan09@gmail.com, libang.li@antgroup.com, wangkefeng.wang@huawei.com, songmuchun@bytedance.com, peterx@redhat.com, minchan@kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 2D8D1180007 X-Stat-Signature: exr7rpf19yeyy1j88h8i7jhobtbhm9zo X-HE-Tag: 1715675814-628578 X-HE-Meta: U2FsdGVkX1/uctjnHVDPaAsgJPmjIqc4kkaljfeFocY/G1P/RV94BBy1oxf5cTyuex12njJS0w5de46u0PfHXuXeMabXg7ueyQEFayJo5aP7qOsQmllDirHq8TvOPxVL1n7hrjwj+Ni4xibENhX/oDTjsuQkCG5eRtyLWFRsyVIaRUhvsi1APEQez9oybYoq0lA7sjfpzUkEUHd/jc7zC8jaPtJftGDSpVrj6NSIyXz15EyiEbMtVtCKWlMLaKRzs8omiyrtWpsb4ML0kKJ2lKdAra+/Pl1uknIe83LK6PkABR6SguOPPrcHgURrlpSa+ToY2pnISjYK1vIDqXJkWaNvJKfnl+l13ailtHu3ne4hzljvKUr+XW6ZUnR2zxZ9DRe5ffAyRIASM+7+jFW6ul8Dp01KTSAWfyodbNOw4HQkO0oxaeITiSCrY9SZrK3diGbE4G+hnmOhMYY6PUpI8rVKOkN7PrMvilzvbWF1Fa6p8mO9dHwct4rC5mzSPuA18PqKe9DMjH7/T7HsZHMJ2YvffUZ4HQuQfyaPRmiy2GUTZ2+p9RkbpJjXYmQaiKiIccTnwre36eKP00VBBfngjshx9pz05veycJxRjZgbn4647JnznjSO3n9CTSMNNvtDE8JPAX0by7OJx3BXN/D+XgD6+sG5WpiC8H4wiyd4UbxsQFVSV2o4YgevkQb8tmM8J1brqHo6wUWNHm7+yXL4inTLK5qV/Zdxqg6EQz5FJDw/KuyfPWmeZMFO/S7cXCFWVZSwJIWJldJ/1nFkciqOgaHuBn4implLwHZAyzpa/7NVKDB9m3sn3mY1XC9VtPrtIqOeUucoPL1a4X/lnvlvYjo142JeYuj3nXHu09rZnibj3xROoDZvzPkiut6UAkIiu+9wG6DgywXBgICwZpgu093BiXGjeWsAeUR6XMGf3RM3L5GPvns1eXxeuMuA4SFwuLTTJzNsuWO14h+13Hm U2SCqCtF FiGfTfKmVQ/IqpGjLcqLS6y0Ul1bBH4pyR8l1qmFEjSVRaQmsDLdt0mr35FOB417VCuzaqC6b3ptJ+kLdBRH++ClKZmXLQ4PpEhqGdjSNcNwoMQihJA2ydkWW2gIiCmcCnHeiY+/i2CW8IQAFw/G9PtFO/mEpFj3Fwnf6QR0VNqKW3PQCS/wu8sYBNGcaGZR916n7jz/jHvvNwFcjNLsVDcH6l0p8aC7+tyNyrbour/HTE1PXE10SGCPzo6KE0k5+m0NmFFPFYKltIYrmOSlpUY1YK52xJaAPIlD+F1E+GLnsbnbyMKMNLNDypcQKEp0fqKaZbptubYEH7XrHM61KDcJ3TVywqHN+jXX15YxWdP4KBvYx9hij5Inx+UXsdOxjHowFSzQmFgcpuTlA1kd1HlrG8tW00VVEJzJDe/k/5hO6ctUBbWLhklGmpPG+W+M+1W/ArWWlzphN8UNeUP9/CWKYxGeENN0S0+Mj571oSy8/3E0= 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: Hi Baolin, Thanks for taking time to review! On Tue, May 14, 2024 at 3:09=E2=80=AFPM Baolin Wang wrote: > > > > On 2024/5/13 15:47, Lance Yang wrote: > > When the user no longer requires the pages, they would use > > madvise(MADV_FREE) to mark the pages as lazy free. Subsequently, they > > typically would not re-write to that memory again. > > > > During memory reclaim, if we detect that the large folio and its PMD ar= e > > both still marked as clean and there are no unexpected references > > (such as GUP), so we can just discard the memory lazily, improving the > > efficiency of memory reclamation in this case. > > > > On an Intel i5 CPU, reclaiming 1GiB of lazyfree THPs using > > mem_cgroup_force_empty() results in the following runtimes in seconds > > (shorter is better): > > > > -------------------------------------------- > > | Old | New | Change | > > -------------------------------------------- > > | 0.683426 | 0.049197 | -92.80% | > > -------------------------------------------- > > > > Suggested-by: Zi Yan > > Suggested-by: David Hildenbrand > > Signed-off-by: Lance Yang > > --- > > include/linux/huge_mm.h | 9 +++++ > > mm/huge_memory.c | 75 ++++++++++++++++++++++++++++++++++++++++= + > > mm/rmap.c | 31 ++++++++++------- > > 3 files changed, 103 insertions(+), 12 deletions(-) > > > > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h > > index 9fcb0b0b6ed1..cfd7ec2b6d0a 100644 > > --- a/include/linux/huge_mm.h > > +++ b/include/linux/huge_mm.h > > @@ -411,6 +411,8 @@ static inline bool thp_migration_supported(void) > > > > void split_huge_pmd_locked(struct vm_area_struct *vma, unsigned long = address, > > pmd_t *pmd, bool freeze, struct folio *folio); > > +bool unmap_huge_pmd_locked(struct vm_area_struct *vma, unsigned long a= ddr, > > + pmd_t *pmdp, struct folio *folio); > > > > #else /* CONFIG_TRANSPARENT_HUGEPAGE */ > > > > @@ -478,6 +480,13 @@ static inline void split_huge_pmd_locked(struct vm= _area_struct *vma, > > unsigned long address, pmd_t *pm= d, > > bool freeze, struct folio *folio= ) {} > > > > +static inline bool unmap_huge_pmd_locked(struct vm_area_struct *vma, > > + unsigned long addr, pmd_t *pmdp, > > + struct folio *folio) > > +{ > > + return false; > > +} > > + > > #define split_huge_pud(__vma, __pmd, __address) \ > > do { } while (0) > > > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > > index 425272c6c50b..3ceeeb2f42d4 100644 > > --- a/mm/huge_memory.c > > +++ b/mm/huge_memory.c > > @@ -2687,6 +2687,81 @@ static void unmap_folio(struct folio *folio) > > try_to_unmap_flush(); > > } > > > > +static bool __discard_trans_pmd_locked(struct vm_area_struct *vma, > > + unsigned long addr, pmd_t *pmdp, > > + struct folio *folio) > > +{ > > + struct mm_struct *mm =3D vma->vm_mm; > > + int ref_count, map_count; > > + pmd_t orig_pmd =3D *pmdp; > > + struct mmu_gather tlb; > > + struct page *page; > > + > > + if (pmd_dirty(orig_pmd) || folio_test_dirty(folio)) > > + return false; > > + if (unlikely(!pmd_present(orig_pmd) || !pmd_trans_huge(orig_pmd))= ) > > + return false; > > + > > + page =3D pmd_page(orig_pmd); > > + if (unlikely(page_folio(page) !=3D folio)) > > + return false; > > The function is called under the ptl lock, so I have no idea why the pmd > value can be changed, seems above validation is useless. Yep. For the page reclaim path, page_vma_mapped_walk() will require the PTL, and make sure the PMD mapping is unchanged via check_pmd(). But, IMO, we cannot assume that all callers always do the PMD mapping check. I suggest keeping it to prepare for corner cases :) > > > + > > + tlb_gather_mmu(&tlb, mm); > > You missed tlb_finish_mmu() to do tlb flushing, and ... Good spot! > > > + orig_pmd =3D pmdp_huge_get_and_clear(mm, addr, pmdp); > > + tlb_remove_pmd_tlb_entry(&tlb, pmdp, addr); > > I don't think tlb gather is helpful here, since you just flush one PMD > entry. Just using pmdp_huge_clear_flush() seems enough. Nice, thanks for the suggestion! I completely agree that using pmdp_huge_clear_flush() is sufficient here :) > > > + > > + /* > > + * Syncing against concurrent GUP-fast: > > + * - clear PMD; barrier; read refcount > > + * - inc refcount; barrier; read PMD > > + */ > > + smp_mb(); > > + > > + ref_count =3D folio_ref_count(folio); > > + map_count =3D folio_mapcount(folio); > > + > > + /* > > + * Order reads for folio refcount and dirty flag > > + * (see comments in __remove_mapping()). > > + */ > > + smp_rmb(); > > + > > + /* > > + * If the PMD or folio is redirtied at this point, or if there ar= e > > + * unexpected references, we will give up to discard this folio > > + * and remap it. > > + * > > + * The only folio refs must be one from isolation plus the rmap(s= ). > > + */ > > + if (ref_count !=3D map_count + 1 || folio_test_dirty(folio) || > > + pmd_dirty(orig_pmd)) { > > + set_pmd_at(mm, addr, pmdp, orig_pmd); > > Should we also call 'folio_set_swapbacked()' if the folio was redirtied? Yes. I forgot about that :) Thanks again for the review! Lance > > > + return false; > > + } > > + > > + folio_remove_rmap_pmd(folio, page, vma); > > + zap_deposited_table(mm, pmdp); > > + add_mm_counter(mm, MM_ANONPAGES, -HPAGE_PMD_NR); > > + if (vma->vm_flags & VM_LOCKED) > > + mlock_drain_local(); > > + folio_put(folio); > > + > > + return true; > > +} > > + > > +bool unmap_huge_pmd_locked(struct vm_area_struct *vma, unsigned long a= ddr, > > + pmd_t *pmdp, struct folio *folio) > > +{ > > + VM_WARN_ON_FOLIO(!folio_test_pmd_mappable(folio), folio); > > + VM_WARN_ON_FOLIO(!folio_test_locked(folio), folio); > > + VM_WARN_ON_ONCE(!IS_ALIGNED(addr, HPAGE_PMD_SIZE)); > > + > > + if (folio_test_anon(folio) && !folio_test_swapbacked(folio)) > > + return __discard_trans_pmd_locked(vma, addr, pmdp, folio)= ; > > + > > + return false; > > +} > > + > > static void remap_page(struct folio *folio, unsigned long nr) > > { > > int i =3D 0; > > diff --git a/mm/rmap.c b/mm/rmap.c > > index 08a93347f283..e09f2141b8dc 100644 > > --- a/mm/rmap.c > > +++ b/mm/rmap.c > > @@ -1677,18 +1677,25 @@ static bool try_to_unmap_one(struct folio *foli= o, struct vm_area_struct *vma, > > goto walk_done_err; > > } > > > > - if (!pvmw.pte && (flags & TTU_SPLIT_HUGE_PMD)) { > > - /* > > - * We temporarily have to drop the PTL and start = once > > - * again from that now-PTE-mapped page table. > > - */ > > - split_huge_pmd_locked(vma, range.start, pvmw.pmd,= false, > > - folio); > > - pvmw.pmd =3D NULL; > > - spin_unlock(pvmw.ptl); > > - pvmw.ptl =3D NULL; > > - flags &=3D ~TTU_SPLIT_HUGE_PMD; > > - continue; > > + if (!pvmw.pte) { > > + if (unmap_huge_pmd_locked(vma, range.start, pvmw.= pmd, > > + folio)) > > + goto walk_done; > > + > > + if (flags & TTU_SPLIT_HUGE_PMD) { > > + /* > > + * We temporarily have to drop the PTL an= d start > > + * once again from that now-PTE-mapped pa= ge > > + * table. > > + */ > > + split_huge_pmd_locked(vma, range.start, > > + pvmw.pmd, false, fo= lio); > > + pvmw.pmd =3D NULL; > > + spin_unlock(pvmw.ptl); > > + pvmw.ptl =3D NULL; > > + flags &=3D ~TTU_SPLIT_HUGE_PMD; > > + continue; > > + } > > } > > > > /* Unexpected PMD-mapped THP? */