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 4D36DC54791 for ; Wed, 13 Mar 2024 09:16:54 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D1E8B94001C; Wed, 13 Mar 2024 05:16:53 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CCE11940010; Wed, 13 Mar 2024 05:16:53 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B6EE094001C; Wed, 13 Mar 2024 05:16:53 -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 A5CF1940010 for ; Wed, 13 Mar 2024 05:16:53 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 46A38C05E4 for ; Wed, 13 Mar 2024 09:16:53 +0000 (UTC) X-FDA: 81891461106.20.C21E2B0 Received: from mail-ua1-f42.google.com (mail-ua1-f42.google.com [209.85.222.42]) by imf24.hostedemail.com (Postfix) with ESMTP id A705F18001A for ; Wed, 13 Mar 2024 09:16:51 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=VH0TqkC+; spf=pass (imf24.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.222.42 as permitted sender) smtp.mailfrom=21cnbao@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=1710321411; 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=YbGysNzy8azciQHDutj0Ihd2Nn5dFJyFhOYOJqFccXQ=; b=20KKXBBSXZpmQeSXB3KOJbVsAnpSEie5RByAv6AqJwaJv2WNg7YFW97TXjdF6YzGopLaPF HW/7/JBGJLsT5/nYlNn767kF+waMl764lSBqCzwGt8J3nn7SFeBYBXR3iWUIpTgyL8wSgc fuDShOC97ImCbT+byV6YtGfInt+ia2s= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1710321411; a=rsa-sha256; cv=none; b=gwNufLW0l5zQPfn/EXwLFSakaTtSDt5b0hD2F2j244e7HKSd7FKii0RoY54paQE3Z3lJ+s BT8vB9C7y9fT2kGgAAgFEIPKYEnBkdS+y0/YoHJlAa9NIRJXzzqPvsfXVEwjL0ibvph/vz 8WZbaXneQK/+FvezJbgd2E5gQEGufIc= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=VH0TqkC+; spf=pass (imf24.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.222.42 as permitted sender) smtp.mailfrom=21cnbao@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-ua1-f42.google.com with SMTP id a1e0cc1a2514c-7ce4512d308so414145241.1 for ; Wed, 13 Mar 2024 02:16:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1710321411; x=1710926211; 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=YbGysNzy8azciQHDutj0Ihd2Nn5dFJyFhOYOJqFccXQ=; b=VH0TqkC+Z/Xp2fJeMU+YXYnSDnXN2JzH18lHYEP9owhVDSzQ8Rb3IIe8YGihV+zY3E ZJ56lG+BsEZL2cSds2mZLCAbySYfWBKhC+PYqT+BKaoK+nZLoGrVQEGovIyrTxXlG6yI XXawPIHsRkAh1G5ljVWSiVJ17Ml5CbhgGIVlRXia+Lhp1yGXu3jS4FgMWTDutygB6FgP VFV99jrDy2sdHx2l/9sRG2ivjF5CpCUUl4VGP0HryY431rpUA0tAxxiA1RuQI2xxLbhQ SeEoz6ZqU9qhvIDgKXO6YPNLAU1IhpYfIc9rtre4z4/gbdwSbGiyExh0h0ghNOTpVa88 19aw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710321411; x=1710926211; 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=YbGysNzy8azciQHDutj0Ihd2Nn5dFJyFhOYOJqFccXQ=; b=e7FWPGzgS6NonbumRgPn4mk1yCM2WiagiSzhwoqcq9MgOgg4qeGJ9ZTwXyaoocztyN j5L8TDge8tEX1MNc6Yqsoy9ADZK/HY9U05NtTTkAyKKeprEWyRiffzOrqL1c63bRkXWs dYLxNl4MsIwhZo34AgvnXlCGfWFptwqRU3jU/CoxyG/9DC5usLyzoFBghRZcUyv3W/RA z4UchSOn/GeN+y8UVAi3IV0qGx2u+TzKaZPU8fXimFDev1GM0t+SgFLkbtAOlLDtz7o1 b+3QPwcukqw6qptUacFX2y5797+4jA5NI5w7nW0/Y2RlUrK5X7hWSXWYqDMdMRfHlhP3 VcwQ== X-Forwarded-Encrypted: i=1; AJvYcCUSUFtsB7Lrw0OSdNY3UrcvuOHRb6/yLwyZlMD518jhem6LRjTIHy8xGq6xphS+CFKZQbYnuioTxbPkQP5U7lzE6qs= X-Gm-Message-State: AOJu0Yxe3JyWHsYDNBexjgzba2M/B9iEGWqwVsIbcpl8UnoTMZiCoxAw uUySMkhNActOAKlTyEJ4tCqAYA2VtekcED975nSaVQM8hzA3eLERnIrm7/6BnPTAqJFQEyXbIVx RWR7UMl3jX+xijOFa9yYkbb2hBlg= X-Google-Smtp-Source: AGHT+IHdLr+dSfOx+0apEmwfQsguD+wSPA81VYst3oPaFU7vcWvGMlEme/8iEUKyaXTE+PHL1Z51xqqp6EEQDFTP/p8= X-Received: by 2002:a05:6102:721:b0:474:c620:129a with SMTP id u1-20020a056102072100b00474c620129amr1095310vsg.17.1710321410524; Wed, 13 Mar 2024 02:16:50 -0700 (PDT) MIME-Version: 1.0 References: <20240311150058.1122862-1-ryan.roberts@arm.com> <20240311150058.1122862-7-ryan.roberts@arm.com> In-Reply-To: From: Barry Song <21cnbao@gmail.com> Date: Wed, 13 Mar 2024 22:16:39 +1300 Message-ID: Subject: Re: [PATCH v4 6/6] mm: madvise: Avoid split during MADV_PAGEOUT and MADV_COLD To: Ryan Roberts Cc: Lance Yang , 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: A705F18001A X-Rspam-User: X-Stat-Signature: u1y4me816k7axue7x57kgtxf47p3u9ca X-Rspamd-Server: rspam03 X-HE-Tag: 1710321411-30707 X-HE-Meta: U2FsdGVkX19W7xy/Y7PEmRBOJelOkfGKRaTXSwjJM/Z0izGi7PR1W1PbJLda3mMMGuha/TlGtg5PU7jc1xuWToHOcERboGLhKIsZFB+pisOC356rmShIrJGZdrTYhBQgi53akreg1dr8tjtVLHdMUDYNMJU7ImdQRATUCjyuqlpAhpm/Sk7F+SuwXeYDv9aqh+VVcJBnmDegp8Ql+RMj6DYiQzJd02IsleU/S7Eru0XBGV/uQPPFrjoXgBKTdos6T9b6CeIAIHUMdTwO9KmMCkd8pWQmeQkI5DbEZZAg21GJrTRKCVmbRr90wnewNBiihtc0uc2hyRnM767wTerLVJ689HnwH0kLofJEbRwygNoB0/3uyzAEEW17BVIniMpBaB6S2V4V1JwYX+MBRS1aoSRRZRqObp3NSTYvxb+v/Y1ibWx9KTLkHSGFmUhLa350M2wpB6RQwdfLY0l8BM2o1t8QyOzWJSLIxR+oc6R1Bq5LunRzAQZ2V3dSPCIHVrauQQn1LsQVqkc1+9UMVCfxN+tuUgNnkxXW55WToD3ZTJr2taQuSChaCcr6Wcu1V+kuiKy5dkxKutf+ulCXPE4vweQjkv7xWced4i+U0za/p6InUktjKHNAaYGFpNbcEZcmhii83kGUrlmZ/ORDwQS05E6KJORldKcyAQMMOVI8aH+X6ly/7pC3eJMBQ4wDROKbM9vNFRE2CLSd+OOeOgx+NH9UXElsOJgpRdqX1WLX9RhuV5sVv/MRVIUMKEL18N47kO1L7k4mei37WtgYMpPYoJddC6b8MEPUbf1Y63s5iURj5cuU4KMTH9ZAL1gItUaiFpUNLyzVUJ+3Zgh7ZHJJCd/wSe5rYrBfv6/Zrcc1+mxLu/XAGL+oXz32tZu5bK34rSVjwfflBhdJwsZIicbbyGTvYDKSnrbAgop3Wbef9tFN+WAD/Ly+VpBtGkQl4dDypx2itkNDa4dL60HdVpf QCGnOQ+U HtbrDr2KE/AS4030Cr12a3RlLiORMiNOfGyEFVsQwgWhbJYOhmc64x2zbQwgzg1C+h8YDuf5AwqImls99WOTkwbNtJw== 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, Mar 13, 2024 at 10: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 larg= e > >> 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-out, b= y > >> 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 covered = in > >> a separate series). > >> > >> Folios that are not fully mapped in the target range are still split, > >> but note that behavior is changed so that if the split fails for any > >> reason (folio locked, shared, etc) we now leave it as is and move to t= he > >> next pte in the range and continue work on the proceeding folios. > >> Previously any failure of this sort would cause the entire operation t= o > >> give up and no folios mapped at higher addresses were paged out or mad= e > >> cold. Given large folios are becoming more common, this old behavior > >> would have likely lead to wasted opportunities. > >> > >> While we are at it, change the code that clears young from the ptes 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 new > >> 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 batching= 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(pmd_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(pmd_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 only i= f we > >> - * are sure it's worth. Split it if we are only owner. > >> + * 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 whole.= If we > >> + * fail to split a folio, leave it in place and advanc= e 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_test_an= on(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, &pt= l); > >> - 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_DIRTY; > >> + int max_nr =3D (end - addr) / PAGE_SIZE; > >> + > >> + nr =3D folio_pte_batch(folio, addr, pte, ptent= , 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) what about "If (pte_pfn(pte) =3D=3D folio_pfn(folio) && max_nr >=3D nr_pages)" just to account for cases where the user's end address falls within the middle of a large folio? BTW, another minor issue is here: if (++batch_count =3D=3D SWAP_CLUSTER_MAX) { batch_count =3D 0; if (need_resched()) { arch_leave_lazy_mmu_mode(); pte_unmap_unlock(start_pte, ptl); cond_resched(); goto restart; } } We are increasing 1 for nr ptes, thus, we are holding PTL longer than small folios case? we used to increase 1 for each PTE. Does it matter? > 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 ful= l > mapping, so no need to call folio_pte_batch(). Just split it. > > > > >> + > >> + 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)) > >> + 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, pmd, a= ddr, &ptl); > >> + if (!start_pte) > >> + break; > >> + arch_enter_lazy_mmu_mode(); > >> + nr =3D 0; > >> + continue; > >> + } > >> } > >> > >> /* > >> * Do not interfere with other mappings of this folio = and > >> - * non-LRU folio. > >> + * non-LRU folio. If we have a large folio at this poi= nt, we > >> + * know it is fully mapped so if its mapcount is the s= ame 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(folio)) > >> continue; > > > > This looks so perfect and is exactly what I wanted to achieve. > > > >> > >> if (pageout_anon_only_filter && !folio_test_anon(folio= )) > >> continue; > >> > >> - VM_BUG_ON_FOLIO(folio_test_large(folio), folio); > >> - > >> - if (!pageout && pte_young(ptent)) { > >> - ptent =3D ptep_get_and_clear_full(mm, addr, pt= e, > >> - tlb->fullmm); > >> - 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 PAGE_= SIZE) { > >> + if (ptep_test_and_clear_young(vma, add= r, pte)) > >> + tlb_remove_tlb_entry(tlb, pte,= addr); > >> + } > > > > 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 for > madvise_free_pte_range(). > > > > > > > >> } > >> > >> /* > >> -- > >> 2.25.1 > >> > > > > Overall, LGTM, > > > > Reviewed-by: Barry Song > > Thanks! > >