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 6B80AC10F1A for ; Tue, 7 May 2024 04:44:48 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 898926B0082; Tue, 7 May 2024 00:44:47 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8485E6B0087; Tue, 7 May 2024 00:44:47 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 737616B0088; Tue, 7 May 2024 00:44:47 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 565A06B0082 for ; Tue, 7 May 2024 00:44:47 -0400 (EDT) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id C42BA140376 for ; Tue, 7 May 2024 04:44:46 +0000 (UTC) X-FDA: 82090359372.17.B62C813 Received: from mail-lf1-f53.google.com (mail-lf1-f53.google.com [209.85.167.53]) by imf14.hostedemail.com (Postfix) with ESMTP id E3A4A100006 for ; Tue, 7 May 2024 04:44:44 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=Y88JCp4z; spf=pass (imf14.hostedemail.com: domain of ioworker0@gmail.com designates 209.85.167.53 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=1715057085; 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=NDEi0vI8Iofmi+Jb2rsF44b3FI1pcGL98G5b9QzTf5k=; b=HtY7hCc4deJakoSWN2x0UHlgsb54HWfhGEF7lwyJq1DNRvV7dZEXFeXz/7l9pxb6BHFKV0 y2kJTjqhizqnb8tbsc6s6rLuTONjzmrLX3GKVOknBeD7Gh3bjXft+G1CpeD2zUfIiyycV9 x3PwVBp22A/Lxn8jmY7fngWVTtlMPfs= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=Y88JCp4z; spf=pass (imf14.hostedemail.com: domain of ioworker0@gmail.com designates 209.85.167.53 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=1715057085; a=rsa-sha256; cv=none; b=RvhQM1VeHyzfjT5dWDbRePMI7bLLEvBHuubekKP7aW9dYhCs+fklif1q+LpdxkxSXg0SgO PeFjquxs3X7jctVjLn1UvFi5DkPECeXFkoDHzTxP/Sqxdzk1SgKoLr6j1txfRzM2hZ5uC4 HEHordYSNmBBZoU1D4Fg29K5q+em1RM= Received: by mail-lf1-f53.google.com with SMTP id 2adb3069b0e04-52006fbae67so3722379e87.0 for ; Mon, 06 May 2024 21:44:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1715057083; x=1715661883; 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=NDEi0vI8Iofmi+Jb2rsF44b3FI1pcGL98G5b9QzTf5k=; b=Y88JCp4zxEwournJKIrZgFTNmrp18L1qpkQi4zF2+PPInEVAtBumHVFFBH30XZXDJt BmVHZzVz92YOwypM5tZdgiZB2bYBuP1CqSAeODlNI2wYtXj4GJdWkXqRVI5Yz+xJFjri PG0YJIXc+0tA3NRFqwbiyN6z4Z7CP4bkt6cB4R2OhqhBY9ZG4epF0hLVNdw4gnAVcRel hfWP9+7w4FQ0TR5mo4qpdb/Zfy0l7DNMYr0ZhpJ7npupUdEvj4vn/fXjOzj29RhrjLQT AZpuqKyiLEVmDnNY4yhFewOvgmDVZk3NmwjCIsob1m76051y5pld1zrI9pd8bDhhcoBH y1qw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1715057083; x=1715661883; 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=NDEi0vI8Iofmi+Jb2rsF44b3FI1pcGL98G5b9QzTf5k=; b=e4/9Q9iUqbxO92MCWcctZ539wTW15mkIykIuTfs/SSB3LOMFcp0pkRzjdS4Hsuw7qu dlHPbCZq9Osrw2f6eN1a+zR7q9qDRQ5qPtPusW6/BqHulkTz5a/KHjTl3QkTd5lPOHt3 EAzgfpUfJTm9gG1SOvJSOfLxrR2vCMYFI1c29P85ehI1gtZnS3qnjUriowjBX1uoGFMh P2CWhESnwccgTuphMTtvQNCNPrglRD6T+Sd3o6dwKMUDTqX8l000D5QQ3wbGmqD2+HPP s3Mk5whRX+J7hqB3nxsg1GPAW4k9HYqwaIUIeN0A8ONyz9yYfDzFQTEdgr+OZBIycwyf D4cA== X-Forwarded-Encrypted: i=1; AJvYcCVnXyXADgqSeO+vaYhtXy8Ghi93kC2g+6ZcNIMSc2ezOljQTttHDdRnsk2w+xicqPz2iZ+0PLcdHeVX5MBoXUjfgV4= X-Gm-Message-State: AOJu0YzxbhZ1gI+zkOHCswBg5GYmDO0zP8vdLKLpVWQ9CpnXAuDlk/Jj NKfQuVRl9bZCIVqElgNeQIK2RbsDSPACD6+Nb6XhPECSUGGV3YelpzQVeILh2YLAvg1G1ez3W0L T7dKMJJm0AaYpNdG8s5ljHXlx6iH1JN4M+xI= X-Google-Smtp-Source: AGHT+IGjnJ4I7BOw3fF6piZmjPbN81pLzS9+ifamu3i+TBYkXnV5ID2ExlH22y0NF+lLcXO0JIGg9JngmdGFQeVPVKc= X-Received: by 2002:a50:8d1e:0:b0:566:f5d6:4b4 with SMTP id s30-20020a508d1e000000b00566f5d604b4mr7087493eds.12.1715056641224; Mon, 06 May 2024 21:37:21 -0700 (PDT) MIME-Version: 1.0 References: <20240501042700.83974-1-ioworker0@gmail.com> <20240501042700.83974-3-ioworker0@gmail.com> In-Reply-To: From: Lance Yang Date: Tue, 7 May 2024 12:37:09 +0800 Message-ID: Subject: Re: [PATCH v4 2/3] mm/rmap: integrate PMD-mapped folio splitting into pagewalk loop 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-Queue-Id: E3A4A100006 X-Rspamd-Server: rspam06 X-Stat-Signature: tz5c75ii7j9hmur8drriuhnx46f9c8q6 X-HE-Tag: 1715057084-544242 X-HE-Meta: U2FsdGVkX18Y9aZ+1eZwhfrn7a4qtZHu/AvfVNkkIx/XeEZ2O3jCnTsDQagxLSSWt1E4L8PXS8Ntujtv48KRxLD8wEnYghsVqTG+eKYg7LSnyePpLgESjPkkNT/GRxNXm/dse26eNmCiLFHDKkND96ug5CUKw4QbGAJqrR0F0o0ck1VA33B+xpcs4TbIajZaZMVOvpaKMTGuCUfc6/b4WRhSZxYpmXrFuMFTHt+UWrXi0ZR+nn1tLe3V39Dlv+o1oAem8PgKRwkJL85Ft3mVe+ScTEyzwuM2ELy02KS70jFMl3EiOwH8S3T0m7VYV5a1pX/awNbKmABncFpB/RpD4L17ZYJCZBFJPdwxOX3ZIat+wFow9aaXnXOpnnor3cgt5xu0JTbG+jNmUxloL0H6TqBQOZ4panr7rHBFh4XbK7hrwGxG/fY5Fcl89kSAaZvPuv53x4S9i/JGI+bTFKEkjs7NfFEgnFl70/S5+PZDKeDw+J8TGLjpcDqCFrQ82OhoeUrYLDoTn4tX9yvBbxCxofGcx/F21g/EZKR8iF52jNM4/62g92NUAtrJ72zJzH0TNHHf4lpFQchEyjMEiAReZQ3BoWZFoRXW1nHYaFiHUUTplaqzIuUXaKrjon7rFXvXLONtDP3TvKZexpKFEot9/Q4liGAsRIL2V752WEX3XxfhO+AHCCLS7cGG9qJZww74u6ez3obdyILaN042ZIatKmsLmccAZfxo1oqH0dFKZ97vIulblqJ4xwacNjynNoVWyuAArnOPrd1XuDg130Za+KzMSNdGePkdi9JfL/aR0esbKRaveARir/QZkvjAuKa9o5AHTmEzOTyp+4/e82x/Gsl/Hikso28m+P36yOe7QcYdXQlEy/Tgc3/JQq9nvHMhsQcA3vD/fFxX3ogseCzczOYdIAq5yH3B5smOT/feOB1YA7E41JFk9boh8MQc4LbRnoJAvap9O+vszHeNa1O DXnO0Oi7 7VwtP5StSIP8BPzpXcrRl3INjgi0dO/aBVHzuJ7wf1kLSzsTJ5y88lUwOoUzTjyn241ZKg1AOnA+ISwDkKkEuwNIbtEtwsYF4kNp7tZi4lIB4jilTFXa0rKAO3Q8kOt1DLVMz1YkSDlTk/0krv+OHIjG2YIVhtX91bhPTXkTR15gFJQcr+xuURmbnegHwWBHoY3HDXGnMhN3dyXWZMy8FkQ9aBH84kJ1O3Ordk95uQzpbfotx3qRJTGpOTp3b03GwbdjgqS/n6BCnmQaNyQk21/1oouXt4gfvgFNbZbQppsRc45fI4gVNb5Odnh3GsItBTTHi2hyR3+MaErkOsQRrNQy99eUHKAfPIH0ij0QeuwZpf7rao+9Vi7HfSCSFwwzuI3n5PpTvpXV6rFVkSbDc2ekugd9w8LO4a2yY8RWN1Jkxw98= 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: Hey Baolin, Thanks for taking time to review! On Tue, May 7, 2024 at 11:40=E2=80=AFAM Baolin Wang wrote: > > > > On 2024/5/1 12:26, Lance Yang wrote: > > In preparation for supporting try_to_unmap_one() to unmap PMD-mapped > > folios, start the pagewalk first, then call split_huge_pmd_address() > > to split the folio. > > > > Suggested-by: David Hildenbrand > > Signed-off-by: Lance Yang > > --- > > include/linux/huge_mm.h | 20 ++++++++++++++++++++ > > mm/huge_memory.c | 42 +++++++++++++++++++++-------------------= - > > mm/rmap.c | 24 +++++++++++++++++------ > > 3 files changed, 60 insertions(+), 26 deletions(-) > > > > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h > > index c8d3ec116e29..38c4b5537715 100644 > > --- a/include/linux/huge_mm.h > > +++ b/include/linux/huge_mm.h > > @@ -409,6 +409,20 @@ static inline bool thp_migration_supported(void) > > return IS_ENABLED(CONFIG_ARCH_ENABLE_THP_MIGRATION); > > } > > > > +void split_huge_pmd_locked(struct vm_area_struct *vma, unsigned long a= ddress, > > + pmd_t *pmd, bool freeze, struct folio *folio); > > + > > +static inline void align_huge_pmd_range(struct vm_area_struct *vma, > > + unsigned long *start, > > + unsigned long *end) > > +{ > > + *start =3D ALIGN(*start, HPAGE_PMD_SIZE); > > + *end =3D ALIGN_DOWN(*end, HPAGE_PMD_SIZE); > > + > > + VM_WARN_ON_ONCE(vma->vm_start > *start); > > + VM_WARN_ON_ONCE(vma->vm_end < *end); > > +} > > + > > #else /* CONFIG_TRANSPARENT_HUGEPAGE */ > > > > static inline bool folio_test_pmd_mappable(struct folio *folio) > > @@ -471,6 +485,12 @@ static inline void __split_huge_pmd(struct vm_area= _struct *vma, pmd_t *pmd, > > unsigned long address, bool freeze, struct folio *folio) = {} > > static inline void split_huge_pmd_address(struct vm_area_struct *vma, > > unsigned long address, bool freeze, struct folio *folio) = {} > > +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 void align_huge_pmd_range(struct vm_area_struct *vma, > > + unsigned long *start, > > + unsigned long *end) {} > > > > #define split_huge_pud(__vma, __pmd, __address) \ > > do { } while (0) > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > > index 8261b5669397..145505a1dd05 100644 > > --- a/mm/huge_memory.c > > +++ b/mm/huge_memory.c > > @@ -2584,6 +2584,27 @@ static void __split_huge_pmd_locked(struct vm_ar= ea_struct *vma, pmd_t *pmd, > > pmd_populate(mm, pmd, pgtable); > > } > > > > +void split_huge_pmd_locked(struct vm_area_struct *vma, unsigned long a= ddress, > > + pmd_t *pmd, bool freeze, struct folio *folio) > > +{ > > + VM_WARN_ON_ONCE(folio && !folio_test_pmd_mappable(folio)); > > + VM_WARN_ON_ONCE(!IS_ALIGNED(address, HPAGE_PMD_SIZE)); > > + VM_WARN_ON_ONCE(folio && !folio_test_locked(folio)); > > + VM_BUG_ON(freeze && !folio); > > + > > + /* > > + * When the caller requests to set up a migration entry, we > > + * require a folio to check the PMD against. Otherwise, there > > + * is a risk of replacing the wrong folio. > > + */ > > + if (pmd_trans_huge(*pmd) || pmd_devmap(*pmd) || > > + is_pmd_migration_entry(*pmd)) { > > + if (folio && folio !=3D pmd_folio(*pmd)) > > + return; > > + __split_huge_pmd_locked(vma, pmd, address, freeze); > > + } > > +} > > + > > void __split_huge_pmd(struct vm_area_struct *vma, pmd_t *pmd, > > unsigned long address, bool freeze, struct folio *folio) > > { > > @@ -2595,26 +2616,7 @@ void __split_huge_pmd(struct vm_area_struct *vma= , pmd_t *pmd, > > (address & HPAGE_PMD_MASK) + HPAGE_PMD_SI= ZE); > > mmu_notifier_invalidate_range_start(&range); > > ptl =3D pmd_lock(vma->vm_mm, pmd); > > - > > - /* > > - * If caller asks to setup a migration entry, we need a folio to = check > > - * pmd against. Otherwise we can end up replacing wrong folio. > > - */ > > - VM_BUG_ON(freeze && !folio); > > - VM_WARN_ON_ONCE(folio && !folio_test_locked(folio)); > > - > > - if (pmd_trans_huge(*pmd) || pmd_devmap(*pmd) || > > - is_pmd_migration_entry(*pmd)) { > > - /* > > - * It's safe to call pmd_page when folio is set because i= t's > > - * guaranteed that pmd is present. > > - */ > > - if (folio && folio !=3D pmd_folio(*pmd)) > > - goto out; > > - __split_huge_pmd_locked(vma, pmd, range.start, freeze); > > - } > > - > > -out: > > + split_huge_pmd_locked(vma, range.start, pmd, freeze, folio); > > spin_unlock(ptl); > > mmu_notifier_invalidate_range_end(&range); > > } > > diff --git a/mm/rmap.c b/mm/rmap.c > > index 7e2575d669a9..432601154583 100644 > > --- a/mm/rmap.c > > +++ b/mm/rmap.c > > @@ -1636,9 +1636,6 @@ static bool try_to_unmap_one(struct folio *folio,= struct vm_area_struct *vma, > > if (flags & TTU_SYNC) > > pvmw.flags =3D PVMW_SYNC; > > > > - if (flags & TTU_SPLIT_HUGE_PMD) > > - split_huge_pmd_address(vma, address, false, folio); > > - > > /* > > * For THP, we have to assume the worse case ie pmd for invalidat= ion. > > * For hugetlb, it could be much worse if we need to do pud > > @@ -1650,6 +1647,8 @@ static bool try_to_unmap_one(struct folio *folio,= struct vm_area_struct *vma, > > range.end =3D vma_address_end(&pvmw); > > mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, vma->vm_mm, > > address, range.end); > > + if (flags & TTU_SPLIT_HUGE_PMD) > > + align_huge_pmd_range(vma, &range.start, &range.end); > > I am not sure why need this alignment? > (1) For a partially mapped THP, 'range.start' and 'range.end' can beyond > the VMA limits. For a PMD mapped THP, I think the address is already THP > size alignment returned from vma_address(&folio->page, vma). > (2) The range.end is not used. Thanks for pointing that out! I agree that this alignment is indeed redundant, and we should remove it. Especially for a partially mapped THP, it could cause 'range.start' and 'range.end' beyond the VMA limits, which could lead us into trouble. > > > if (folio_test_hugetlb(folio)) { > > /* > > * If sharing is possible, start and end will be adjusted > > @@ -1664,9 +1663,6 @@ static bool try_to_unmap_one(struct folio *folio,= struct vm_area_struct *vma, > > mmu_notifier_invalidate_range_start(&range); > > > > while (page_vma_mapped_walk(&pvmw)) { > > - /* Unexpected PMD-mapped THP? */ > > - VM_BUG_ON_FOLIO(!pvmw.pte, folio); > > - > > /* > > * If the folio is in an mlock()d vma, we must not swap i= t out. > > */ > > @@ -1678,6 +1674,22 @@ static bool try_to_unmap_one(struct folio *folio= , 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); > > IMO, you should also make the 'pvmw.ptl =3D NULL;' after unlocking as > page_vma_mapped_walk() did, in case some corner case met. Yep, I'll also set pvmw.ptl to NULL here if any corner cases arise. Thanks again for the review! Lance > > > + flags &=3D ~TTU_SPLIT_HUGE_PMD; > > + continue; > > + } > > + > > + /* Unexpected PMD-mapped THP? */ > > + VM_BUG_ON_FOLIO(!pvmw.pte, folio); > > + > > pfn =3D pte_pfn(ptep_get(pvmw.pte)); > > subpage =3D folio_page(folio, pfn - folio_pfn(folio)); > > address =3D pvmw.address;