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 7B93AC4345F for ; Mon, 15 Apr 2024 09:41:33 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id F187F6B0088; Mon, 15 Apr 2024 05:41:32 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id EC87F6B0093; Mon, 15 Apr 2024 05:41:32 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D905A6B00A0; Mon, 15 Apr 2024 05:41:32 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id BBA5C6B0088 for ; Mon, 15 Apr 2024 05:41:32 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 2D822C0417 for ; Mon, 15 Apr 2024 09:41:32 +0000 (UTC) X-FDA: 82011273624.07.B9DABF1 Received: from mail-ed1-f41.google.com (mail-ed1-f41.google.com [209.85.208.41]) by imf21.hostedemail.com (Postfix) with ESMTP id 4EEFF1C000A for ; Mon, 15 Apr 2024 09:41:29 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=MfZRjpgr; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf21.hostedemail.com: domain of ioworker0@gmail.com designates 209.85.208.41 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=1713174089; 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=vmvgoDnU5iPDaNfHEWNk9nMHC+RqFBpn8cszCtXpYEU=; b=jhXXY6qpik9iGmqIbMiFH24iLC7EjF2/1u322t1rKPzP47LzxXo83VmjIK2cGJKvP9iLF+ 8AKuuX9xa37YWpdJZvZQVmxYU1jFdXKGaesvrqE8+C+S5GOchjcBRJ5qdg186SOHYxeyXj tW7I4m47F9bf5GoC30wNCwd+Na0KC1w= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=MfZRjpgr; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf21.hostedemail.com: domain of ioworker0@gmail.com designates 209.85.208.41 as permitted sender) smtp.mailfrom=ioworker0@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1713174089; a=rsa-sha256; cv=none; b=wW6guM2YvZKwuCZac7dV1lyogd23roqeP4aKmdZMTfu4stEDMdJU/hBH5ciksAXvT74kqS 1saLL7dkjT3ORbOcefzxlD7Pj2VTPBhEpyxw6MuaQrcx16UTEzrLKAx2LkHELlc8LrJkOU 0IMpj1Cktvu5h8cYUD5+gd5SdCUoEfA= Received: by mail-ed1-f41.google.com with SMTP id 4fb4d7f45d1cf-56e1baf0380so3109924a12.3 for ; Mon, 15 Apr 2024 02:41:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1713174088; x=1713778888; 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=vmvgoDnU5iPDaNfHEWNk9nMHC+RqFBpn8cszCtXpYEU=; b=MfZRjpgrVJageiiDOf7tvDCY9avDmA6n+Igl8Zx5oYWgZql2AczJG+73fZkMieExdd h+ieF9OcIA/IwJV72UrRE5HRCOvthallMmwYS6xIkb+EH8SA1sdTy3k1flP/aaX+/Zox gEsB6+a++O6Jx8lpg83UPTspU6O5N1dvbzHOtScc17THoeP4FZDRM+N1z8lGTYXF35wf CZypOcWjoj3fP31McD7kDft6GWraAztuMyMWJ4riRkRkHIwVDntia72mjyzzMh0ecwZl 4NbWIvS0JySEQcxJzwZLjNC/UKRY9J7TuF+GD69ugDOW+OATfciXgPkJfDfsZl53Rfom 4XWA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713174088; x=1713778888; 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=vmvgoDnU5iPDaNfHEWNk9nMHC+RqFBpn8cszCtXpYEU=; b=nvPbp9KG6auFRDZ97/y1msN8qwT2K6hyAAhw6p9Xvb7P0m33pEhgF6OTNdG7Zt7gA9 lLpz37AdlGwVunHDmXUe28cC+EG6LLVM/PIDf4531bKq+0qPchPCtcUeSj9UW5ZQS8y+ hJUB9ZR27F7akCqLioqnyHe8XGpRFUCL+lU782ru4IQ2DLwZfV0VqqLkv0+kY7Ahm8db uZe1zKt/I13rE5MIOkGojdpS4PNHWUNPWN/GkimKbWWVuRP4HbdbHimrFdr22rU+trEA SzoHkBDWRmUcIOfJ8IFWh1qwr5qI6s1ntAHmYDK8avOgJiwRvWmFEm50tOlq7wAmnrXN +eXw== X-Forwarded-Encrypted: i=1; AJvYcCW29BF5rCVJkHr9wNttxEUylOk4AceD1p6td+mCVlB8u0M9K6F6LYNVDVTTVyQXZpdYOaLCumMpD3mBslrxltP01sk= X-Gm-Message-State: AOJu0YwaEYblqsQ8qFDv5H3aTYo/rQ++4Wv8mi2o/j7iQr/IxIECOfu9 Ii26NTYbvDOMebpiFWOtvMY6VI8HENpVnNE3dxFhHnPTD0RtdP/3gnvPEWr6gDcbUvAEtdF1h/p 6Rg5rqBvyYi7WfgykS/IYogS/jNI= X-Google-Smtp-Source: AGHT+IEbqfHAiws4+f53DJkjb2XkVtw3Iy6rmNtsHEqP4Je0cKaSTwBd5go4csOUYSYpO15EOo32AgPHDP1wrC9oGe8= X-Received: by 2002:a50:8e5d:0:b0:56d:f00c:2b13 with SMTP id 29-20020a508e5d000000b0056df00c2b13mr5423638edx.32.1713174087534; Mon, 15 Apr 2024 02:41:27 -0700 (PDT) MIME-Version: 1.0 References: <20240413002219.71246-1-ioworker0@gmail.com> <20240413002219.71246-3-ioworker0@gmail.com> <209b9341-8bd0-47fe-b8fd-9a0f02beeab0@arm.com> In-Reply-To: <209b9341-8bd0-47fe-b8fd-9a0f02beeab0@arm.com> From: Lance Yang Date: Mon, 15 Apr 2024 17:41:16 +0800 Message-ID: Subject: Re: [PATCH v6 2/2] mm/arm64: override clear_young_dirty_ptes() batch helper To: Ryan Roberts Cc: akpm@linux-foundation.org, zokeefe@google.com, 21cnbao@gmail.com, shy828301@gmail.com, david@redhat.com, mhocko@suse.com, fengwei.yin@intel.com, xiehuan09@gmail.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: rspam12 X-Rspamd-Queue-Id: 4EEFF1C000A X-Stat-Signature: nmshw87b3r1hkhjf94ntekniajr1sao6 X-HE-Tag: 1713174089-27937 X-HE-Meta: U2FsdGVkX1+cHhJ0rp1M8Rh/N3GBhK3SxzWLMJI+dnRYK34tKmPeqnltTDQweWY5CAElVmBOlDCl5oyAQzi4daOnqYfl3APJ+gI0rs7zEeXADnlVfFnDT4OgacU2AeBYRmzqnqr+HWyClQlXfGPGroPcH7z/8+EWtkBDWKJsSzkDUBWD33BQTD8O/G4h3YXfezcuiVtiIFFkO7x9Zb0qqrnKgH3rZdAgQ9ywl/ts1XAwCCNMvR0ukVd4AiYqkSIlfUHQ5N65ePM8G38L1kw5ctwqeJsCSbaPl3EdANp7OhobKvnQdMBqoKKY0BFPiU2Ggu1t5gyBgFP2mUAdvbfUmvlAiU8OqUdzyQ8nKcQBPUDCme0GIFc63pBNHiGjzAeFiHQY+BlYygCop9PNNdlX9An7SmbntP/chlHoIJ3hNoWcDP+vl3xJFiBhOZATyLXRUU1VaA7FWzfSQ0pPxkbVzGsfai0RCTNbrCpploJwR/U3PedlHLDGgGAnD/cRVU3NE+ScVfw2YaOup0s1WwglamHnO4lHuHaY+Ru6om+wXp2lvbphhRQB3JUnndeSCMCeY3HfbzAvWHKPiKcQBsYhiXgXrcnJu4ywLObEzSSbyBZllIDWcTPblWcpv0My+5kCpKI4MpjTcaKVCFDlPLm38lmUdiBZpjO1+g7K4c/4Yi8LwOZy4YAtUHZexQyNwbKbsNng8KhqIGJczLOhMRwIFcwGXDLY1S612TgVBH9P+YRXGQPrzvM7q/ewf581xYLGA+sD9qxKdpT79V/XbMx7SG+Ick1NKrODnBav/8yM++3mubButF87mdrjpGjDaudlini7ly63/ADbE8HYKmaJLqmlzxyA5khlurB2XOUhpbfHhMJH1Fn21FnpdcU+3CnZWjbs08DrprjNMWy8FV76ZF5+kGsLpmsyi5S2mFwv65szRIiFwf6a+Nua8EOUG5TKX5/95MPIhQTq+lLqQHO SVGcyzkp VhrUQV87JdH8tMtByudsfKFqSfhzzzmNjo5yroEr4RvMR0kLHr9spcSW1LKEX4ANuCmzsDATBH+y/PG7OS16345v2DsfiX4CciwbE3PTDuetWa22ui8KzoAja40ejbbrSq4zOFFMRMw3ZetQcdY1uhjC3PnTdoserGL7uMO8Wy73QO11IXEBZvYCRpU3sB3Gw0RwDi88znAuucaZALZkyQtieiocky9nwAtwYiMaERQ3UehqDE3+JRD98YZ6+6kUP4k28UmI7Zo9fv0ANxfrmIAkxvsExzpPE+YwX+TtZOuQEZalJ8Z4HZaCRz07wHqdmYWk9C8neAjERZVRc+8Ez2xu0CpBeRorNYDlWaml5MoY+DA8XmOdF1hd64uKLZBXm6my0gFOWQbo1MRag1NXajkRI5i+xtwkSIRTXrKWymMqbwLz+KznlK5OlQv+aFTHd8TabLfwtOijm+H7GdJsygQModUIDSuGA7J1+UqW5Pcyk9djKsnjtep99MmHgpVECl6IW 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 Mon, Apr 15, 2024 at 4:59=E2=80=AFPM Ryan Roberts = wrote: > > On 13/04/2024 01:22, Lance Yang wrote: > > The per-pte get_and_clear/modify/set approach would result in > > unfolding/refolding for contpte mappings on arm64. So we need > > to override clear_young_dirty_ptes() for arm64 to avoid it. > > > > Suggested-by: David Hildenbrand > > Suggested-by: Barry Song <21cnbao@gmail.com> > > Signed-off-by: Ryan Roberts > > No, afraid I haven't signed off yet! Actually, you've done most of this change, and I just do the legwork :) But I'll remove this s-o-b. > > > Signed-off-by: Lance Yang > > --- > > arch/arm64/include/asm/pgtable.h | 37 ++++++++++++++++++++++++++++++++ > > arch/arm64/mm/contpte.c | 28 ++++++++++++++++++++++++ > > 2 files changed, 65 insertions(+) > > > > diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/= pgtable.h > > index 9fd8613b2db2..f951774dd2d6 100644 > > --- a/arch/arm64/include/asm/pgtable.h > > +++ b/arch/arm64/include/asm/pgtable.h > > @@ -1223,6 +1223,28 @@ static inline void __wrprotect_ptes(struct mm_st= ruct *mm, unsigned long address, > > __ptep_set_wrprotect(mm, address, ptep); > > } > > > > +static inline void __clear_young_dirty_ptes(struct mm_struct *mm, > > + unsigned long addr, pte_t *pt= ep, > > + unsigned int nr, cydp_t flags= ) > > +{ > > + pte_t pte; > > + > > + for (;;) { > > + pte =3D __ptep_get(ptep); > > + > > + if (flags | CYDP_CLEAR_YOUNG) > > bug: should be bitwise AND (&). Good spot! Thanks! > > > + pte =3D pte_mkold(pte); > > + if (flags | CYDP_CLEAR_DIRTY) > > + pte =3D pte_mkclean(pte); > > + > > + __set_pte(ptep, pte); > > The __ptep_get() and __set_pte() are not atomic. This is only safe when y= ou are > clearing BOTH access and dirty (as I explained in the previous version). = If you > are only clearing one of the flags, you will need a similar cmpxchg loop = as for > __ptep_test_and_clear_young(). Otherwise you can race with the HW and los= e > information. Thanks again for your patience and explanation! I still got it wrong :( > > > + if (--nr =3D=3D 0) > > + break; > > + ptep++; > > + addr +=3D PAGE_SIZE; > > + } > > +} > > + > > #ifdef CONFIG_TRANSPARENT_HUGEPAGE > > #define __HAVE_ARCH_PMDP_SET_WRPROTECT > > static inline void pmdp_set_wrprotect(struct mm_struct *mm, > > @@ -1379,6 +1401,9 @@ extern void contpte_wrprotect_ptes(struct mm_stru= ct *mm, unsigned long addr, > > extern int contpte_ptep_set_access_flags(struct vm_area_struct *vma, > > unsigned long addr, pte_t *ptep, > > pte_t entry, int dirty); > > +extern void contpte_clear_young_dirty_ptes(struct mm_struct *mm, > > + unsigned long addr, pte_t *ptep, > > + unsigned int nr, cydp_t flags); > > > > static __always_inline void contpte_try_fold(struct mm_struct *mm, > > unsigned long addr, pte_t *ptep, pte_t pt= e) > > @@ -1603,6 +1628,17 @@ static inline int ptep_set_access_flags(struct v= m_area_struct *vma, > > return contpte_ptep_set_access_flags(vma, addr, ptep, entry, dirt= y); > > } > > > > +#define clear_young_dirty_ptes clear_young_dirty_ptes > > +static inline void clear_young_dirty_ptes(struct mm_struct *mm, > > + unsigned long addr, pte_t *ptep= , > > + unsigned int nr, cydp_t flags) > > +{ > > + if (likely(nr =3D=3D 1 && !pte_cont(__ptep_get(ptep)))) > > + __clear_young_dirty_ptes(mm, addr, ptep, nr, flags); > > + else > > + contpte_clear_young_dirty_ptes(mm, addr, ptep, nr, flags)= ; > > +} > > + > > #else /* CONFIG_ARM64_CONTPTE */ > > > > #define ptep_get __ptep_get > > @@ -1622,6 +1658,7 @@ static inline int ptep_set_access_flags(struct vm= _area_struct *vma, > > #define wrprotect_ptes __wrprotect_ptes > > #define __HAVE_ARCH_PTEP_SET_ACCESS_FLAGS > > #define ptep_set_access_flags __ptep_set_access= _flags > > +#define clear_young_dirty_ptes __clear_young_dir= ty_ptes > > > > #endif /* CONFIG_ARM64_CONTPTE */ > > > > diff --git a/arch/arm64/mm/contpte.c b/arch/arm64/mm/contpte.c > > index 1b64b4c3f8bf..bf3b089d9641 100644 > > --- a/arch/arm64/mm/contpte.c > > +++ b/arch/arm64/mm/contpte.c > > @@ -361,6 +361,34 @@ void contpte_wrprotect_ptes(struct mm_struct *mm, = unsigned long addr, > > } > > EXPORT_SYMBOL_GPL(contpte_wrprotect_ptes); > > > > +void contpte_clear_young_dirty_ptes(struct mm_struct *mm, unsigned lon= g addr, > > + pte_t *ptep, unsigned int nr, cydp_t = flags) > > +{ > > + /* > > + * We can safely clear access/dirty without needing to unfold fro= m > > + * the architectures perspective, even when contpte is set. If th= e > > + * range starts or ends midway through a contpte block, we can ju= st > > + * expand to include the full contpte block. While this is not > > + * exactly what the core-mm asked for, it tracks access/dirty per > > + * folio, not per page. And since we only create a contpte block > > + * when it is covered by a single folio, we can get away with > > + * clearing access/dirty for the whole block. > > + */ > > + unsigned int start =3D addr; > > + unsigned int end =3D start + nr; > > There are addresses; they should be unsigned long. May have been my error > originally when I sent you the example snippet. Got it. I'll sort it. Thanks again for your time! Thanks, Lance > > Thanks, > Ryan > > > + > > + if (pte_cont(__ptep_get(ptep + nr - 1))) > > + end =3D ALIGN(end, CONT_PTE_SIZE); > > + > > + if (pte_cont(__ptep_get(ptep))) { > > + start =3D ALIGN_DOWN(start, CONT_PTE_SIZE); > > + ptep =3D contpte_align_down(ptep); > > + } > > + > > + __clear_young_dirty_ptes(mm, start, ptep, end - start, flags); > > +} > > +EXPORT_SYMBOL_GPL(contpte_clear_young_dirty_ptes); > > + > > int contpte_ptep_set_access_flags(struct vm_area_struct *vma, > > unsigned long addr, pte_t *ptep, > > pte_t entry, int dirty) >