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 33CBDC2BD09 for ; Tue, 9 Jul 2024 13:03:04 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 32B376B0095; Tue, 9 Jul 2024 09:03:03 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2DB316B0096; Tue, 9 Jul 2024 09:03:03 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1A4266B0099; Tue, 9 Jul 2024 09:03:03 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id F0EF36B0095 for ; Tue, 9 Jul 2024 09:03:02 -0400 (EDT) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 96A4712191F for ; Tue, 9 Jul 2024 13:03:02 +0000 (UTC) X-FDA: 82320229404.23.089C062 Received: from mail-ua1-f50.google.com (mail-ua1-f50.google.com [209.85.222.50]) by imf02.hostedemail.com (Postfix) with ESMTP id 6118980015 for ; Tue, 9 Jul 2024 13:02:59 +0000 (UTC) Authentication-Results: imf02.hostedemail.com; dkim=none; spf=pass (imf02.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.222.50 as permitted sender) smtp.mailfrom=21cnbao@gmail.com; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=kernel.org (policy=none) ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1720530164; 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; bh=IMi7us1d4l4SwS+BzMlANRr5BsQBfw+Uxq4fBAlUSPM=; b=QQkA2tu7uBn8bP+MNA3tA0+0tfiwiwgNEfHHWhUMv2tB9ny5ZpJu7wAYYijeViIxZ0qa7/ 8moPCojXi9T+j0Xyf3zhaVJxvl5dI907v4p+Pzs1KYzJJ7B6CNWYL/CuqW9a4iNUuAWzm/ ELXFzJBnM/egghbf13DdoevP1DGSCjY= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=none; spf=pass (imf02.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.222.50 as permitted sender) smtp.mailfrom=21cnbao@gmail.com; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=kernel.org (policy=none) ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1720530164; a=rsa-sha256; cv=none; b=ExLc5LAZRJ58mHEmO8Wit+QJmTe7vnBFSIJsFvG/FWndl17cHhlpM7rLG7EdcotizZmwQN TmDl5Hlh+b1dHttBSsv/PI3KeZwCWCyZ+q0nIzVg4uZbirHc0uzM6KKmrHwl2Eo+yOoHZo zdJYMheDCg3F/A36XSE0MWFWw6TR6EE= Received: by mail-ua1-f50.google.com with SMTP id a1e0cc1a2514c-81052163538so914760241.2 for ; Tue, 09 Jul 2024 06:02:59 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1720530178; x=1721134978; 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=IMi7us1d4l4SwS+BzMlANRr5BsQBfw+Uxq4fBAlUSPM=; b=sDLp3dNFfe1wUqtW49Q1nnl+Ev2VK4AmCEY1XX7I1qB21QSmGWI6+5jlDIMkVbpvMv OTHwyv4ILLcqfcfTKmLzU5hqdqZhNc5yy1MPogBZa55fSuda4AUgs8a3TaMLvHGVURgd mzuE9vUa5+htQLOE6ND022JVweDS4+Wi+ZG3Ull7V4qn4NPBMfwta7eh3WeD7cNn3Bh0 eqQGIf6K+idfUixx5wDPSRuDhdJTX/0HJAUX4xg9eMkgC8+47wmxRX+Z94eTc7sFyLMK RaaNxrazKMa3eEY4gxSmTxAeEnT6hmHxQMi9l48oeg9S0u4qlTMOuZbAKocMPbFfKAlB FJ2Q== X-Forwarded-Encrypted: i=1; AJvYcCVCM24W9S0DoVqebtpEM4zd/80oJV83Y+MxFLCrZtedERpaDycCptulsFTEaiIiA5VLUYBkFGgtXtH4ZeeJzUmjVfY= X-Gm-Message-State: AOJu0YzKoeXkGguCZ6rbzZA0qvDJpDucLjDGjZDF2GDnCXcJMaZho4BZ mRyA55cFcsFtrIBFQDPjOpCOz2vFuWfzCTNTSZgSY3/wiiB15Wg9S0wGyQoe2x2gem+fqJZRs8j WKWcdXE27c4iJMNvAW0Jnllk7pko= X-Google-Smtp-Source: AGHT+IH1DUYHkvESitkatNt9z8R+LNiPwJYkZABrwfIcxNbp5gyHkiGQMCZMExTRkccnJg7pFIdkBN6IrWrox6EkwPs= X-Received: by 2002:a05:6102:fa9:b0:48f:1763:c389 with SMTP id ada2fe7eead31-4903222af4bmr2398604137.35.1720530178226; Tue, 09 Jul 2024 06:02:58 -0700 (PDT) MIME-Version: 1.0 References: <20240709123115.117-1-justinjiang@vivo.com> In-Reply-To: <20240709123115.117-1-justinjiang@vivo.com> From: Barry Song Date: Tue, 9 Jul 2024 21:02:45 +0800 Message-ID: Subject: Re: [PATCH v7] mm: shrink skip folio mapped by an exiting process To: Zhiguo Jiang Cc: Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, David Hildenbrand , Matthew Wilcox , opensource.kernel@vivo.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam03 X-Rspam-User: X-Rspamd-Queue-Id: 6118980015 X-Stat-Signature: ehzpgcqdz4usp5j7ptq7gf4qbzb4gz15 X-HE-Tag: 1720530179-50325 X-HE-Meta: U2FsdGVkX1+BWG75ifU0uzvq2LUAB2zjWpQmdhyWZ2WMUDTrV796BDz2qb8XTs6A49nDI4h1jQWM1OvGhnZudWFy1o9JGwsl/YkdjlwAL+WHByGwN8co38jsZRrWNgoXHekWzSj5itlFwk5gaaHSZfAsyc1C2+ii8CNpFzuG/qCRNpnriyBZqbj0hNgxwNFgcHF7vGCQoh4w/yueHjsktHkF1avqj5A87VOCwcVg1GgH52Y2HkMg9Wj0noO7hbqwrKNLY151PFFOwrmbGB3sWD9J4jm0gfgf9IxiMeoAML2dSoroi6lvwnPAxEJTSBsLgmGDJLKKUGDv0+/cweEoCG5qrVHgDTMNxN/tjb6yMbrX2iFSwrm9wYweXU++kBrwX2ikOHMqsp1O/m+DAXslBSoEmu2BsOQgiST4rmVknXVY3Ri+ofAg+sQfY6broyOhtsHOmfEgv8IF1AvmS/QXyGPTw4jMrV3LhiON9b5Q6JoM0o0gKrgCagpdm4pJtITH2e12fD5SwfcJVPIGjtjrpBxqVemdZFOy3zxGIH6J50lq5MQxj37ZUsiNpb5Pf7jWZUyFzs9JjnyYz6Ido+Q2FfvxtZSj3l+kVzpAKwyeV9TExikmy5aYYZh4lP5LOrnnZMWXGGvuaRTui6KaJQ/ZQaSoRvVF9uPDPvOIRVvKVVmePObCkmrcLW0pGEj0n2w7dUc2+PLYgdyTkeelh5pJjYrdyUTmXmfFaCcJ+TwLympugZmCjTg5b4BCkr1a663P3g+I8y3iwEfww5KhaJnXZqhD/dWI52dQJu1fjWQ/eTokTNtT5FkB7wdx+N2RaI7Z0eelNrScHcW1E35aL5U9IFYZb2BE+LuDiWRsB3vE/TyvI25tC06h9VYE6hXgq16HVEuc9zDLoY/pP5X0sDS2nQel65AXKxBtfGVc0HnAKNkwt3P89kgcREtICpAbV46qcaww0trPlNEy+XynSVA vkpryQ/D klC4u9P5zhcsZJJfHuud1Q0whKMTsUhis4kHPZetTP6Naxmbkk4AZitxG4S/a67wwgcOxI4JnsioupdmDlHy6YSazJZzZZOUN3njSUIWJHM2vTvCNc45p8VMerl93Pkx8wmtPUwGai64+qGN7yagFkiNGLiFJADsBuLoAMVx8F6c8/3BF0XAsx/P18SfF7Wk4AWWPvquffa8l7nqW4deZWBcRtcDNCFbxd0ihHDUjgZifkKnUHwT4bXMgEzBdCDETN0g78hbx4XvAxNpfTZ7ZZkM1SCUkvxl7N4W+lH0hcewelgrVj8R333aWXfEvoWcPR3DqNLyHcfs+A4Rft1kpVIb6OELVvXHJgervNgXR4C3jfDzNF9YimBxYJWntPIk8aRX2t55ChrGmTtTxvH3H2X3K7j10h9cxJX8aGAXcvYvw0m14LH2Z3flO3R31aglMA8iOc/1V+XEGgDPvx5ZbMMn0E5H9RosnB6n1U5C707ais6eOeHboUeV23UgxwOpV5hMb 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 Tue, Jul 9, 2024 at 8:31=E2=80=AFPM Zhiguo Jiang = wrote: > > The releasing process of the non-shared anonymous folio mapped solely by > an exiting process may go through two flows: 1) the anonymous folio is > firstly is swaped-out into swapspace and transformed into a swp_entry > in shrink_folio_list; 2) then the swp_entry is released in the process > exiting flow. This will result in the high cpu load of releasing a > non-shared anonymous folio mapped solely by an exiting process. > > When the low system memory and the exiting process exist at the same > time, it will be likely to happen, because the non-shared anonymous > folio mapped solely by an exiting process may be reclaimed by > shrink_folio_list. > > This patch is that shrink skips the non-shared anonymous folio solely > mapped by an exting process and this folio is only released directly in > the process exiting flow, which will save swap-out time and alleviate > the load of the process exiting. > > Signed-off-by: Zhiguo Jiang You should have collected tags such as reviewed-by, acked-by you got in v6 while sending v7. Again, Acked-by: Barry Song > --- > > Change log: > v6->v7: > 1.Modify tab indentation to space indentation of the continuation > lines of the condition. > v5->v6: > 1.Move folio_likely_mapped_shared() under the PTL. > 2.Add check_stable_address_space() to replace MMF_OOM_SKIP. > 3.Remove folio_test_anon(folio). > v4->v5: > 1.Further modify to skip non-shared anonymous folio only. > 2.Update comments for pra->referenced =3D -1. > v3->v4: > 1.Modify to skip only the non-shared anonymous folio mapped solely > by an exiting process. > v2->v3: > Nothing. > v1->v2: > 1.The VM_EXITING added in v1 patch is removed, because it will fail > to compile in 32-bit system. > > > Comments from participants and my responses: > [v6->v7]: > 1.Matthew Wilcox > You told me you'd fix the indentation. You cannot indent both the > continuation lines of the condition and the body of the if by one tab > each! > --> > Modify tab indentation to space indentation of the continuation > lines of the condition. > > [v5->v6]: > 1.David Hildenbrand > I'm currently working on moving all folio_likely_mapped_shared() under > the PTL, where we are then sure that the folio is actually mapped by > this process (e.g., no concurrent unmapping poisslbe). Can we do the > same here directly? > --> > You are right. we might use page_vma_mapped_walk_done() to bail out. > (Barry Song) > > 2.Barry Song > By the way, I am not convinced that using test_bit(MMF_OOM_SKIP, > &vma->vm_mm->flags) is correct (I think it is wrong). And exit_mmap() > automatically has MMF_OOM_SKIP. What is the purpose of this check? > Is there a better way to determine if a process is an OOM target? > What about check_stable_address_space() ? > --> > Sorry, I overlook the situation with if (is_global_init(p)), > MMF_OOM_SKIP is indeed not suitable. It seems feasible for > check_stable_address_space() replacing MMF_OOM_SKIP. > check_stable_address_space() can indicate oom kill, and > !atomic_read(&vma->vm_mm->mm_users) can indicate the normal > process exiting. > > I also think we actually can remove "folio_test_anon(folio)". > --> > Yes, update in patch v6. > > [v4->v5]: > 1.Barry Song > I don't think this is correct. folio_likely_mapped_shared() is almost > "correct" but not always. > Please explain why you set pra->referenced =3D -1. Please address all > comments before you send a new version. > --> > Update in patch v5. > > 2.Matthew Wilcox > How is the file folio similar? File folios are never written to swap, > and they'll be written back from the page cache whenever the filesystem > decides it's a good time to do so. > --> > What do you mean is that the file folio will not have any relevant > identifier left in memory after it is reclamed in the shrink flow, > and it will not be released again during an exiting process? If that's > the case, I think we only need the anon folio is skipped here. > > [v3->v4]: > 1.Barry Song > This is clearly version 3, as you previously sent version 2, correct? > --> > Yes. > > Could you please describe the specific impact on users, including user > experience and power consumption? How serious is this problem? > --> > At present, I do not have a suitable method to accurately measure the > optimization benefit datas of this modifications, but I believe it > theoretically has some benefits. > Launching large memory app (for example, starting the camera) in multiple > backend scenes may result in the high cpu load of the exiting processes. > > Applications? > --> > Yes, when system is low memory, it more likely to occur. > > I'm not completely convinced this patch is correct, but it appears to be > heading in the right direction. Therefore, I expect to see new versions > rather than it being dead. > You changed the file mode to 755, which is incorrect. > --> > Solved. > > Why use -1? Is this meant to simulate lock contention to keep the folio > without activating it? Please do have some comments to explain why. > I'm not convinced this change is appropriate for shared folios. It seems > more suitable for exclusive folios used solely by the exiting process. > --> > The skiped folios are FOLIOREF_KEEP and added into inactive lru, beacase > the folios will be freed soon in the exiting process flow. > Yes, the shared folios can not be simply skipped. I have made relevant > modifications in patch v4 and please help to further review. > https://lore.kernel.org/linux-mm/20240708031517.856-1-justinjiang@vivo.co= m/ > > 2.David Hildenbrand > but what if it is shared among multiple processes and only one of them > is exiting? > --> > Modify to skip only the non-shared anonymous folio mapped solely > by an exiting process in next version v4. > > [v2->v3:] > Nothing. > > [v1->v2]: > 1.Matthew Wilcox > What testing have you done of this patch? How often does it happen? > Are there particular workloads that benefit from this? (I'm not sure > what "mutil backed-applications" are?) > And I do mean specifically of this patch, because to my eyes it > shouldn't even compile. Except on 32-bit where it'll say "warning: > integer constant out of range". > --> > Yes, I have tested. When the low system memory and the exiting process > exist at the same time, it will happen. This modification can alleviate > the load of the exiting process. > "mutil backed-applications" means that there are a large number of > the backend applications in the system. > The VM_EXITING added in v1 patch is removed, because it will fail > to compile in 32-bit system. > > mm/rmap.c | 14 ++++++++++++++ > mm/vmscan.c | 7 ++++++- > 2 files changed, 20 insertions(+), 1 deletion(-) > > diff --git a/mm/rmap.c b/mm/rmap.c > index 88156deb46a6..bb9954773cce 100644 > --- a/mm/rmap.c > +++ b/mm/rmap.c > @@ -877,6 +877,20 @@ static bool folio_referenced_one(struct folio *folio= , > continue; > } > > + /* > + * Skip the non-shared swapbacked folio mapped solely by > + * the exiting or OOM-reaped process. This avoids redunda= nt > + * swap-out followed by an immediate unmap. > + */ > + if ((!atomic_read(&vma->vm_mm->mm_users) || > + check_stable_address_space(vma->vm_mm)) && > + folio_test_swapbacked(folio) && > + !folio_likely_mapped_shared(folio)) { > + pra->referenced =3D -1; > + page_vma_mapped_walk_done(&pvmw); > + return false; > + } > + > if (pvmw.pte) { > if (lru_gen_enabled() && > pte_young(ptep_get(pvmw.pte))) { > diff --git a/mm/vmscan.c b/mm/vmscan.c > index 80f9a486cf27..1d5f78a3dbeb 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -863,7 +863,12 @@ static enum folio_references folio_check_references(= struct folio *folio, > if (vm_flags & VM_LOCKED) > return FOLIOREF_ACTIVATE; > > - /* rmap lock contention: rotate */ > + /* > + * There are two cases to consider. > + * 1) Rmap lock contention: rotate. > + * 2) Skip the non-shared swapbacked folio mapped solely by > + * the exiting or OOM-reaped process. > + */ > if (referenced_ptes =3D=3D -1) > return FOLIOREF_KEEP; > > -- > 2.39.0 >