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 41666E7AD77 for ; Tue, 3 Oct 2023 17:05:56 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id AF5A46B00F9; Tue, 3 Oct 2023 13:05:55 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id AA5476B0259; Tue, 3 Oct 2023 13:05:55 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 96F306B025C; Tue, 3 Oct 2023 13:05:55 -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 878316B00F9 for ; Tue, 3 Oct 2023 13:05:55 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 537BE1402E0 for ; Tue, 3 Oct 2023 17:05:55 +0000 (UTC) X-FDA: 81304777470.22.2D4DCD1 Received: from mail-yw1-f182.google.com (mail-yw1-f182.google.com [209.85.128.182]) by imf30.hostedemail.com (Postfix) with ESMTP id 88E6680006 for ; Tue, 3 Oct 2023 17:05:53 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="FNiTe/3p"; spf=pass (imf30.hostedemail.com: domain of surenb@google.com designates 209.85.128.182 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1696352753; 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=kE/nyAurwDzdK2tqw1L/eVudIzwCJX4Y+og2uP8HFDs=; b=76Ug/PECVMSlnej2crAw4htTOw0YEK3ePbDwX4I1Td5Epgm2cqIumO5rxRsZ2kwIJwGYcK ChDi7U51S69HVD3lpnSY63oaGs/dZ4AiFC4Ea3X+XWD5wTJRLDGO8NaQPz8mY/miRj0acz 7twBGqPXOqUPEjsHi1+0lAzcb7aJHls= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="FNiTe/3p"; spf=pass (imf30.hostedemail.com: domain of surenb@google.com designates 209.85.128.182 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1696352753; a=rsa-sha256; cv=none; b=woVA87SzMS1PMOxL9o0b6FdJFZfBx5L4PePemoAHCx30o86lE+F9OiqarhZZWvCZMJNICw eFbdOqE0gjXSc0MDCqhZCE87TI8kSx8EKa0w/rumymsNMUeQ94MM3vb4oY31ae3rpRsDeM q8FKXZbQCYawY2yxcgpQIe9uxh+ET+4= Received: by mail-yw1-f182.google.com with SMTP id 00721157ae682-59bbdb435bfso14033687b3.3 for ; Tue, 03 Oct 2023 10:05:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1696352752; x=1696957552; 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=kE/nyAurwDzdK2tqw1L/eVudIzwCJX4Y+og2uP8HFDs=; b=FNiTe/3pfvNEqDTzwhKEP6CneOmmSrN2AZAYWg3BCa9opTvZbKa919UNGjL2k2/tqx JSvkC829VXfSFbr/dmQl0WJsQXf+9PSgRZVAd4x/DdFtEYrCH1WIQ7De3aZw+W2e4TMp FGDZs2t8zHtwnuARidHvY0VGYBJa17RgWeebBsMhi2VZ5Evrzg3JzOG54YsGA6+1sWgZ ty1IWhIBKqcClUCWUtoLelT0XI6e8lnaurnr9l506XDPb/q8Qkay/TBVGbO3+BkkTGAr sdhJb4W7wDotCM1rLooc5DNXXzRNrLiil83YJE4NiyOBzLpjsN0Ftm1A0fMyJxU8OzVY J7Jw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696352752; x=1696957552; 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=kE/nyAurwDzdK2tqw1L/eVudIzwCJX4Y+og2uP8HFDs=; b=RMiBZbWtXwyp2C+LrwLxHdwhEYNN6DOFNTdwk9GKhta1qLalwcnLWCHE/tgqWl7Fu5 xedgN9gkHQZ4GuSBk1aeSawYg0mzggduYHAMBO5GP8BRo+7hxblgQpkzGQUVwT0/aJym ZJ46R1vRHARhn+L4PT9n6Lsss6zwSo0+m0AW+z1J7TGpgPfPFavr08biFAW/8RUa4Fir k6pVFPAOm/GJx0j63HmL7hiGxV3TGS5w0/392Rh9aQZFoX2z+02k5Yl4rzoOudFORKTC stdKG1gWVeBozFx5IYRXdel8MkkuX1Gl04zNvaVE8casLSFHSX9gpbO1q0eeoY0OKnkp 1jJQ== X-Gm-Message-State: AOJu0YzsNsBre7DnX9d96UAmqb17wpFEMPfFa3osPD0qH57uUtocvHOh 4TYaWrJZkoXW/Ehmto7y66hpVyVUI0uN+6WykEd9TqWSLS5KdzBCfNFPHw== X-Google-Smtp-Source: AGHT+IFOBTGDr3z17QB1h/c0MEFVwbJHry4jAVb8rptfffIwJ39fp9N6et3l2kKPWmNuGVc3UUb4nugmPHHTo7XY5vw= X-Received: by 2002:a0d:cf01:0:b0:59b:54b5:7d66 with SMTP id r1-20020a0dcf01000000b0059b54b57d66mr164154ywd.34.1696352752406; Tue, 03 Oct 2023 10:05:52 -0700 (PDT) MIME-Version: 1.0 References: <20231002142949.235104-1-david@redhat.com> <20231002142949.235104-4-david@redhat.com> In-Reply-To: <20231002142949.235104-4-david@redhat.com> From: Suren Baghdasaryan Date: Tue, 3 Oct 2023 10:05:39 -0700 Message-ID: Subject: Re: [PATCH v1 3/3] memory: move exclusivity detection in do_wp_page() into wp_can_reuse_anon_folio() To: David Hildenbrand Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, Andrew Morton , Mike Kravetz , Muchun Song Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 88E6680006 X-Rspam-User: X-Stat-Signature: mgk4uzqbppatthw3bpkit1hhcizwou36 X-Rspamd-Server: rspam01 X-HE-Tag: 1696352753-936878 X-HE-Meta: U2FsdGVkX18xzYdFWS4R54GuL49HzIArDXoI9qpusx7pgc297YlkpAMUQKrs0V2fNuH1ANWp+/daZJs40oTyYWlqVGAkfJCv09/CDDsfE+OK5ZkgEyE8Z4I8qIAj8uIjUbCBIGMDyutVQk/YjD2dgojysaEaodmqqES/kW+GpPGnfXuGJULZ+eKUbrook7d6RfmXp2yTd0Qw3/x6QhD6FBaTWVm7DALvr3RZOxdfRkOMvDQgTbG8VouJlunyaTAVNhzsKj7/5YXmkMmPbYXPSWD/Tkghqj3ruRwokx8q3LcxO7B3RBvMxgL+KXVAShHzR/V6UaIY9EcWKr46eBq0S87laYW3k+mAYGJAcVAjcnSChWUqAWv9cWJNbbRf1n8BkcWZVyJ5J2mO6JDNmb3NvPO8igGLrkACfG+JJkQJ43R1jQPtvgb9vHcKiez7zDJDUNG8/iP8btIq+dWWRY7rco5UCn2RRLdTo89jaoV3zVURjO87VeZkmWyr+FwttZqBLUU68FsWYg8m8VlEXGJ7ja7VJJ3di5FApolQ830+Uwf8819mAn6IhUJZB5yj8DZyWW0A1S339UJS3dUVqY3OVZm6aXbY9XIPHaHarwZ17X7G5w46ppBw15C2K5jOJAahfWgKp0kDk7HuK+u90hSAJaOBmVteT1uo6mvyALC9tk69Qb/TuRY0XypMaPDQAk5tjAnVgADKLjcRU5iJ4lrank0qkFOEqShGBHF5POYSbW0rcxkVWdwFf7Yg+t49m5FuuD7iX/24085L5WKEuIm/8ULMoAkQJgT3m0EM2B10eEr+Alr2CXZ8UNWdn91RiFoBeMIahqqlt4t/rQE7gw+peNOXk2BmNgsR4OYEayowKKwlv/sEENEPuAiMWrDsGkBYI7cs2CTl5PUT5OnldljUK1C9SMF+6KBlVvVDxBbFZ/BNFrHI70x83lKwDqcljvPdprcp0e3rGjVKvo+45wA 37IsC4q5 CKYehthgIlTtZsKepk700HNSqpuE+fB4O3E6kH0pMcrOdfL9VbMpDyDq/Zeg5/G2bCZOCuAWljtzEMD1+3f+PlaidWv+Kl931+fpwuxh+tnMkKzGAM3u6jh1jP/P42/QG/ZPtDtpuola3G/YPSUFo+wT8PHp2PFaiuTmjHKZ6HXf+uTTvamd/+Mm/z5xx1CrnKb2MnXa0aEvN2g668AbUKhAef+0g7fFIMSAqoutCI1rYbwqmyXNEvNyJqAWpr0hm1Oh4XUvhnk6QZchEKRtgmpHFfJ+whc/a4oHrILl0bKhiqc4= 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: On Mon, Oct 2, 2023 at 7:29=E2=80=AFAM David Hildenbrand = wrote: > > Let's clean up do_wp_page() a bit, removing two labels and making it > a easier to read. > > wp_can_reuse_anon_folio() now only operates on the whole folio. Move the > SetPageAnonExclusive() out into do_wp_page(). No need to do this under > page lock -- the page table lock is sufficient. > > Signed-off-by: David Hildenbrand > --- > mm/memory.c | 88 +++++++++++++++++++++++++++-------------------------- > 1 file changed, 45 insertions(+), 43 deletions(-) > > diff --git a/mm/memory.c b/mm/memory.c > index 1f0e3317cbdd..512f6f05620e 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -3358,6 +3358,44 @@ static vm_fault_t wp_page_shared(struct vm_fault *= vmf, struct folio *folio) > return ret; > } > > +static bool wp_can_reuse_anon_folio(struct folio *folio, > + struct vm_area_struct *vma) Since this function is calling folio_move_anon_rmap(), I would suggest changing its name to wp_reuse_anon_folio(). This would clarify that it's actually doing that operation instead of just checking if it's possible. That would also let us keep unconditional SetPageAnonExclusive() in it and do that under folio lock like it used to do (keeping rules simple). Other than that, it looks good to me. > +{ > + /* > + * We have to verify under folio lock: these early checks are > + * just an optimization to avoid locking the folio and freeing > + * the swapcache if there is little hope that we can reuse. > + * > + * KSM doesn't necessarily raise the folio refcount. > + */ > + if (folio_test_ksm(folio) || folio_ref_count(folio) > 3) > + return false; > + if (!folio_test_lru(folio)) > + /* > + * We cannot easily detect+handle references from > + * remote LRU caches or references to LRU folios. > + */ > + lru_add_drain(); > + if (folio_ref_count(folio) > 1 + folio_test_swapcache(folio)) > + return false; > + if (!folio_trylock(folio)) > + return false; > + if (folio_test_swapcache(folio)) > + folio_free_swap(folio); > + if (folio_test_ksm(folio) || folio_ref_count(folio) !=3D 1) { > + folio_unlock(folio); > + return false; > + } > + /* > + * Ok, we've got the only folio reference from our mapping > + * and the folio is locked, it's dark out, and we're wearing > + * sunglasses. Hit it. > + */ > + folio_move_anon_rmap(folio, vma); > + folio_unlock(folio); > + return true; > +} > + > /* > * This routine handles present pages, when > * * users try to write to a shared page (FAULT_FLAG_WRITE) > @@ -3444,49 +3482,14 @@ static vm_fault_t do_wp_page(struct vm_fault *vmf= ) > /* > * Private mapping: create an exclusive anonymous page copy if re= use > * is impossible. We might miss VM_WRITE for FOLL_FORCE handling. > + * > + * If we encounter a page that is marked exclusive, we must reuse > + * the page without further checks. > */ > - if (folio && folio_test_anon(folio)) { > - /* > - * If the page is exclusive to this process we must reuse= the > - * page without further checks. > - */ > - if (PageAnonExclusive(vmf->page)) > - goto reuse; > - > - /* > - * We have to verify under folio lock: these early checks= are > - * just an optimization to avoid locking the folio and fr= eeing > - * the swapcache if there is little hope that we can reus= e. > - * > - * KSM doesn't necessarily raise the folio refcount. > - */ > - if (folio_test_ksm(folio) || folio_ref_count(folio) > 3) > - goto copy; > - if (!folio_test_lru(folio)) > - /* > - * We cannot easily detect+handle references from > - * remote LRU caches or references to LRU folios. > - */ > - lru_add_drain(); > - if (folio_ref_count(folio) > 1 + folio_test_swapcache(fol= io)) > - goto copy; > - if (!folio_trylock(folio)) > - goto copy; > - if (folio_test_swapcache(folio)) > - folio_free_swap(folio); > - if (folio_test_ksm(folio) || folio_ref_count(folio) !=3D = 1) { > - folio_unlock(folio); > - goto copy; > - } > - /* > - * Ok, we've got the only folio reference from our mappin= g > - * and the folio is locked, it's dark out, and we're wear= ing > - * sunglasses. Hit it. > - */ > - folio_move_anon_rmap(folio, vma); > - SetPageAnonExclusive(vmf->page); > - folio_unlock(folio); > -reuse: > + if (folio && folio_test_anon(folio) && > + (PageAnonExclusive(vmf->page) || wp_can_reuse_anon_folio(foli= o, vma))) { > + if (!PageAnonExclusive(vmf->page)) > + SetPageAnonExclusive(vmf->page); > if (unlikely(unshare)) { > pte_unmap_unlock(vmf->pte, vmf->ptl); > return 0; > @@ -3494,7 +3497,6 @@ static vm_fault_t do_wp_page(struct vm_fault *vmf) > wp_page_reuse(vmf); > return 0; > } > -copy: > /* > * Ok, we need to copy. Oh, well.. > */ > -- > 2.41.0 >