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 E42CDC2BD09 for ; Wed, 10 Jul 2024 02:13:10 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 77BB56B007B; Tue, 9 Jul 2024 22:13:10 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 72BA76B0085; Tue, 9 Jul 2024 22:13:10 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5F2D86B0088; Tue, 9 Jul 2024 22:13:10 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 426866B007B for ; Tue, 9 Jul 2024 22:13:10 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id E7CA8C1A9C for ; Wed, 10 Jul 2024 02:13:09 +0000 (UTC) X-FDA: 82322220498.07.45109CF Received: from mail-ua1-f50.google.com (mail-ua1-f50.google.com [209.85.222.50]) by imf23.hostedemail.com (Postfix) with ESMTP id 30961140012 for ; Wed, 10 Jul 2024 02:13:08 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=kernel.org (policy=none); spf=pass (imf23.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.222.50 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1720577564; a=rsa-sha256; cv=none; b=8ZAZ9oQAqxc0Vp+NuTMgNKUi5Wb4fpsjOE1wmX+R7VCQxyhdZzU7N/c2hyOsV0zjg9Gtex +EyThLyh7XjhOemH5wR2opZ6so2+yJHN/Uib2f38m1jWvrmyWFTI2GoZY989OjOfOIXLcT r/RlkGNuKqDacNiM1HzL5lKriOV4dQg= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=kernel.org (policy=none); spf=pass (imf23.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.222.50 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1720577564; 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=ap0RCxSh+qYkkw7U7sBwAd2BA+YT7I3M9TuJr/Weh2w=; b=TXE8SR8kFAsyzWWd3sgMA1cYgAb3wJbZW2yxUKrLqwmreisilrzZl/z3hGQCn/Uz2SMLRP lESKWuDXj6ILotBr04bwZhiP80RtjD0Y1KWLY065NLThS4EdOvNVyBVbFzCMW5OJh2NRN0 77MEF5kCpe86Z4w1MOneUsSA0yW15x8= Received: by mail-ua1-f50.google.com with SMTP id a1e0cc1a2514c-80fa1509989so201428241.0 for ; Tue, 09 Jul 2024 19:13:07 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1720577587; x=1721182387; 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=ap0RCxSh+qYkkw7U7sBwAd2BA+YT7I3M9TuJr/Weh2w=; b=qXH0xZ94ph+9qwpovrpM/e4ObxTcBY9b25vzmYYrouMgLlxD4wtVTQBL1FD9b7W7pY M4jGbbCWbSoAY0y8SVHoEK2H48t5WdfNGGVgBG2qQ2VgB5GzH6MBe9/FSDby2y3jUpCR jGTl2RgPgKYbknAxsH1i/s2JTJLwZH6LG9c0Oyw2Ds0jCUNIGxfAet+QrF9vOz5pjXJj y5y27s33n8QEgbQJvAzA0RTCa1ZppURa5kJmLxD0WN4JYAc5u46d5R1owRhSy7nhyOyw PQlkewKBUpY0df4pnSITfCalCNhKqdNDzfYwcvzB5EqZaLTiEtWl1P+qjNdCBWC81//P XQpA== X-Forwarded-Encrypted: i=1; AJvYcCVC60A/xynVCKI5tlCzoUNpX4EZ30/dxXeOUTFKK2PgzRgaONNFcWPbJe7pf31/T0RyNus1u6jqj2JCRo3OZI6eFCE= X-Gm-Message-State: AOJu0Yzq/6L7C1mnu03z6kVjr/LyXPIqN7DKtNbjvp4i9F70dAfHqEWd BQ9UgTQnP3nnm6u9pj6xlkcIGt++Fx4rajVh+UzaSkQtV+hj9emkk341t5o08MHd1hfY1YhC8mO XqFxaW69DKyMeAzNrjG3QNiI4zCA= X-Google-Smtp-Source: AGHT+IHLtnhQt3WqlPwpzlThyK97GqekTQutkX585akKzUYeNrFdik1HjEO/wMxioxITvRTH7JA5DTb1d8iskmjlWXE= X-Received: by 2002:a05:6102:2acf:b0:48f:e9f5:2e37 with SMTP id ada2fe7eead31-490332b056fmr2827139137.6.1720577587111; Tue, 09 Jul 2024 19:13:07 -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: Wed, 10 Jul 2024 14:12:55 +1200 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-Queue-Id: 30961140012 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: 1fb5birpdaezzqzh1hf78pj1mphhyc6b X-HE-Tag: 1720577588-212061 X-HE-Meta: U2FsdGVkX198+nGHZmSYiXDqM5R2Lj0yhfBals7rRjQX6a3jIgoZsAGMrM+rlqNJPxsGua/dR673Lrrpf5IXt2SN4vtBgx8nqeN+WtX9Di35KXpB5FewKZJqZ2KEEhILvbGPIx1yYC0o4HMXME+k6nq7oma47ZEyiBOrBx+8Yp+NU3d6SVtaSP6crd/+7ZHjGtHFku2zdTOAMSUZG6bri9YHFMtfW7asLjOvUj7C3fYAzUi9Nwspx/S4RfJ1lwJKowD8gTawh4c0G8HESfLzQUVIv4GVUDO4t5vC0vmYmBUi8uxfeRpWtABqqDNzzYU8mEkzfeqAcI5LedBZsZ8ZVjLLOkCaXiR8uJxu96w866FKyj4zzqZrss3qB8ImAAJRt1/U+BzoF3v7T34quVHSXChQ+/EZhvSd5E4bd6rAIS//RQezgtvuNwuVPiTEXjH6jykcvvPoZMWJK6i4tLqW131uUwAuXqCD0+9zrBNBv2LI/NJcye3wPtPfnMup1g/sKyUzYWtTpBSeKF4YS7MPB+8dFusqc4/ZU1XiyJYHrwSQbF0/2/tzG8y7Fg0vFUWUGATOcM7+oPh1gezypBYcJf/T7SwNwXsja3crqDOHe7XL7jPerwv01LSTx6p3GX5WwL4VbUEmQvxeDxZ9tdIuch5OY5e7g45pbZRG1voBpAAadwBWMWEMHbWILolTTCtHX9A+QAcuwKa38ZHgUPLcVwjy4IKVOKHjRnZfQla23J1akXl8f1TzSpXFIlZKKkZnviIk8TGLyyVLQbqgQ+x9NXg2L+zXH60D30D7bI6S0To7KFxo2Ys3I7uravWZIfxyS97n9cuBDbswGaFL5Yyz+O7eLE6hrggsZc2hHFxeC1O53L1E87dJ/8C+fFFefpDJ3ysygIjwnehf6hKAOTNvwqkUxffNWXOuJRFohosqKZN3TqeSrgi9NvjqOHldVByQW39pGPAcL25XR/JNVjn n6yRKdIg wWgUEI04f5ldLMacOMsYfYe7UstMWOuUov4UBZlWE/1IjdxG4KuBr5/lLB2da2YA6Wp1C3mQacssAVrOMtKmcYNumdQgvlsLNIHvFDs3fIPRGYN7raBY5YPchtw9uTTdiKtfVT8E7OuEJmfqVw6oETLTMQ/tR2zsI6rqbQsedsaX10xl7+82RIfZmexS1CQ8xZRGN9DAWOsTLeT4mmswCoY7wbhBOyfBk5zPU+mweNQSgysddguaqoH8X0qxbrmYgUydVf0fPiO7bhO8NbBHrl8aKj+zRNNOpQUmvXZ5+bEOWrqkCgQHNWEUXW63HEuG5BtUwvcgooN5gi7H0YyLkoYSurhDPcjKNfCfAYK1xqfT+k3wmrqT2eEDIQp+tSDYT1ZEy8CvJFWQJj1lg7Gan0/MVtodVhlSvenNpnk8XHvJT07+tCQ775rfomD6Dd3hDefmKfKyqbpcPza2BqQvg+yROKeAkLpi0tjtPukFhhwaNAPPvNVaYRDaWF0kLaJL60soe 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, Jul 10, 2024 at 12:31=E2=80=AFAM 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 > --- > > 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)) && You didn't even pass the compilation stage because you're missing 'linux/oo= m.h'. It's quite disappointing because I believe in your idea, but you didn't even build it before sending. @@ -75,6 +75,7 @@ #include #include #include +#include > + 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 >