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 91A31C27C75 for ; Thu, 13 Jun 2024 09:21:23 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2E92E6B00A5; Thu, 13 Jun 2024 05:21:23 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2985A6B00B1; Thu, 13 Jun 2024 05:21:23 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 138F16B00B2; Thu, 13 Jun 2024 05:21:23 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id E979A6B00A5 for ; Thu, 13 Jun 2024 05:21:22 -0400 (EDT) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 95DB712124F for ; Thu, 13 Jun 2024 09:21:22 +0000 (UTC) X-FDA: 82225322004.15.48E8003 Received: from mail-ed1-f52.google.com (mail-ed1-f52.google.com [209.85.208.52]) by imf02.hostedemail.com (Postfix) with ESMTP id 92B5D80008 for ; Thu, 13 Jun 2024 09:21:20 +0000 (UTC) Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=nhHThqlD; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf02.hostedemail.com: domain of ioworker0@gmail.com designates 209.85.208.52 as permitted sender) smtp.mailfrom=ioworker0@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1718270479; a=rsa-sha256; cv=none; b=sk3uIw6B3tUkkO4kgEXQw2SzdkKpJhIqhevOUDq8FdKr1NgfUqzak8QzBH1S9oeEZV+akK USRAZQO5L1XjEQm/wLCOmko4oLayvCu/AY4XMonEz/U2PtBQA2yGvLMpP8BUYVuyq97Ayu 5LeNjBkNG1mNgauoB0Ix62cIPGQGEfk= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=nhHThqlD; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf02.hostedemail.com: domain of ioworker0@gmail.com designates 209.85.208.52 as permitted sender) smtp.mailfrom=ioworker0@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1718270479; 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=ZRtqnD5t+ohGXkbp2lKM09Q2LnCUwBKJroE7NvFdMjI=; b=VDmXQEyXQlQrGpeu1NWs1IBFOLRtmE9f6I5Sozu9LAGEpOgmgsZ5cSQc8faV4Zi5I1/Ctt ftn271JcvZEko3lKoWH+sPv+d+sBvV3NpXjQ82mFGrBfkrFNI4xdzmhrUYKJUJc44iqME3 uOD/ANcGzoS9hz1Kuzmk44j/feaVuY4= Received: by mail-ed1-f52.google.com with SMTP id 4fb4d7f45d1cf-5751bcb3139so715354a12.1 for ; Thu, 13 Jun 2024 02:21:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1718270479; x=1718875279; 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=ZRtqnD5t+ohGXkbp2lKM09Q2LnCUwBKJroE7NvFdMjI=; b=nhHThqlDKHl3+WZF1NsG6dgDqkWFxHrOTrXgi79jXSo+Y6lzsuhCGGDgUM7FGndm1B UFEbVqv4Iw2RPATqrJwX7UQIbBxm1yb4GAJtQOX8msrMO2AQ0mAsL6H7AnMXkh2y76fB 2btSfFeUwGf0OWAMhSrYirTJx0AWUOafEejCMIFHYB83oWlvraGw24NdcBoDfFfxXDB+ WkZPaWRlBJ6iZDid0MM/RyWzb4nJvqH2pb0SOS0wZbLmjbHO3tPf9J+DE1z//ER/TFZa MKArlDCLo5Ud2aWPZLsniooDu3CoJkPpFhXU5rwoVFvMJAgrBbT0vztqHuBuE/TOwm8K xjIA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718270479; x=1718875279; 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=ZRtqnD5t+ohGXkbp2lKM09Q2LnCUwBKJroE7NvFdMjI=; b=tFl2nf2PumSvwRMtR5Wj0xDiW98aWcUsWM7yF4F+iBldTd4QsvkFUlBeQ9uh+LQJlK CjX7D7UnPfk6nMTavduITlY2/AP+IPoqaShVUOqElBGwjxaawY2l8S9uLFmKpwboQluG sQX2SMX3k2Dnru0/Sqw01TrahQ7eKKW9tKFrc8cYLr/mGJO5x5ZcuowrmsYDaX0LpE82 MedaBR9vPlBbPctSWxDEn2bgiNldPUMSoa1XbkHTJe3ZZXdIdUPYJUBcl58+xsycgc1V ETxFqsdzTtSdyuYMgUx0Dra++R3OHw6lBnaPrFYq48mdTKs95P1YFEZ6hpkqjdPXeBAu gteA== X-Forwarded-Encrypted: i=1; AJvYcCWXFpvfpDJiXyWc6T4SYie7kpL8gdpAR+27mmaeVkNkEnqPKjyprvBqYP+y0CIob7HLYHxFvYywYj0FItP+l6S6mLY= X-Gm-Message-State: AOJu0Yw3HtHP7/BsMgy3mMxblIucfbyE2TkPCiEd1qkcue/y2YWq1GjP LqMjsFIXykxRT29DsvZTkDBY6jsZADDiu76LpDFIN96oWKhB92Nn0V90H07RhDFnWv2dc3ZyOcq 4+TTBMeOhwVF5jzw6/41oSOkvpNY= X-Google-Smtp-Source: AGHT+IEuhg0v5vMIzqD0pYtPQG2SRGJinZeB6/vLBsysPeHGUCktyBgiRl+ZI1biCozI5+D2q92AiJ3o8kYmYc2So8M= X-Received: by 2002:a50:d6da:0:b0:57c:7413:a6e0 with SMTP id 4fb4d7f45d1cf-57ca9749920mr2871493a12.2.1718270478935; Thu, 13 Jun 2024 02:21:18 -0700 (PDT) MIME-Version: 1.0 References: <20240610120209.66311-1-ioworker0@gmail.com> <20240610120618.66520-1-ioworker0@gmail.com> <933c7339-2dbd-464b-b342-e4cff7ad75a3@redhat.com> In-Reply-To: <933c7339-2dbd-464b-b342-e4cff7ad75a3@redhat.com> From: Lance Yang Date: Thu, 13 Jun 2024 17:21:07 +0800 Message-ID: Subject: Re: [PATCH v7 3/4] mm/rmap: integrate PMD-mapped folio splitting into pagewalk loop To: David Hildenbrand Cc: 21cnbao@gmail.com, akpm@linux-foundation.org, baolin.wang@linux.alibaba.com, fengwei.yin@intel.com, libang.li@antgroup.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, maskray@google.com, mhocko@suse.com, minchan@kernel.org, peterx@redhat.com, ryan.roberts@arm.com, shy828301@gmail.com, sj@kernel.org, songmuchun@bytedance.com, wangkefeng.wang@huawei.com, willy@infradead.org, xiehuan09@gmail.com, ziy@nvidia.com, zokeefe@google.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 92B5D80008 X-Stat-Signature: 1ywcq6s1mccsn9pobs7p93drhr5a6t66 X-Rspam-User: X-HE-Tag: 1718270480-722879 X-HE-Meta: U2FsdGVkX19hQG/aVrbiEjw4a/5zt5x9nVhw5K2wrSag7deOE99U8fEp2txM9ucJDyMXSx24BzO2Ni4mXtMZgAI8eAitt+M3D8W44Oy8IIbIkpGjgfkOZtcyNbrswkT2cXSwE7BDBwcTSVVJFVAbDAFxiLqEnIltEBs5xR1XOc2DRC5U/Hcmvc39+KcbWOVBpjNBmjrV/sikksmCW5ihwqKRMP7Vgh+MCauq1PyooDcxG3kI1iFcWzKikLJoJ/NpCNfXh11ZC3Kd931n3R9T+YIy7wLUXB/EGOcK8yxHw+0tWEJ/onIJR7zLRiXjqyTEB3z69TSdcLf9KznQVRWcfXDflZk/uMBtqM/fyIeJu/RRP+orB9fQcfXyM85d8f3/3RxcBU8OHsKlaBWmsB2IBtyBIFnaXRkbG/t3TIqayndl2HF9o1slAoGVx8h7pGtHHLTsxp28nWeY0bXhLyFzLWqtZ1EmVgSUb77zth0Yz7tVh/GMGzK28v6yD4FbgN7Ej5/sPa5gkzHdtYV1DGluL0CfX7uYKWFHEZB9KFZNkXzIhCSDfX3wZDuKV/xB3agPetrEOgQAxxmWZ03KP2L67v2laoc+jVwRXvQ7bjF1TE1yWtihR1BgqGC/8cjNnNo2a4acufIgsUwvJemTCz3PLCD8fvCPr5sdlc+eCJ9Fd6AaxHZQsF7mQSEykM9GKAJ1KCzgdqqUYJADBNIp66EhxakoplUkzqBjJQhrK1dF+xg4z4M42AY2eM/cVhlF/aKvQBYGa3W+PPpsOF6Jxk+xF9tWXPz9S/dEgUhTtkZQm1PGOAiQi4nU+hYc7Y1UiYRdIlkv5v6ze4ool5YkMoAODeet3QzWs6RK9ojABp6qJ8KI+DlcnckGgM3xeubJVeXvHdFnUrsCa2xwhwiWEMT+JMM20J5CotoCM3inXG7+PXbFFIuJANWZa+1n75ZemVUsqEhe5yZFPn+YE1d2Xnz bxbtbDsK x0TtNOca9KsykwMfPA0aMr0bpOzym/VOFWvvXoW+TcY/bOJP4Qe4JYokcta8H5+Bvz5G5E/T+IKFxDGm1sF8SVdy7Poel5HSGnkziQM79so4AatchpVmvrFKiuhCek4VHfXzFg3pvOsOB75MjyUyOvLsuMJg7UkZtf0Jluo1dFhdlXMz2+EbSP+l6r5kc0t4cpMbxTSpXC5UU+om2zIdXc4/TGU+WxLMoVkoQEYpuiZGcNxz0OPl2a96B4lLE2zJp/Bew1Ln8C2GXHrbB7ecLCvEwBrKyJLIDMjjoLg6i17JwBWLPnRsqhaMVOwR2JdI97n8iluKB7INCcN/3o9Po9et4soTnY9fX5jhMb3XrEtz5QaIDbsvRXBPGBw== 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 Thu, Jun 13, 2024 at 4:34=E2=80=AFPM David Hildenbrand wrote: > > On 10.06.24 14:06, 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 > > Suggested-by: Baolin Wang > > Signed-off-by: Lance Yang > > --- > > include/linux/huge_mm.h | 6 ++++++ > > mm/huge_memory.c | 42 +++++++++++++++++++++-------------------= - > > mm/rmap.c | 21 +++++++++++++++------ > > 3 files changed, 43 insertions(+), 26 deletions(-) > > > > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h > > index 088d66a54643..4670c6ee118b 100644 > > --- a/include/linux/huge_mm.h > > +++ b/include/linux/huge_mm.h > > @@ -415,6 +415,9 @@ 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); > > + > > #else /* CONFIG_TRANSPARENT_HUGEPAGE */ > > > > static inline bool folio_test_pmd_mappable(struct folio *folio) > > @@ -477,6 +480,9 @@ 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= ) {} > > > > #define split_huge_pud(__vma, __pmd, __address) \ > > do { } while (0) > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > > index e6d26c2eb670..d2697cc8f9d4 100644 > > --- a/mm/huge_memory.c > > +++ b/mm/huge_memory.c > > @@ -2581,6 +2581,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); > > Curious: could we actually end up here without a folio right now? That > would mean, that try_to_unmap_one() would be called with folio=3D=3DNULL. try_to_unmap_one() would not be called with folio=3D=3DNULL, I guess. I just moved 'VM_BUG_ON(freeze && !folio)' from __split_huge_pmd() to here, and now __split_huge_pmd() will call split_huge_pmd_locked(). > > > + > > + /* > > + * 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) > > { > > @@ -2592,26 +2613,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 ddffa30c79fb..b77f88695588 100644 > > --- a/mm/rmap.c > > +++ b/mm/rmap.c > > @@ -1640,9 +1640,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 > > @@ -1668,9 +1665,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. > > */ > > @@ -1682,6 +1676,21 @@ 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, pvmw.address, pvmw.pmd= , > > + false, folio); > > + flags &=3D ~TTU_SPLIT_HUGE_PMD; > > + page_vma_mapped_walk_restart(&pvmw); > > If, for some reason, split_huge_pmd_locked() would fail, we would keep > looping and never hit the VM_BUG_ON_FOLIO() below. Maybe we'd want to > let split_huge_pmd_locked() return whether splitting succeeded, and > handle that case differently? Hmm... after calling split_huge_pmd_locked(), we also do "flags &=3D ~TTU_SPLIT_HUGE_PMD", preventing re-entry into this block, then triggering the VM_BUG_ON_FOLIO() below if split_huge_pmd_locked() fails, IIUC. What do you think? Thanks, Lance > > > + 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; > > -- > Cheers, > > David / dhildenb >