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 2B875C25B75 for ; Thu, 6 Jun 2024 03:55:29 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9734D6B00A4; Wed, 5 Jun 2024 23:55:28 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8D4B86B00A6; Wed, 5 Jun 2024 23:55:28 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6657C6B00A7; Wed, 5 Jun 2024 23:55:28 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 423BA6B00A4 for ; Wed, 5 Jun 2024 23:55:28 -0400 (EDT) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id D234E140CF7 for ; Thu, 6 Jun 2024 03:55:27 +0000 (UTC) X-FDA: 82199099094.11.35E01B1 Received: from mail-ed1-f46.google.com (mail-ed1-f46.google.com [209.85.208.46]) by imf14.hostedemail.com (Postfix) with ESMTP id F39AC100004 for ; Thu, 6 Jun 2024 03:55:25 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=mOwmE5q6; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf14.hostedemail.com: domain of ioworker0@gmail.com designates 209.85.208.46 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=1717646126; 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=zwuCiWLAKDNqOkyMb2HE5nvZt4WSpv0eSc7Hotju51I=; b=ND0OIK6aufRXnHaWVs8LEhTTf8rVjkOBgxerT/Eu5dOqmVke701XOzssyLDFAnDtmPRPst k+trLdJaCbcOrsX+Pwmwf26YhMgdzX6auwVV9zbgkImtS5BUrnInqaLrSsd6/vdxjKEiVg yxueLFJt7NXXaCs6c54oLyZ72nwdnT0= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=mOwmE5q6; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf14.hostedemail.com: domain of ioworker0@gmail.com designates 209.85.208.46 as permitted sender) smtp.mailfrom=ioworker0@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1717646126; a=rsa-sha256; cv=none; b=1PVvFeB/YTjQytmLJZDjHUSMzWine7mDUcGLo4jzZyhSyr5C6ZsoEqL1gNEHeMDoEnhQ6m yDE+wmDdzs9BbK15q1jFDRQKvdFQHntwQJqmwFn+WuPAPq7fCo6tkVjXcAp7avkE4wqS9i V+BDty8rHwUy1/El7r4NAJchIwE+oq4= Received: by mail-ed1-f46.google.com with SMTP id 4fb4d7f45d1cf-57a6985ab58so222302a12.2 for ; Wed, 05 Jun 2024 20:55:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1717646124; x=1718250924; 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=zwuCiWLAKDNqOkyMb2HE5nvZt4WSpv0eSc7Hotju51I=; b=mOwmE5q6gFzpJYtnnqqwOQDgnrs1P+K4QV9ufztY7z0bw0eR5SCcmFsWR57ZT3Q+XK cjx8nZb/cUZ7z8PPjXCC9vBNt6jPuc7wG9fszY2wOWDtOAtZUMInqL3VS4B2+4y2y8wA A/0q3oTwzkW2xhCeS+2LfIaYGr3Al3ag3lzBeUeXyDSLglu+3jl03DB5UzHPVt8Tl1gL WpdXN+gYCqfiWSiU/kSGJ9Yc8OEmIllo2vkpPUCvIN0VkGnPZIxpZS4Lrg5EYIDhJt9k orAEBqLIlQQWRSe+4ixyBZRTPWv5jf9hQCis7VT+F0UsBYj9TyZZw3jvWx/pbZKytnRr gpIQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717646124; x=1718250924; 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=zwuCiWLAKDNqOkyMb2HE5nvZt4WSpv0eSc7Hotju51I=; b=h+ymPS7de5JmFeTzGLjlzzjd69bBaJnJQtZ3o35AlDrhpCfH668r+UvejRV09lYzn5 3eZtnD0Y13Z4rgGowAPqECatOAhaZqO8waq0OHOLQf8oj+ZApO8zxGX87MRiIIlBBr+8 GYvbZofAk6C6ZSv6z7n/YcihSHSiHqbHJjSMci34eKzScdhkLVuTctsbnH4vFid6zfEH UXoGCD6KqmJT8DSLtk5ZW18qMTSVgkdpBBpsFguQBITODWuTHBbfauGh4Hp6WxrBSDkM eMxUGKh6B5dD69dBg5Ux6WKcgQl0iROAN1oPHCPSCjvTQd0HhA+HD3+Rnx7Zgk3Uuq+0 +BXQ== X-Forwarded-Encrypted: i=1; AJvYcCUrg147pbSfqGLIsh5TBy06KtTPwTPTqAbE1Z9zFhUCX2CEZjW8DrxoabzkoYmA/utgALrNLbzXJpbT+1jEDpkoBRI= X-Gm-Message-State: AOJu0YwbpLERxgzjClbDPYEJc1rCYRgJinKWIH1YCYgnY0/+jDLLiVdv AQQ0U1FD96yiOkarkMcjSeWuRVsSZ1LSNesS8YKIRMngKQHRvVeOdcFbYLSTgVnGm3gbxhPsLZp Vsje13GEDI+XNziUyoF33BNQ8aQc= X-Google-Smtp-Source: AGHT+IHgwrML+4LDRnTpPeDCgixWl8mUVUFcKEtrgg+POneny9u/ICUeTtLD2xfHc3Pl22kHYnI+jZe7oz7f4aYcwoQ= X-Received: by 2002:a50:aace:0:b0:579:edab:eff6 with SMTP id 4fb4d7f45d1cf-57a8b6f8269mr3412557a12.23.1717646124105; Wed, 05 Jun 2024 20:55:24 -0700 (PDT) MIME-Version: 1.0 References: <20240521040244.48760-1-ioworker0@gmail.com> <20240521040244.48760-3-ioworker0@gmail.com> In-Reply-To: From: Lance Yang Date: Thu, 6 Jun 2024 11:55:12 +0800 Message-ID: Subject: Re: [PATCH v6 2/3] mm/rmap: integrate PMD-mapped folio splitting into pagewalk loop To: David Hildenbrand Cc: akpm@linux-foundation.org, willy@infradead.org, sj@kernel.org, baolin.wang@linux.alibaba.com, maskray@google.com, ziy@nvidia.com, ryan.roberts@arm.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-Rspamd-Queue-Id: F39AC100004 X-Stat-Signature: fjid9qpods75m8jqmiif9yuepag4cf6y X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1717646125-228876 X-HE-Meta: U2FsdGVkX1//Rr0TNreSyHw0GOItZVhiDciC6bI2KfmjYy5+fE7VtD0CmuYTbWATRmZSi22j5bqR0jmociiMokMVxqoD7/GhjDsUwY5up0p0di8Devgis6R9ru3BpmFbzAhcVPHeM1D1LdC/wPTXWitwF/GVbx5ZRHuFvoYCBtYvwTlFYwC1o4U27Xb4o/v5wvvS+R7Q0QmHZIZXK3QoKES81uJZQVKEEHkQr79nc5MprVe6e81qJDuRyKEl28ySz+rJrYlBTGFU6vlXRSylT9A/RIssnHuUSwf0Uu0oUqmVX8IY6xqC+UF6c55gWh3APpkINaZs3Sgk/MYZETB5zanwbIfuMzm/Y9czVBz0MIVAfIS8MPq26srFvRIJAqYpAEWyMR/uVqzzrwT9Kynpcc1hBXUwYG+4sXP9X6fYL+KbB/iH6wVHvZz4ePDtIUPBL088zMvjPTfCyyrjsaDWpszd/yAX927r2n1feP79ZyTeygexic8cP0ZLD1AkRIqwFoaWzbnaXISuXk1mqnkd8QtTvoDd7LTED6EH6n1Vso0aoCc0v0bCwgOTUJsPdMczhVsnnn06JdQxkI9lIREwefYwdfgXya4jqfEqLUbmui9b/4DH2GlahRguQ4UVbp5hgE7EACoTqFIixMPZYjg/l9+8kn42z/UsOG9ak5xaI+DqvyFpqYVrOZa+PHJF0kSVf7npfekkr6FasnWFoBsAE3H1MFtjaVTAw+Wwj24DZHZl2gv3oYk+2seDrf33sEkyf8lbgByf7NF600mmnhoDQTasrjQ56IA5GcceYYGV2WiafpwnUyyvml4ptpJg0N7yJFXnk0nSJzgxfvX0Ic5vb7tSmWKy2jCP9rU32Nz38UcPsGCBGG+MiqQRCXPa0J9PL/2YS+4JaqVY5UYUaI9CjAhtOcf4fVIy/MA82hKDysXudoqohXCcphVGS55A6Q2SmMDZIo/DvcWXzlTag/Z 3zPcs0Ko 3vcR02lvY2PaKqmFO++ZkskmZx6S73LtxKG1cWXlHoZU+8qcu5nhknX/LBwVOxI0xf9tuDo2xqHtSrv8uZMu9cthJJTUBgV6lm/JqU183r2ZNJ1GjPq1ys9k0/L3JlcWOTg2g5Zeeri2f+cXi8dsQf+toi6XemErTTHD5eFtki7ZX8SN4oYzb1AzNKZo2ljn39ysxlKVXVWM8qP9L+QuZ2hh50WOXz/5h56y3+xdp7/fO7Y8ziHa7m12kRe5zmxaI9TKG5vd8jWCoeMotmdrE4RVDYBPVLUfrl+FFr3z4iqa7Ki+T/kZSkE2YQ8dIIRGbg1Kd/j5r2DzDbMQfRC6IJGOfmslIoEcsxXx+sl83qF1vqjAe4Dv/qVABDF9MRs1e9BbF3Fs4djhfFu2z8jge9eagf+opJzFMSYnwWLXYpWOKMGc= 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 Wed, Jun 5, 2024 at 10:28=E2=80=AFPM David Hildenbrand wrote: > > On 05.06.24 16:20, Lance Yang wrote: > > Hi David, > > > > On Wed, Jun 5, 2024 at 8:46=E2=80=AFPM David Hildenbrand wrote: > >> > >> On 21.05.24 06:02, 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. > >>> > >>> Since TTU_SPLIT_HUGE_PMD will no longer perform immediately, we might > >>> encounter a PMD-mapped THP missing the mlock in the VM_LOCKED range d= uring > >>> the page walk. It=E2=80=99s probably necessary to mlock this THP to p= revent it from > >>> being picked up during page reclaim. > >>> > >>> Suggested-by: David Hildenbrand > >>> Suggested-by: Baolin Wang > >>> Signed-off-by: Lance Yang > >>> --- > >> > >> [...] again, sorry for the late review. > > > > No worries at all, thanks for taking time to review! > > > >> > >>> diff --git a/mm/rmap.c b/mm/rmap.c > >>> index ddffa30c79fb..08a93347f283 100644 > >>> --- a/mm/rmap.c > >>> +++ b/mm/rmap.c > >>> @@ -1640,9 +1640,6 @@ static bool try_to_unmap_one(struct folio *foli= o, 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 invali= dation. > >>> * For hugetlb, it could be much worse if we need to do pud > >>> @@ -1668,20 +1665,35 @@ static bool try_to_unmap_one(struct folio *fo= lio, 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 swa= p it out. > >>> */ > >>> if (!(flags & TTU_IGNORE_MLOCK) && > >>> (vma->vm_flags & VM_LOCKED)) { > >>> /* Restore the mlock which got missed */ > >>> - if (!folio_test_large(folio)) > >>> + if (!folio_test_large(folio) || > >>> + (!pvmw.pte && (flags & TTU_SPLIT_HUGE_PMD))= ) > >>> mlock_vma_folio(folio, vma); > >> > >> Can you elaborate why you think this would be required? If we would ha= ve > >> performed the split_huge_pmd_address() beforehand, we would still be > >> left with a large folio, no? > > > > Yep, there would still be a large folio, but it wouldn't be PMD-mapped. > > > > After Weifeng's series[1], the kernel supports mlock for PTE-mapped lar= ge > > folio, but there are a few scenarios where we don't mlock a large folio= , such > > as when it crosses a VM_LOCKed VMA boundary. > > > > - if (!folio_test_large(folio)) > > + if (!folio_test_large(folio) || > > + (!pvmw.pte && (flags & TTU_SPLIT_HUGE_PMD))= ) > > > > And this check is just future-proofing and likely unnecessary. If encou= ntering a > > PMD-mapped THP missing the mlock for some reason, we can mlock this > > THP to prevent it from being picked up during page reclaim, since it is= fully > > mapped and doesn't cross the VMA boundary, IIUC. > > > > What do you think? > > I would appreciate any suggestions regarding this check ;) > > Reading this patch only, I wonder if this change makes sense in the > context here. Allow me to try explaining it again ;) > > Before this patch, we would have PTE-mapped the PMD-mapped THP before > reaching this call and skipped it due to "!folio_test_large(folio)". Yes, there is only a PTE-mapped THP when doing the "!folio_test_large(folio= )" check, as we will first conditionally split the PMD via split_huge_pmd_address(). > > After this patch, we either Things will change. We'll first do the "!folio_test_large(folio)" check, th= en conditionally split the PMD via split_huge_pmd_address(). > > a) PTE-remap the THP after this check, but retry and end-up here again, > whereby we would skip it due to "!folio_test_large(folio)". Hmm... IIUC, we will skip it after this check, stop the page walk, and not PTE-remap the THP. > > b) Discard the PMD-mapped THP due to lazyfree directly. Can that > co-exist with mlock and what would be the problem here with mlock? Before discarding a PMD-mapped THP as a whole, as patch #3 did, we also perform the "!folio_test_large(folio)" check. If the THP coexists with mlock, we will skip it, stop the page walk, and not discard it. IIUC. > > > So if the check is required in this patch, we really have to understand > why. If not, we should better drop it from this patch. I added the "!pvmw.pte && (flags & TTU_SPLIT_HUGE_PMD))" check in this patch just to future-proof mlock for a PMD-mapped THP missing the mlock, to prevent it from being picked up during page reclaim. But is this really required? It seems like nothing should really be broken without this check. Perhaps, we should drop it from this patch until we fully understand the reason for it. Could you get me some suggestions? Thanks, Lance > > At least my opinion, still struggling to understand why it would be > required (I have 0 knowledge about mlock interaction with large folios :)= ). > > -- > Cheers, > > David / dhildenb >