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 987B2C54791 for ; Wed, 13 Mar 2024 14:03:11 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 08D838002D; Wed, 13 Mar 2024 10:03:11 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 03D2F940010; Wed, 13 Mar 2024 10:03:10 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E46F38002D; Wed, 13 Mar 2024 10:03:10 -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 D3B8D940010 for ; Wed, 13 Mar 2024 10:03:10 -0400 (EDT) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 7825F80FD0 for ; Wed, 13 Mar 2024 14:03:10 +0000 (UTC) X-FDA: 81892182540.23.8CCD815 Received: from mail-yw1-f180.google.com (mail-yw1-f180.google.com [209.85.128.180]) by imf21.hostedemail.com (Postfix) with ESMTP id C36321C0020 for ; Wed, 13 Mar 2024 14:03:05 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="YWQWgo/I"; spf=pass (imf21.hostedemail.com: domain of ioworker0@gmail.com designates 209.85.128.180 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=1710338585; 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=JK0XHQjO5WSH/o6x2gIUnZMlirm1AkRi9XQwO93zFbE=; b=2PgAYhmXzKo/rH+sXSsWu3m/Lf5/7IPvfF1NxKB738/8w4a5sNN2zfU0zVycGPtkmSPBBH EGC+4iLGhMluGJM1ta1g6zB1/ncr4wzJR6O+0b0yI7IT5bOF8DblKaMvnzBJQVWv652rwV 14AQ6+ByhHnIyNE+JXOPVx3xPrPbdIg= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="YWQWgo/I"; spf=pass (imf21.hostedemail.com: domain of ioworker0@gmail.com designates 209.85.128.180 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=1710338585; a=rsa-sha256; cv=none; b=dhksN8fsall0u7haGJEqkBWOaAf+/S9KxONDVJVAHymWHSa04w0ot/R785+oa3loE8YygX xkE9dg4sKTUFXEq457t3kouOIjA4OPHiHqGO8LQt9JkPRJOS5NSRGK+W2qtsgz+GhoWys0 /Yy1b8vHzk85OrHHuHtM4ARKokYzYo0= Received: by mail-yw1-f180.google.com with SMTP id 00721157ae682-60a15449303so42832167b3.0 for ; Wed, 13 Mar 2024 07:03:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1710338585; x=1710943385; 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=JK0XHQjO5WSH/o6x2gIUnZMlirm1AkRi9XQwO93zFbE=; b=YWQWgo/ITzlSnyliFEquo9fEq3BjiGBnmo1WuN4VDosCe3fmooWJtlpLKuvvNWPkCp IhdyMmLjAHb7GAQvrYw2zQ5eQHNhr26etY6SSkarHaZwst61KTfyIBEHtOhMvykrzbf8 34XOOQvwX4WTVjCH3T7ZiuD03jhFz15aZAi+WZ+V6KNclOQt/TmrrjP9V01p9NPyUTOG aQOXjZjklSxU1SkhDKzfPOt61KznahBZ0Lv+sE5okcqCjC7VHQCtkG3g+mfE3/jWl42j caao0OVvMgKMSN2a7pzBukKswoWMamvoH/kUEoRvy0/bjjefPa9KBodz99T7P2miSFV+ N2fw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710338585; x=1710943385; 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=JK0XHQjO5WSH/o6x2gIUnZMlirm1AkRi9XQwO93zFbE=; b=KWWQahrXhxqwpNYEUdYc/rWxFpT/D+BHeff4+/a4e9ByYrYsYl+6xyTB3yHX4UFdtz sNup3cULTSDafVA29nqEni/Sds0shj5LaYtLNeJ0SXgsfqgnxUpsb92wgDVMfF/xpIX1 u4Y3Sze32Gw4wvakFgqdQe3PxH4AjM2T3M14tPHyDejQWlnMIFHjA0cErBKcrjAeSi0v dTbEiEyhEuT2YGa4mDA3Us7EqD+nyeWxyR8rKPm37HJqF/IWmbeZnItqdeem0EsVJKJb j0MtNVU6VpFS+28YYcfcPrsIoAoawmXH0rZtef9kxYKy7bQzpcyD5NTu27rfN9eDq44d 8ffw== X-Forwarded-Encrypted: i=1; AJvYcCVRKT0XL5eJMOSGv02eZ9vAoFgFh5430qnzkMPZ+dmsII/ZYyoGyev36FYn1hcTtY8X38HfN9Yxw6KU2OJICELosQQ= X-Gm-Message-State: AOJu0YwCeS8CdLCeG2eqr7VKlPZUthGxFpKN+Nmxi/hSgAnwtqL5YoZM NiK4ujVrl/ynfE8HFrWWl5PWffPtGuJXgjkK2HKIKnA5EU2jAn9y6warMbOlNAuxCrxm834wROP tw6pUh1vGoQZptu1g9Wl15RFrYo4= X-Google-Smtp-Source: AGHT+IEdATua92ZogM+M95mw8t+NJgO5bNgZ4k6YtcnvzQvIJZmud6j4FqiH5e25lwAvTtzM7/reB0mZmvzhH/4qJPI= X-Received: by 2002:a81:5250:0:b0:60a:425c:10cf with SMTP id g77-20020a815250000000b0060a425c10cfmr2754365ywb.9.1710338583128; Wed, 13 Mar 2024 07:03:03 -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: Lance Yang Date: Wed, 13 Mar 2024 22:02:51 +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: C36321C0020 X-Rspam-User: X-Stat-Signature: 9c89diq8gu74kkq7bu3p73qr6xcz1i89 X-Rspamd-Server: rspam01 X-HE-Tag: 1710338585-979863 X-HE-Meta: U2FsdGVkX19d16NoS6tLF5ENqGs+FC+D34xeAV2SD6Fj0SR8aTARf/pnlJYfzDduKuMztZnO1k6K4wu/OqYzwMnsMEPp0QWcrEtjVPcRNuHEYEdg1syyE9HuJmmRpigEPAYf8V6HbJ580bxARVKJpXSc2CKnPSXLFpLU2L03qjx4qdqJKG6xy9jLAEcfPFXPmXmgucCTDwpk09hhQjFKhyonrgf6VGBHdpNvRx64H+L3JndY1NfguKB5fN2wfG4fWuRQm2fr5BQLcg0bVhUS0aE7kK57OdG9w4vL2hKOobqnqLDMTCBAmiAicpFiebIYEX5Q/DLhYp8CBNX6peoYdJjw96GKECuknrckjlrUsO1ix6dHs0WYwNs8X/fvtdlETbI/D4nHd2TXEaKYEZx88C6OsVi8HpjaBd7HwoWSPNcWxo6jkePbb7MbIQ+4KGFwsDiRBUKPX66BAGED/Nwq0aNMA/QjkES6BOSUHrsGwQZZme3GrvECtMG7RtaUSmPS1IM25KZhr+dPkVG52XyCuklM5hSBxsMHK3pOSmROjik2Xr+kbyU8ofnVEc9tEQGdMAUHhZOUMC4B7Qe8/caINZq6u+dn8Z3+DKOHcq5uzc7Mgry3gIESJkyFGQch8aJ9nBqgkX1Pu8xfFl3SidJGsP6w65T2pVZPWDr/NbLFWikuwy+tgmcrq1YO0ruyVjb8mLUcKKCjkxCLnTx4VjaR1FjHbCJUK3+Dsf+3j7Z2HODdzyQnjjih+8BwwNBa+OT9Q3b0f+LuSdodlD5XacsQwjpK7vW7L8he8FtuA9lJGbGs3XPbYCyVacGAhKl8xqeUZizIKvzAckJbg6tK0aoLQ8NHBPa1MHPAkLYhDQNYkOVFmgm75z55pVjrF6SV2C6Gpbw05B2VBXZwvmY/4VnGIPS8M+AyiRdC+R85ODWNotDKvitEyeAUpbr0WD8fVCJkQoTsxLDb4BTrqiO1AmV W3Nh2fGK Grbpehwyt9EYYwLg39gp5qiixYG7bzZ2qGF8pSJctvfNfJZmYkm8ZNDUAWzw3u83YfFCPUmn1vWx/zfQdVU6tgCMYNg== 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 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 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) > 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 (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? 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 && !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); IIRC, some of the architecture(ex, PPC) don't update TLB with set_pte_at an= d tlb_remove_tlb_entry. So, didn't we consider remapping the PTE with old aft= er pte clearing? 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 for > madvise_free_pte_range(). > > > > > > > >> } > >> > >> /* > >> -- > >> 2.25.1 > >> > > > > Overall, LGTM, > > > > Reviewed-by: Barry Song > > Thanks! > >