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 3D099C54E67 for ; Thu, 21 Mar 2024 01:39:00 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5B5B16B007B; Wed, 20 Mar 2024 21:38:59 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 58C316B0082; Wed, 20 Mar 2024 21:38:59 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 42CD76B0083; Wed, 20 Mar 2024 21:38:59 -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 30B466B007B for ; Wed, 20 Mar 2024 21:38:59 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id BA61A814B4 for ; Thu, 21 Mar 2024 01:38:58 +0000 (UTC) X-FDA: 81919337556.02.A5E53AF Received: from mail-yb1-f175.google.com (mail-yb1-f175.google.com [209.85.219.175]) by imf19.hostedemail.com (Postfix) with ESMTP id 14E991A0003 for ; Thu, 21 Mar 2024 01:38:56 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=dx2oWvto; spf=pass (imf19.hostedemail.com: domain of ioworker0@gmail.com designates 209.85.219.175 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=1710985137; 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=PbxvN/eptl8J7guLXsZRG6QV4UBWnSMiGDtR2pNZ6ks=; b=BQGtEmdHsRKd5a0LkpWHL8JB/JhVxOdm7DJENOq0gxLVjq3GF9rO37eWNC18QeK7N9mRZZ acD/OPwgZ3lgzIOXJqWMC3Yf4Qz/vusVQ6qiHNDfClXr3xvKhBeTsc7eSVBhhw2+Lv3dOQ QomgM5UAS9QgWvTaDhInD/OqyjfiMP0= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=dx2oWvto; spf=pass (imf19.hostedemail.com: domain of ioworker0@gmail.com designates 209.85.219.175 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=1710985137; a=rsa-sha256; cv=none; b=iiUzO99ZS5ELwRh/W1v9AQU+rzh5VX7ItB4tZaqXkjJ85YHRI42HovoVcX6i9tYoxwOV+T XvN2fjlh7jdQjSihkNEOd/hxI2n/+lKYADhokujAuyXnnuMu4NpAOHJzqQ1NRTbmBShJnn h9G2OeuCpq8QnDybGRKeK4DFC/rt1oY= Received: by mail-yb1-f175.google.com with SMTP id 3f1490d57ef6-dbed179f0faso1136546276.1 for ; Wed, 20 Mar 2024 18:38:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1710985136; x=1711589936; 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=PbxvN/eptl8J7guLXsZRG6QV4UBWnSMiGDtR2pNZ6ks=; b=dx2oWvtoNm71WdwmGzUffB5OB99/uhcEKJK8NJRI4m+rdwUlTcK4r2QdOoVM3mZrCE IGtdv5RtLgY5oa39kE0sll7MaSFhwoW1F98LK4gLwnkuAQURJjQu8Fo5fDYdYDMnhUO+ BiOYsJF6BTbgXlDjqUuQVagdP8lXZvMZno1BNA/oAeH3P7LcrlMn7BBBnxOV0w0xbuQc KyPvVgYBwttdjjNttEymsuV1z92dpFAtgkQb9EjFuzeNxCTOIupo+YZd3lashtmx+ht8 YJsz5aiMikkleuc7QWMfXsY+yUxJ6TEset3QPafSqkEMmjKkhf54xsYBAEfW//v8G8Wz Jojg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710985136; x=1711589936; 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=PbxvN/eptl8J7guLXsZRG6QV4UBWnSMiGDtR2pNZ6ks=; b=CP97T7b5z2EgOnqSEliuXcz/Ot0OwyD6Jkeuw8BmhviRREs8mR+vRRPWI3A6HYej5M TOu6yUGMlfb4fU36lA5+jde32/61AWFQay9jfn49UMG8SaT+ht6B4EmW17l/l4pp5Ra+ EoyKmpSUUpLB09yt3LWp1OTS3bQS5spqbgkZgahPLpKvIzCF7SWLY6a+/zp/CMtZeCXH tjMemvh5Dyvacs+T1b6mudDU4CSrPE+qIa+eJEVi3QHc8P8zxnHf/AaxQOCRrWYftNlN 65dp/78AdVcmJcmmEYuq3CYuMFyIIddvQJIhOoYOIUruhQGSnI8DL1Wbpq/QO4A3LaMq D0Yw== X-Forwarded-Encrypted: i=1; AJvYcCVL0RJ1fNAFYUnaL2Fb2ofWmY75WdlASXj42wYdw21eAOjatNoysuMgae6tCnCxAEdqx2tKJqzPPo+rr6tWXYGPf+s= X-Gm-Message-State: AOJu0YxeXI1jau11+H6e87zvxQz/2WKWWUqogt2G2Q0+QE6iH0C8Uh86 YOET02NIs8q/0v4tmsTCQ9Ck6T7xNIT+YcRj7RwPMxIZDuTdsDN0b7aaI3xD4KNM/YI5MAmKiTt 6ucMWNO4kpPkM8WywTbdNxVLwJR4= X-Google-Smtp-Source: AGHT+IF6JC7sHUsGz6eFmseFmwmTXyU02gOD/gXBx8x0uSObFTkB8UHCs+PFgXMlb/gUFVBUysAm0hXu6i46/zbbdxs= X-Received: by 2002:a25:9f07:0:b0:dc7:4645:83ab with SMTP id n7-20020a259f07000000b00dc7464583abmr989665ybq.0.1710985136089; Wed, 20 Mar 2024 18:38:56 -0700 (PDT) MIME-Version: 1.0 References: <20240311150058.1122862-1-ryan.roberts@arm.com> <20240311150058.1122862-7-ryan.roberts@arm.com> <7ba06704-2090-4eb2-9534-c4d467cc085a@arm.com> In-Reply-To: From: Lance Yang Date: Thu, 21 Mar 2024 09:38:44 +0800 Message-ID: Subject: Re: [PATCH v4 6/6] mm: madvise: Avoid split during MADV_PAGEOUT and MADV_COLD To: Ryan Roberts Cc: Barry Song <21cnbao@gmail.com>, Andrew Morton , David Hildenbrand , Matthew Wilcox , Huang Ying , Gao Xiang , Yu Zhao , Yang Shi , Michal Hocko , Kefeng Wang , Chris Li , 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: 14E991A0003 X-Rspam-User: X-Stat-Signature: wn57cikar4fxopx4wxkrgqr43zzrunfu X-Rspamd-Server: rspam01 X-HE-Tag: 1710985136-135098 X-HE-Meta: U2FsdGVkX18tTbtI7CV/Ylj1OyfxX+dYSZ/I+u/3kxfM6F3GqvdlaZHspbMZF32TROh5DPHn+qM7a4U63hVrgglEtBIxvbexaw6oywQnD20I5PajzLPby5QRhUU82Pe8wryq5CbDEzs09q1K/7nYaR3pfrW9rYkYYbS1nPCu6IJu1kLkGquAIjx754sAomgIfettzO/1OrzWviQvTa7VOsp/hERrmsXcowXogBLbZXhyl27Y0b0ldgh2yNq/xHkOehiyoGg4o87+Lv8LIUdbOWP304Za3TqIKmv8vZi2FPfUBg9x1iNQkUZjRWlAt49G/PmUwHji3qpPuiP9lAgY7rXJtgLmUOa+qLgrWhxOvnlZDohOrIhiRKvLiyNaZUcyQwSndJlcRQbqEmwTLjIMQruRA5pib/6QHgl/7JzA8eZOhoXkySaQBU6uJ78QwMkCFqKH51lEpfWpDXbfH39achI7unIuyVPQbaG7bSxHUcZDxq9x0r20dYNgfHiV1Bd3yIYUPhytp/kgK83XJB8f/P51IOV0JKuSmF/PmOcehrSqPvlRrgWTsWmbTHFoWUmfpqh+8b+cc623dnhreoIeLNwkYRW1Nu0coAW0LYsx/OUADXyyoJTtjPzbv3LjmnpzVHa1fpJSrR7ntJ7TU9j9jcv8tCdHHhMfaxqUc4ZT/mTaX3/Vgq37WgJuj0MH61Tru+tmg9u/3351n8xyuM4ME9cdslAQRfYeeNYoQCEkDn0DcsMwB+7f1+mMm1+++BxQocWGmG3hQ59Z6acsWZuzbszQDBwDeSpBL0A5HrV5WluPOWHNkofRAN5SR0ZidTBhqzjw3vsO31LEiAQbV4PM/YSNIJ+78VPxMvp1wfqQCWVFc5uCoUJ+I4dCG9BqRLayksqb9xYulxqTSUuhSPfC9tpW0nQmnwGu8PcwkGxPOrMYQD566Bs28XhvJiWF9t/Z49l0Z1CPzY/EGbK0U4Y 8w2CBslh Cw6hqHj2Nm2HHSTZIYrnJt/p4G0FlglKjUIZd53PYnz/vvR3d4Kdvocgh5qJAfPR4k87RSd6SdOHP3ngQhf8JmUZv0g== 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, Mar 21, 2024 at 1:38=E2=80=AFAM Ryan Roberts = wrote: > > On 20/03/2024 14:35, Lance Yang wrote: > > On Wed, Mar 20, 2024 at 9:49=E2=80=AFPM Ryan Roberts wrote: > >> > >> Hi Lance, Barry, > >> > >> Sorry - I totally missed this when you originally sent it! > > > > No worries at all :) > > > >> > >> > >> On 13/03/2024 14:02, Lance Yang wrote: > >>> On Wed, Mar 13, 2024 at 5:03=E2=80=AFPM Ryan Roberts wrote: > >>>> > >>>> On 13/03/2024 07:19, Barry Song wrote: > >>>>> On Tue, Mar 12, 2024 at 4:01=E2=80=AFAM Ryan Roberts wrote: > >>>>>> > >>>>>> Rework madvise_cold_or_pageout_pte_range() to avoid splitting any = large > >>>>>> folio that is fully and contiguously mapped in the pageout/cold vm > >>>>>> range. This change means that large folios will be maintained all = the > >>>>>> way to swap storage. This both improves performance during swap-ou= t, by > >>>>>> eliding the cost of splitting the folio, and sets us up nicely for > >>>>>> maintaining the large folio when it is swapped back in (to be cove= red in > >>>>>> a separate series). > >>>>>> > >>>>>> Folios that are not fully mapped in the target range are still spl= it, > >>>>>> but note that behavior is changed so that if the split fails for a= ny > >>>>>> reason (folio locked, shared, etc) we now leave it as is and move = to the > >>>>>> next pte in the range and continue work on the proceeding folios. > >>>>>> Previously any failure of this sort would cause the entire operati= on to > >>>>>> give up and no folios mapped at higher addresses were paged out or= made > >>>>>> cold. Given large folios are becoming more common, this old behavi= or > >>>>>> would have likely lead to wasted opportunities. > >>>>>> > >>>>>> While we are at it, change the code that clears young from the pte= s to > >>>>>> use ptep_test_and_clear_young(), which is more efficent than > >>>>>> get_and_clear/modify/set, especially for contpte mappings on arm64= , > >>>>>> where the old approach would require unfolding/refolding and the n= ew > >>>>>> approach can be done in place. > >>>>>> > >>>>>> Signed-off-by: Ryan Roberts > >>>>> > >>>>> This looks so much better than our initial RFC. > >>>>> Thank you for your excellent work! > >>>> > >>>> Thanks - its a team effort - I had your PoC and David's previous bat= ching work > >>>> to use as a template. > >>>> > >>>>> > >>>>>> --- > >>>>>> mm/madvise.c | 89 ++++++++++++++++++++++++++++++-----------------= ----- > >>>>>> 1 file changed, 51 insertions(+), 38 deletions(-) > >>>>>> > >>>>>> diff --git a/mm/madvise.c b/mm/madvise.c > >>>>>> index 547dcd1f7a39..56c7ba7bd558 100644 > >>>>>> --- a/mm/madvise.c > >>>>>> +++ b/mm/madvise.c > >>>>>> @@ -336,6 +336,7 @@ static int madvise_cold_or_pageout_pte_range(p= md_t *pmd, > >>>>>> LIST_HEAD(folio_list); > >>>>>> bool pageout_anon_only_filter; > >>>>>> unsigned int batch_count =3D 0; > >>>>>> + int nr; > >>>>>> > >>>>>> if (fatal_signal_pending(current)) > >>>>>> return -EINTR; > >>>>>> @@ -423,7 +424,8 @@ static int madvise_cold_or_pageout_pte_range(p= md_t *pmd, > >>>>>> return 0; > >>>>>> flush_tlb_batched_pending(mm); > >>>>>> arch_enter_lazy_mmu_mode(); > >>>>>> - for (; addr < end; pte++, addr +=3D PAGE_SIZE) { > >>>>>> + for (; addr < end; pte +=3D nr, addr +=3D nr * PAGE_SIZE) = { > >>>>>> + nr =3D 1; > >>>>>> ptent =3D ptep_get(pte); > >>>>>> > >>>>>> if (++batch_count =3D=3D SWAP_CLUSTER_MAX) { > >>>>>> @@ -447,55 +449,66 @@ static int madvise_cold_or_pageout_pte_range= (pmd_t *pmd, > >>>>>> continue; > >>>>>> > >>>>>> /* > >>>>>> - * Creating a THP page is expensive so split it on= ly if we > >>>>>> - * are sure it's worth. Split it if we are only ow= ner. > >>>>>> + * If we encounter a large folio, only split it if= it is not > >>>>>> + * fully mapped within the range we are operating = on. Otherwise > >>>>>> + * leave it as is so that it can be swapped out wh= ole. If we > >>>>>> + * fail to split a folio, leave it in place and ad= vance to the > >>>>>> + * next pte in the range. > >>>>>> */ > >>>>>> if (folio_test_large(folio)) { > >>>>>> - int err; > >>>>>> - > >>>>>> - if (folio_estimated_sharers(folio) > 1) > >>>>>> - break; > >>>>>> - if (pageout_anon_only_filter && !folio_tes= t_anon(folio)) > >>>>>> - break; > >>>>>> - if (!folio_trylock(folio)) > >>>>>> - break; > >>>>>> - folio_get(folio); > >>>>>> - arch_leave_lazy_mmu_mode(); > >>>>>> - pte_unmap_unlock(start_pte, ptl); > >>>>>> - start_pte =3D NULL; > >>>>>> - err =3D split_folio(folio); > >>>>>> - folio_unlock(folio); > >>>>>> - folio_put(folio); > >>>>>> - if (err) > >>>>>> - break; > >>>>>> - start_pte =3D pte =3D > >>>>>> - pte_offset_map_lock(mm, pmd, addr,= &ptl); > >>>>>> - if (!start_pte) > >>>>>> - break; > >>>>>> - arch_enter_lazy_mmu_mode(); > >>>>>> - pte--; > >>>>>> - addr -=3D PAGE_SIZE; > >>>>>> - continue; > >>>>>> + const fpb_t fpb_flags =3D FPB_IGNORE_DIRTY= | > >>>>>> + FPB_IGNORE_SOFT_DI= RTY; > >>>>>> + int max_nr =3D (end - addr) / PAGE_SIZE; > >>>>>> + > >>>>>> + nr =3D folio_pte_batch(folio, addr, pte, p= tent, max_nr, > >>>>>> + fpb_flags, NULL); > >>>>> > >>>>> I wonder if we have a quick way to avoid folio_pte_batch() if users > >>>>> are doing madvise() on a portion of a large folio. > >>>> > >>>> Good idea. Something like this?: > >>>> > >>>> if (pte_pfn(pte) =3D=3D folio_pfn(folio) > >>>> nr =3D folio_pte_batch(folio, addr, pte, ptent, max_= nr, > >>>> fpb_flags, NULL); > >>>> > >>>> If we are not mapping the first page of the folio, then it can't be = a full > >>>> mapping, so no need to call folio_pte_batch(). Just split it. > >>> > >>> if (folio_test_large(folio)) { > >>> [...] > >>> nr =3D folio_pte_batch(folio, addr, pte, ptent= , max_nr, > >>> fpb_flags, NULL); > >>> + if (folio_estimated_sharers(folio) > 1) > >>> + continue; > >>> > >>> Could we use folio_estimated_sharers as an early exit point here? > >> > >> I'm not sure what this is saving where you have it? Did you mean to pu= t it > >> before folio_pte_batch()? Currently it is just saving a single conditi= onal. > > > > Apologies for the confusion. I made a diff to provide clarity. > > > > diff --git a/mm/madvise.c b/mm/madvise.c > > index 56c7ba7bd558..c3458fdea82a 100644 > > --- a/mm/madvise.c > > +++ b/mm/madvise.c > > @@ -462,12 +462,11 @@ static int madvise_cold_or_pageout_pte_range(pmd_= t *pmd, > > > > nr =3D folio_pte_batch(folio, addr, pte, ptent,= max_nr, > > fpb_flags, NULL); > > - > > // Could we use folio_estimated_sharers as an early exit point here? > > + if (folio_estimated_sharers(folio) > 1) > > + continue; > > if (nr < folio_nr_pages(folio)) { > > int err; > > > > - if (folio_estimated_sharers(folio) > 1) > > - continue; > > if (pageout_anon_only_filter && > > !folio_test_anon(folio)) > > continue; > > if (!folio_trylock(folio)) > > > I'm still not really getting it; with my code, if nr < the folio size, we= will > try to split and if we estimate that the folio is not exclusive we will a= void > locking the folio, etc. If nr =3D=3D folio size, we will proceed to the p= recise > exclusivity check (which is cheap once we know the folio is fully mapped = by this > process). > > With your change, we will always do the estimated exclusive check then pr= oceed > to the precise check; seems like duplication to me? Agreed. The estimated exclusive check is indeed redundant with my change. > > > > >> > >> But now that I think about it a bit more, I remember why I was origina= lly > >> unconditionally calling folio_pte_batch(). Given its a large folio, if= the split > >> fails, we can move the cursor to the pte where the next folio begins s= o we don't > >> have to iterate through one pte at a time which would cause us to keep= calling > >> folio_estimated_sharers(), folio_test_anon(), etc on the same folio un= til we get > >> to the next boundary. > >> > >> Of course the common case at this point will be for the split to succe= ed, but > >> then we are going to iterate over ever single PTE anyway - one way or = another > >> they are all fetched into cache. So I feel like its neater not to add = the > >> conditionals for calling folio_pte_batch(), and just leave this as I h= ave it here. > >> > >>> > >>> if (nr < folio_nr_pages(folio)) { > >>> int err; > >>> > >>> - if (folio_estimated_sharers(folio) > = 1) > >>> - continue; > >>> [...] > >>> > >>>> > >>>>> > >>>>>> + > >>>>>> + if (nr < folio_nr_pages(folio)) { > >>>>>> + int err; > >>>>>> + > >>>>>> + if (folio_estimated_sharers(folio)= > 1) > >>>>>> + continue; > >>>>>> + if (pageout_anon_only_filter && !f= olio_test_anon(folio)) > >>>>>> + continue; > >>>>>> + if (!folio_trylock(folio)) > >>>>>> + continue; > >>>>>> + folio_get(folio); > >>>>>> + arch_leave_lazy_mmu_mode(); > >>>>>> + pte_unmap_unlock(start_pte, ptl); > >>>>>> + start_pte =3D NULL; > >>>>>> + err =3D split_folio(folio); > >>>>>> + folio_unlock(folio); > >>>>>> + folio_put(folio); > >>>>>> + if (err) > >>>>>> + continue; > >>>>>> + start_pte =3D pte =3D > >>>>>> + pte_offset_map_lock(mm, pm= d, addr, &ptl); > >>>>>> + if (!start_pte) > >>>>>> + break; > >>>>>> + arch_enter_lazy_mmu_mode(); > >>>>>> + nr =3D 0; > >>>>>> + continue; > >>>>>> + } > >>>>>> } > >>>>>> > >>>>>> /* > >>>>>> * Do not interfere with other mappings of this fo= lio and > >>>>>> - * non-LRU folio. > >>>>>> + * non-LRU folio. If we have a large folio at this= point, we > >>>>>> + * know it is fully mapped so if its mapcount is t= he same as its > >>>>>> + * number of pages, it must be exclusive. > >>>>>> */ > >>>>>> - if (!folio_test_lru(folio) || folio_mapcount(folio= ) !=3D 1) > >>>>>> + if (!folio_test_lru(folio) || > >>>>>> + folio_mapcount(folio) !=3D folio_nr_pages(foli= o)) > >>>>>> continue; > >>>>> > >>>>> This looks so perfect and is exactly what I wanted to achieve. > >>>>> > >>>>>> > >>>>>> if (pageout_anon_only_filter && !folio_test_anon(f= olio)) > >>>>>> continue; > >>>>>> > >>>>>> - VM_BUG_ON_FOLIO(folio_test_large(folio), folio); > >>>>>> - > >>>>>> - if (!pageout && pte_young(ptent)) { > >>>>>> - ptent =3D ptep_get_and_clear_full(mm, addr= , pte, > >>>>>> - tlb->fullm= m); > >>>>>> - ptent =3D pte_mkold(ptent); > >>>>>> - set_pte_at(mm, addr, pte, ptent); > >>>>>> - tlb_remove_tlb_entry(tlb, pte, addr); > >>>>>> + if (!pageout) { > >>>>>> + for (; nr !=3D 0; nr--, pte++, addr +=3D P= AGE_SIZE) { > >>>>>> + if (ptep_test_and_clear_young(vma,= addr, pte)) > >>>>>> + tlb_remove_tlb_entry(tlb, = pte, addr); > >>> > >>> IIRC, some of the architecture(ex, PPC) don't update TLB with set_pte= _at and > >>> tlb_remove_tlb_entry. So, didn't we consider remapping the PTE with o= ld after > >>> pte clearing? > >> > >> Sorry Lance, I don't understand this question, can you rephrase? Are y= ou saying > >> there is a good reason to do the original clear-mkold-set for some arc= hes? > > > > IIRC, some of the architecture(ex, PPC) don't update TLB with > > ptep_test_and_clear_young() > > and tlb_remove_tlb_entry(). > > Err, I assumed tlb_remove_tlb_entry() meant "invalidate the TLB entry for= this > address please" - albeit its deferred and batched. I'll look into this. > > > > > In my new patch[1], I use refresh_full_ptes() and > > tlb_remove_tlb_entries() to batch-update the > > access and dirty bits. > > I want to avoid the per-pte clear-modify-set approach, because this doesn= 't > perform well on arm64 when using contpte mappings; it will cause the cont= pe > mapping to be unfolded by the first clear that touches the contpte block,= then > refolded by the last set to touch the block. That's expensive. > ptep_test_and_clear_young() doesn't suffer that problem. Thanks for explaining. I got it. I think that other architectures will benefit from the per-pte clear-modify= -set approach. IMO, refresh_full_ptes() can be overridden by arm64. Thanks, Lance > > > > > [1] https://lore.kernel.org/linux-mm/20240316102952.39233-1-ioworker0@g= mail.com > > > > Thanks, > > Lance > > > >> > >>> > >>> Thanks, > >>> Lance > >>> > >>> > >>> > >>>>>> + } > >>>>> > >>>>> This looks so smart. if it is not pageout, we have increased pte > >>>>> and addr here; so nr is 0 and we don't need to increase again in > >>>>> for (; addr < end; pte +=3D nr, addr +=3D nr * PAGE_SIZE) > >>>>> > >>>>> otherwise, nr won't be 0. so we will increase addr and > >>>>> pte by nr. > >>>> > >>>> Indeed. I'm hoping that Lance is able to follow a similar pattern fo= r > >>>> madvise_free_pte_range(). > >>>> > >>>> > >>>>> > >>>>> > >>>>>> } > >>>>>> > >>>>>> /* > >>>>>> -- > >>>>>> 2.25.1 > >>>>>> > >>>>> > >>>>> Overall, LGTM, > >>>>> > >>>>> Reviewed-by: Barry Song > >>>> > >>>> Thanks! > >>>> > >>>> > >> >