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 E0D7AC0015E for ; Wed, 26 Jul 2023 05:41:36 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2EF5B6B0071; Wed, 26 Jul 2023 01:41:36 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 29F816B0074; Wed, 26 Jul 2023 01:41:36 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1666A8D0001; Wed, 26 Jul 2023 01:41:36 -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 070AD6B0071 for ; Wed, 26 Jul 2023 01:41:36 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id CB6A7C0D2B for ; Wed, 26 Jul 2023 05:41:35 +0000 (UTC) X-FDA: 81052665750.04.88D2D67 Received: from mail-qt1-f181.google.com (mail-qt1-f181.google.com [209.85.160.181]) by imf04.hostedemail.com (Postfix) with ESMTP id 0C2DD40003 for ; Wed, 26 Jul 2023 05:41:33 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=UHzaH3GM; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf04.hostedemail.com: domain of yuzhao@google.com designates 209.85.160.181 as permitted sender) smtp.mailfrom=yuzhao@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1690350094; 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=8vB3ZsOk9cBST7fTSWJNjOBqYs2YpfH524LGGeQPGGI=; b=CnlUJvN9NsLzqFHYdtapinFMUxqCBsIf28a4vi2rw1ULeuKWn73bel9WSgBwySoUq8VPP3 VjI74+yGl8duhZmx0r3scceKL+1+c7xAtvOcIyRQ+fzr+AJvj58jQLfDSaABXs26uh7d7v A6xJEjiHGKj2+tEGuzl126kJxvdSyNc= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=UHzaH3GM; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf04.hostedemail.com: domain of yuzhao@google.com designates 209.85.160.181 as permitted sender) smtp.mailfrom=yuzhao@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1690350094; a=rsa-sha256; cv=none; b=XoYfPeWDZ7PCe6rruyIBz2Xmh5qwZ1b+OORCL/VlSJpMnNFgAovfyrv4j+WzoVVc7jHfNb NZyEUmPV+alA1yuvNHC0ux/+dzrTjrNzd7GxyxiDA56IMdCeYufJB1hQMYv70lZnjawlbX eEOedwouPuZrodi+kQLybGDt4aCaKQ8= Received: by mail-qt1-f181.google.com with SMTP id d75a77b69052e-4036bd4fff1so231661cf.0 for ; Tue, 25 Jul 2023 22:41:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1690350093; x=1690954893; 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=8vB3ZsOk9cBST7fTSWJNjOBqYs2YpfH524LGGeQPGGI=; b=UHzaH3GMBlUN53PXMgiPoDwHZvBsNNsXvdAXb1Ekxe6UUmqyn7kMSP90Lzj6GFrH/P Y0MI9lNMzk9XywbShvsxGRsiQ5et4/rn9WQcy5TyHqtJpoPfUzVGSsidg2FLy6xQy5kS A6WCbsca1dyj0G2yTj4GJVY4w5Ix+Erg92OuC8cFyi5vhuMGEulFcnpFUFDOfuDxfzEt wfVM9TLPOz5GpESsZtAcHne/Z3ervD77SSsoz52/AxGzJJ4rr9UrwtwJvaSgJDcAj/y+ lNuopdHog6PPBtDPCt4wU00ZSdn1mn6x7jcvW79M0d7uqetC4LF9llDAMC6Q/izdCsFa sydg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690350093; x=1690954893; 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=8vB3ZsOk9cBST7fTSWJNjOBqYs2YpfH524LGGeQPGGI=; b=dYSRJz8mu4WsiS5QuUOQnDVZ4cABTSq1nzN2tLAVf00owkQmeJEgn1GtfCJGGBi2J9 o199erjknH4rv9DxxMEayGKviOvMJ4bPsaK/kEgQsHo8wA/8SmdKb7rVAtWSqzzHK27E SpLAHiKZw/DIwQnUQ5fawmROYQquvaIOipl01pL4FWvuQkmPgE8QWMfQYBCdHX6foHaa 7fTgdMuBodyZ6zVeL+KDlsZFJ9Cq8gKlF/yKWPq/YxulUcs/pKK1YflxtLmxU26XuAhK tOzjluBM4DBAsut3tTIZ8U7ZkLKZCrdSeqfh8Je/a+G7bdmbFgsBV6TweQYbVz0vFirL Ipnw== X-Gm-Message-State: ABy/qLY2aDXYyxNx+MJUISR/6RCPqCxKrQYBd9Ev2R1L0HJoydAEyO4n 8vq30bf7JWB9YyNHGKA5472Xn3wRzY4Pd5F2w6PwrQ== X-Google-Smtp-Source: APBJJlEj+qdOhXPl2y8FC9vEJhkxeCPvWBDORu6ehn7BGavUlsMyMd3oTqBekQxBjtl/mHLHK4C/W3zyH0YTgme3mCo= X-Received: by 2002:a05:622a:1756:b0:3f8:e0a:3e66 with SMTP id l22-20020a05622a175600b003f80e0a3e66mr369800qtk.3.1690350092955; Tue, 25 Jul 2023 22:41:32 -0700 (PDT) MIME-Version: 1.0 References: <20230721094043.2506691-1-fengwei.yin@intel.com> <20230721094043.2506691-3-fengwei.yin@intel.com> <05bc90b6-4954-b945-f0d8-373f565c1248@intel.com> In-Reply-To: From: Yu Zhao Date: Tue, 25 Jul 2023 23:40:56 -0600 Message-ID: Subject: Re: [RFC PATCH v2 2/4] madvise: Use notify-able API to clear and flush page table entries To: Yin Fengwei Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, akpm@linux-foundation.org, minchan@kernel.org, willy@infradead.org, david@redhat.com, ryan.roberts@arm.com, shy828301@gmail.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 0C2DD40003 X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: 1qysz4d1qgw4idbazwd3d6kgzfnrwmqu X-HE-Tag: 1690350093-227948 X-HE-Meta: U2FsdGVkX1+s+/0kUG/sW0B2BiJiEq+M6DdyS2ywZAjZVX+iLyyee/PCVUECCsalJdshgPUbqriRnpKC+6eJ4F8r3GJf0F3lqI2QH1qhW4xCdFjm/hNDQwmA6ub6Ci7HQY917XFbPaz1Tm6PLfFyzL+MOK0zRaES250X0WSnDNDlk30OMgaEPMBQK2hkStOB0oC6IGNWKSf4iY0jJx0ayn/rICH8VV1QJWSyd12OKP2htgs+MYP5S+WaSbTIkbxA6Ine4LWS05SMNlM5XBDIk/2sNWOAykaZ8Tid96Hp0Yn0BBBulmbibob3x/DddEQSKngcQXVphf87xrYTH5cU1G8+43zy4HG1UwoDtC6za1RL7tPXjXAGmza1tgN4qCi0CcW1/NvS0WFkpz0sbui1SlJKobtGuV9S1cjvSyMbjr6cm/hE/IPyFH6srkhHECerUAY09N7dBFWi28i+jSwjWIssf/o8FbTodDsB6abGW83Fh3E/v8jHEwjXwQeAo8dIcDeD7Qh23VuQaQUoRYNBOVubsNdicrC5WymnQw+9ItJf3irGFW+iXZz4txbt6H+6a/S0OBRdSYmWtLqiqcT+ZK0S6V8kjMVHNvRU9aDAZHilIeDfObGXQf2H0PWozoNoYBCIFyRNRgOuaSPcC4ScP7Ubb+vr4RFWEEmReRoaVtk0exMcJrdIfe8amI6OdsOlCDn5Ju5dIljMsEr/4JIAbcOJybG0O5DhIZut8fgABoeJ6WkxKUWMcyQjFXSyl1nV9Kug5QrdfaBHBucJwXF29idPD66hFyOfEWAaR7rLWwyQnwsRRCJdWo/5ONW8lfxh8eBTg7zhHDRTTFUvVxKgB9kFNUN1cGttCwXUwOR0kdBdzHXejChVL/oCJ/r0pE76xqZ5aZHssNgpInzep5zHrCJlhO3dtzoBkDFW8x+v5lOof6BLMmfaXz69KVFxdn997fm7z2Muj8WSVH1FUgK GSHAnbeg Sl7xQ5+AcFFX+85dAPlVrZyIQ/ohHRS8EVlXpkmrOlW8oMjEEodGCLVak5u/SlqRoPs68xdPTXqKidpL5kg8GxwuoI0NcjpzZVVfjV+T8y8xt/fqFCoGiNHeYExKm0/FgNDptX5/8Nwpd25lOb6BPytqj0EAE06cJrX6D/l1MflexLNWUyZIMNtMAJ3RxEkr2b7s4mVJzUQ92RJ4M0KBIHIGHdqV/U5AU1dsvIcyxkXf3KrhtG4Tx2OKjXk64X0wIcTJgkbC41ySgbJjhoDc8fmoT7rq3CqanZAajZotZ8ZkqCCPqn48jG01WIrv598oztdhTjO6IaIV8SYYaYuuqVYsGuT+hPhbuuO/6LD3l+2ANwRc= 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 Tue, Jul 25, 2023 at 10:44=E2=80=AFPM Yin Fengwei wrote: > > > > On 7/26/23 11:26, Yu Zhao wrote: > > On Tue, Jul 25, 2023 at 8:49=E2=80=AFPM Yin Fengwei wrote: > >> > >> > >> On 7/25/23 13:55, Yu Zhao wrote: > >>> On Fri, Jul 21, 2023 at 3:41=E2=80=AFAM Yin Fengwei wrote: > >>>> > >>>> Currently, in function madvise_cold_or_pageout_pte_range(), the > >>>> young bit of pte/pmd is cleared notify subscripter. > >>>> > >>>> Using notify-able API to make sure the subscripter is signaled about > >>>> the young bit clearing. > >>>> > >>>> Signed-off-by: Yin Fengwei > >>>> --- > >>>> mm/madvise.c | 18 ++---------------- > >>>> 1 file changed, 2 insertions(+), 16 deletions(-) > >>>> > >>>> diff --git a/mm/madvise.c b/mm/madvise.c > >>>> index f12933ebcc24..b236e201a738 100644 > >>>> --- a/mm/madvise.c > >>>> +++ b/mm/madvise.c > >>>> @@ -403,14 +403,7 @@ static int madvise_cold_or_pageout_pte_range(pm= d_t *pmd, > >>>> return 0; > >>>> } > >>>> > >>>> - if (pmd_young(orig_pmd)) { > >>>> - pmdp_invalidate(vma, addr, pmd); > >>>> - orig_pmd =3D pmd_mkold(orig_pmd); > >>>> - > >>>> - set_pmd_at(mm, addr, pmd, orig_pmd); > >>>> - tlb_remove_pmd_tlb_entry(tlb, pmd, addr); > >>>> - } > >>>> - > >>>> + pmdp_clear_flush_young_notify(vma, addr, pmd); > >>>> folio_clear_referenced(folio); > >>>> folio_test_clear_young(folio); > >>>> if (folio_test_active(folio)) > >>>> @@ -496,14 +489,7 @@ static int madvise_cold_or_pageout_pte_range(pm= d_t *pmd, > >>>> > >>>> VM_BUG_ON_FOLIO(folio_test_large(folio), folio); > >>>> > >>>> - if (pte_young(ptent)) { > >>>> - ptent =3D ptep_get_and_clear_full(mm, addr, = pte, > >>>> - tlb->fullmm)= ; > >>>> - ptent =3D pte_mkold(ptent); > >>>> - set_pte_at(mm, addr, pte, ptent); > >>>> - tlb_remove_tlb_entry(tlb, pte, addr); > >>>> - } > >>>> - > >>>> + ptep_clear_flush_young_notify(vma, addr, pte); > >>> > >>> These two places are tricky. > >>> > >>> I agree there is a problem here, i.e., we are not consulting the mmu > >>> notifier. In fact, we do pageout on VMs on ChromeOS, and it's been a > >>> known problem to me for a while (not a high priority one). > >>> > >>> tlb_remove_tlb_entry() is batched flush, ptep_clear_flush_young() is > >>> not. But, on x86, we might see a performance improvement since > >>> ptep_clear_flush_young() doesn't flush TLB at all. On ARM, there migh= t > >>> be regressions though. > >>> > >>> I'd go with ptep_clear_young_notify(), but IIRC, Minchan mentioned he > >>> prefers flush. So I'll let him chime in. > >> I am OK with either way even no flush way here is more efficient for > >> arm64. Let's wait for Minchan's comment. > > > > Yes, and I don't think there would be any "negative" consequences > > without tlb flushes when clearing the A-bit. > > > >>> If we do end up with ptep_clear_young_notify(), please remove > >>> mmu_gather -- it should have been done in this patch. > >> > >> I suppose "remove mmu_gather" means to trigger flush tlb operation in > >> batched way to make sure no stale data in TLB for long time on arm64 > >> platform. > > > > In madvise_cold_or_pageout_pte_range(), we only need struct > > mmu_gather *tlb because of tlb_remove_pmd_tlb_entry(), i.e., flushing > > tlb after clearing the A-bit. There is no correction, e.g., potential > > data corruption, involved there. > > From https://lore.kernel.org/lkml/20181029105515.GD14127@arm.com/, > the reason that arm64 didn't drop whole flush tlb in ptep_clear_flush_you= ng() > is to prevent the stale data in TLB. I suppose there is no correction iss= ue > there also. > > So why keep stale data in TLB in madvise_cold_or_pageout_pte_range() is f= ine? Sorry, I'm not sure I understand your question here. In this patch, you removed tlb_remove_tlb_entry(), so we don't need struct mmu_gather *tlb any more. If you are asking why I prefer ptep_clear_young_notify() (no flush), which also doesn't need tlb_remove_tlb_entry(), then the answer is that the TLB size doesn't scale like DRAM does: the gap has been growing exponentially. So there is no way TLB can hold stale entries long enough to cause a measurable effect on the A-bit. This isn't a conjecture -- it's been proven conversely: we encountered bugs (almost every year) caused by missing TLB flushes and resulting in data corruption. They were never easy to reproduce, meaning stale entries never stayed long in TLB.