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 ED6F5C54791 for ; Wed, 13 Mar 2024 09:19:42 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 816B380015; Wed, 13 Mar 2024 05:19:42 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 77844940010; Wed, 13 Mar 2024 05:19:42 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5A3B680015; Wed, 13 Mar 2024 05:19:42 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 43746940010 for ; Wed, 13 Mar 2024 05:19:42 -0400 (EDT) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 0C1621611B0 for ; Wed, 13 Mar 2024 09:19:42 +0000 (UTC) X-FDA: 81891468204.09.9275BAE Received: from mail-yw1-f172.google.com (mail-yw1-f172.google.com [209.85.128.172]) by imf03.hostedemail.com (Postfix) with ESMTP id 6507220014 for ; Wed, 13 Mar 2024 09:19:40 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=fShDi7O6; spf=pass (imf03.hostedemail.com: domain of ioworker0@gmail.com designates 209.85.128.172 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=1710321580; a=rsa-sha256; cv=none; b=1dCErx2oiWpznwpEOBG4Zc+Wp1p+5lxQANlV23NckuIr2hLvciaHQ9znY/+gnhRmZdoMJp lHjIVECzLypkc3e3ENFjd1iQpeKWP3n4UVZlf7O4WtbLuU7Ft1ExWTCJSyVWAXvyDnyfky Ml5/qBXSEbR3ozi0UlmOoZYPRd+ZOg4= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=fShDi7O6; spf=pass (imf03.hostedemail.com: domain of ioworker0@gmail.com designates 209.85.128.172 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=1710321580; 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=byrgibNpCPgvs0VjxR8ixo3YcvPfmoIHBrOfCYQ1v7w=; b=gbwJ2gbjOku0evQGw74YYUiQrPIxbqCuuF/9O5616UaEYlrKeyyt2zJjL435VRInxS7NHl jYVsi5TxpyQSzOyf7QTHe5FnS81GvL/BW4XjKhds2kRKCpoznuBLD9A7nuUgWUFXq7yvsJ q66iPS/Ev79QgP/42SE0ugvyK1xX/qg= Received: by mail-yw1-f172.google.com with SMTP id 00721157ae682-60a0579a955so54305207b3.3 for ; Wed, 13 Mar 2024 02:19:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1710321579; x=1710926379; 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=byrgibNpCPgvs0VjxR8ixo3YcvPfmoIHBrOfCYQ1v7w=; b=fShDi7O6QbK+HG4bYVz5qRtW0WVUFmrE9uWM/s8BD3j5bhO4Qfcfr8qvea9b6oiTVs IW0Jjik0HCupHVPyEs2ALcDf/iQAtJEYTekcrYRql3yDP0miCv6QyDwiSDsjk7MEBvL3 DC7/c2AFu4R2Nmg9zmAxhHUFMioLnmAYp9cZGv2MaR2ze6M7iafOGmAxXjSuWdq2nbGy i5A6o0pQc8s9XADHffKZPfcTYy2SXNgluSJaF19FNsnTsV06EN2NrtzOGhgljZqklYJZ ryb+WEBBiQ9efE0zzBrFxp43+l8LVPIlXMXjynCti7kSQNEfI6GopmkmUEAESDAaYHGt Mvmg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710321579; x=1710926379; 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=byrgibNpCPgvs0VjxR8ixo3YcvPfmoIHBrOfCYQ1v7w=; b=FRnxJj0d0AOiMxGK13tihEW71BHAWDcKelE2pspwzbx1GZmmWO/g2Qs3SZw5wEX6Ob kCEENAjqvt06GKda7x9MKjd67APKuO9YbGJzC4jVNiLAbtU4s27O72ruaw7gjgaHHonZ gtfoGO8l/SwAt47zW1mdGJAdln0IELDvy98zMhGWqRhygp6QsyPJUAHPHEQU9eIggbRT 85/3hfKDYhlHIC3F+qaJzwxQXw/UNZao08z1RDrk3NOl91EZjZXeoAYFzyl7QiidrzJ3 /FCLThayOSQBqrGxqlvtyYcme3GgpLeYQPKnD1GT7tbd18OH2NjPQX4Sc/1MQMFWgSNS xtNw== X-Forwarded-Encrypted: i=1; AJvYcCUXscGdjaWq5j8U5LxebcJVMBGj/pRIfJf2Zrrxv6cn+k2zcWu7rEZxJ7c4fpUIRaIFIZ4QwO53oefsFgl6eQrml6Y= X-Gm-Message-State: AOJu0YwobzNLoZAvD3YU+i6WIslvN3ZgO5ayIOYaMkgSKr2zf/OT6BYu aomGJpbuU53qMXKBLaQkaGHZqdc8htQRFf5d6/1i5F7Wir1i8WUMylX1F1zTjjyXueuAATtIfhc twyieH7dl3XBnh5pQNR7RV2Uu6Xk= X-Google-Smtp-Source: AGHT+IHoC7z1DfEvHDZgGApKmQBizeiAz+vhVlh1IGEGjHTO8li7DuWkTIapgNV4fRB6spOiUCyAq9xpeyEsXA+3N/c= X-Received: by 2002:a81:690b:0:b0:60a:4a:a668 with SMTP id e11-20020a81690b000000b0060a004aa668mr2015733ywc.27.1710321579292; Wed, 13 Mar 2024 02:19:39 -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 17:19:26 +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-Server: rspam08 X-Rspamd-Queue-Id: 6507220014 X-Stat-Signature: gx98bkwbr8kffz5wcdrgqf6hhcijm33w X-Rspam-User: X-HE-Tag: 1710321580-100895 X-HE-Meta: U2FsdGVkX1+5H5B1JVLXUVGwbe8hdeKTxJzHwfEYmEO2+a3EyVVA3bBd96LPnGqQzHAaKkN8GpJpvGaxnID/Mu5Nwi6pIYNpJkCENnvCTsza2EZJDtUuSsUIV1vLwNsXf2ELISt6gNgnz7anthohNXMfqtH9Zrt8e6ckUa0e4xT2ihq9KLycJqUb0CTFYnnaVkP0pOJiKjl+zoHKCVoF0cOJbaxl/BB+F6Mzjkp9Ck216BsT5Cf0aOBe9XVX9T5ayTHSv+Vd6uIeEaHLc5+Bn1S2pecicbKb/cQm/dwrvK0UCqhhaprekpSHJ0IM+8vRe6+w9BdqIdC7ZGOvtmdm3EY+RcK37kKNlPrYm9aHFnptR7NVz5R/W2nhGIrcN5UeIEUdRUGDGtLRxJViTWN8uH0Yc3oUeoaaD64BpYGPVmmcRxGSAYHZbeTkjw5VZ0fJLGadj3CvSfPkEgJ1irD+PtvGbdlu/ExjSrtzz728M98SPduVfFg8Ezz4yOG9zYOYnvQFLMP7rAarJAzkDkplOihdq5nykNQNZceSlii/+vm1V8crM1nZLp8lExdTrup1oS34p2PtHROwklp4KNpNdUIjvUO1Z1lZ8JykSpmNnqCRxxvXfK3xWpGm6h7OELgsRtS8ugTvvMnC3O3srBe7TFs3fJgVISmj9HLQaQgECPZWIMa9XKtbhhKMYPoHcI5QntuzRGracay294xAklYdbv7xcF04IYKvpwnZnODIcuj5UdRQWjkCOwmqPhyGdXTVSzVTedH9bdqgM6eT2/bHALcxIFoNMQL2OK82M4pekwPU3mf8TsWzeCXv8mFmkOp4AQD+ecN4FNojbsFOhPtpvHQUJF9D1zYPuWhHsfNSHbljw7VuYMBYHtMJsOEiguywM6cV1SGCTvZYRKirle1PnfhACH+vjnqbkXG9nOKQDF7dy3OslJ/AIAcxPulgty6A2P497SyQVIkjd49Afcu tjuN3hEo xB77PpzKIEbLrNdZJOhwo1x7fBkVhxtEI0OuWom4Ws7fLIODhQo3/D4ga1m8loqRgVpWWvek6Y+mxxRh+SSHbiX0BQQ== 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 (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(). Thanks a lot, Ryan! I'll make sure to follow a similar pattern for madvise_free_pte_range(). Best, Lance > > > > > > > >> } > >> > >> /* > >> -- > >> 2.25.1 > >> > > > > Overall, LGTM, > > > > Reviewed-by: Barry Song > > Thanks! > >