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 EE0D9C2BD09 for ; Wed, 10 Jul 2024 02:01:00 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8347B6B00A5; Tue, 9 Jul 2024 22:01:00 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 80B666B00A6; Tue, 9 Jul 2024 22:01:00 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6D2D16B00A7; Tue, 9 Jul 2024 22:01:00 -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 50DD36B00A5 for ; Tue, 9 Jul 2024 22:01:00 -0400 (EDT) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id EF27F161A5F for ; Wed, 10 Jul 2024 02:00:59 +0000 (UTC) X-FDA: 82322189838.27.F7B7B60 Received: from mail-ua1-f41.google.com (mail-ua1-f41.google.com [209.85.222.41]) by imf11.hostedemail.com (Postfix) with ESMTP id 396AA40017 for ; Wed, 10 Jul 2024 02:00:58 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=none; spf=pass (imf11.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.222.41 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=1720576842; 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=hwplGrTzrWU0Nuw4LQrAWRSRooLeWGZ6YTTKuTBHp38=; b=0RvCyvCZS+A5T6w1QpRiO4EgrrvY+NAFEPioO3AtxF/euhwc3SMCtdfFSc6x3BePSHglqG PdGbYrEZphkHxw2mVp3c1Nx7p1TjW4iZ7dIouK0h6Of9NDJj/7y5yTHjHU6zEeyA6uC5/N Nd3qUs3SallvKrE8qTBB/cVJ26/hLMw= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=none; spf=pass (imf11.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.222.41 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=1720576842; a=rsa-sha256; cv=none; b=wOB+wp+QrqPkiLy7yF56c+q133J3LEGSHozT0zkcDukWjt0CM88noGFIjKG/c3UnRcpGLf EuiWcBp1QVLcUGqeBx3QNXqKpP/oaOfL6JQGotDNIyersfmDgVsF6y73mdrB+0/1A2vIXA l+9em9XhdAmBJr7JUJeYK/HIszeeCTA= Received: by mail-ua1-f41.google.com with SMTP id a1e0cc1a2514c-8102bbc95d1so1940391241.2 for ; Tue, 09 Jul 2024 19:00:58 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1720576857; x=1721181657; 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=hwplGrTzrWU0Nuw4LQrAWRSRooLeWGZ6YTTKuTBHp38=; b=deasZbYXmqGPGM1iCPI1BLxZehSoKiAqxIlbvBnL9/yrrkHc3snsCp0dEln7kN6Q8q CdFjgvXytqF1Ox4bsX4PVSndw+9Hr6Lo6fx1Dh8AGXSrkEHQZn9ihpvnRBTuPggM4Wi5 4E7Yo9LBog01RlzfSZiolngyXjUoN5QoxXUme4Z9TlbX7I51WUWWvd6dpHNLYx7sfvcB nK26V6idVxWuim8M4d7VGvVSZIxns1lEhG/pMUBOdvlh7llKXY0y8IaxeSA1sYs9vss4 S27Xifm27w6xJ6Vlg/wU5JBeItorXoJ4JZeZkO1aFCV8d1oRR9CU8ABGRop76WbYsRxh uF/A== X-Forwarded-Encrypted: i=1; AJvYcCU0S500q70wxNRF+kNuc+RqYgDmemWOt8gs6LfHo7xonc2D3TZbh6s6ySVp9nz8KrDHJOw0vomDNZlJBzHmHdxnCkE= X-Gm-Message-State: AOJu0YyqfJxqbJEOXDsfw4AmMs15nGxWdKCXSTzYElTwkUmATLiLftdE MG9mOc+n6Mc4yNM/uHZLN8D4S1kCCrKWvqOQBs6Vp1O9b4e/99KTzYIgq/DLT5Mtv0kV0fEhmyU kU5N5MnUgBHMk7gHUzUk55doEJBM= X-Google-Smtp-Source: AGHT+IGXdtBbOUBpbSCBUSMjGTEyH1Zi7P4SMCiESwkf4nzGvqgd0C1vf6X+rJCb7UrH8/PGrOZsnaV0NuXrKjqu9Y8= X-Received: by 2002:a05:6102:c91:b0:48f:a83f:7d10 with SMTP id ada2fe7eead31-4903212d717mr5480306137.10.1720576857191; Tue, 09 Jul 2024 19:00:57 -0700 (PDT) MIME-Version: 1.0 References: <20240709123115.117-1-justinjiang@vivo.com> <863b2400-5def-4bd2-8195-d71ce91f1c99@vivo.com> In-Reply-To: <863b2400-5def-4bd2-8195-d71ce91f1c99@vivo.com> From: Barry Song Date: Wed, 10 Jul 2024 14:00:45 +1200 Message-ID: Subject: Re: [PATCH v7] mm: shrink skip folio mapped by an exiting process To: zhiguojiang 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: rspam11 X-Rspamd-Queue-Id: 396AA40017 X-Stat-Signature: 7dm85dpftt4zjn6uwhkttdsg544smjg8 X-Rspam-User: X-HE-Tag: 1720576858-572832 X-HE-Meta: U2FsdGVkX1+GxaVtTAr+Cx9K9sSBB1mukRngIqN1X5o9+zX19kJobH0IYiFvmvDVmLfDPvC2FKj+bcjhm0x4sdVNXPua7bWZBHEwUFmi6slEz6oQUb6dbE3wA858WwOQNzWQwwVxEcA1lbeNSjF+IekxnfjZSwejTQXd43xftBKNVAtlPgw1yo7J5suadpeHsOlVwS19FfNE7JzyjmX9v2ugYA3znY3nfaWJ+6TFWVN3wb1Iv2VK4YhPKk3l3jFTM4aL1gNK3BAF+oFbkTAbMYX0C/LoE32yQLB408xVL5505FxnTg0Cs1tgrD3CAteOmfmVJrsQCMXB4eFwsg4b63JB9YQyBrZac+SOrYaObc3vH85v+Ke8rLrD48D6C0Cn+7+wIdvvXE4Wc1DrY2QySnGcCVkjcgqu5Ml5lvj3cWGFU7ZU9bwBeurzYeYhP2CW/cY5/UjXUuIVoseoGOK1JbSUV6s6LUMy8JJjQBncbDVHlGTFSjE40hI4CInJehmW0/OgRL1nH44CwEXhjwe2/01/Ce7JqKtJqTxzo00q4Vzbr1p9wXCtI12htvAd8Aq66KFWn81GNObReuChOW3hMKcgyg5d8oFDFXeZEC/JrngmTUTU1i3natTgaI+4pHQUSIq7+SA1S4rv8qf4GzFykTDsYBj4LcKsKn/gxbJ2D5eBN4addToV+M9ThM75f0yP0wBlV62g31qqP9mIb2Xe/XSNeD5hOlgOLEZ6rc9kclGadQupto5CpTle5FgG6rJjkEgg+FR1FHogeIF6og2+yZmSqSrq+QTI1SD7Qn+NJsjKMchKfIkMOxgXejrsuia33JZzIL8nGlur0ZUOaQobtWEWcd9K0aYBEMvecT5f2vOA+T1mCuiw4pOJQj7ZgPaA7s/5iNTQPw2mEhhdPE8zIO4D19mEqiK8YvS124vLr1Zb1k0bq3FXa6QNfVY1HhwlUBVpyVuREP2evXHEGPk uE++gNaw gPTflFYINiDacMQNNgzHp0iKrHvF7tZOLENGrPOYoEXeMGSpuSbhdiRjVR44+mSx4/qWqc8fhyjreILvn4dAy8FYWA9yJFOBDOyLb7Yh+i/HOLmQiiNjW0G4F49tINkY69ePHkpuzMPwXhKDNOE0SvBfsHEWsY+EcleUHM6t4xk/F+rILkw1p2hdRHfk4uqg0Qpc4A9kmuXbccAhO/zym5xen2DelIFXrg8aUWljFmCbvMHtzQJZd2BIl70QXbSZ7vWa/fPtslGcZ2QzXCIQ5u0LeS1v8TNkbKWwd049bmuFLhOkeMUo3nWeq/dx0om9doefSjAeRJd5pVYDMH0Ec/x/gZ9Hv0LxDXcpINMxUXpmhcZR6b6+LzhjlqsQVK3R410EF11mZ4/2R0ZS+/aHR1Of5Z506NogVK8bZ2P1GrR4x/RPhPjBJQu8pttbrgBxlRxQgqtMAVFDBVOrRj0xSWprVVihYYngwgtTo39YyPqeaSZrdAYG35GSKIl1i6ItNaKgbklq+hHaXmHzHtjR/mP1X/Q== 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 1:46=E2=80=AFPM zhiguojiang = wrote: > > > > =E5=9C=A8 2024/7/9 21:02, Barry Song =E5=86=99=E9=81=93: > > 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 i= n > >> 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 > Yes, it is alreadly included in patch v7. obviously no. please take a look how people collect tags while sending a new version: https://lore.kernel.org/linux-mm/20240704012905.42971-2-ioworker0@gmail.com= / > > Thanks > Zhiguo > > > >> --- > >> > >> 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 al= l > >> 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 filesyste= m > >> 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 multi= ple > >> backend scenes may result in the high cpu load of the exiting processe= s. > >> > >> 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 version= s > >> 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 foli= o > >> without activating it? Please do have some comments to explain why. > >> I'm not convinced this change is appropriate for shared folios. It see= ms > >> more suitable for exclusive folios used solely by the exiting process. > >> --> > >> The skiped folios are FOLIOREF_KEEP and added into inactive lru, beaca= se > >> 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= .com/ > >> > >> 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 alleviat= e > >> 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 *fo= lio, > >> continue; > >> } > >> > >> + /* > >> + * Skip the non-shared swapbacked folio mapped solely = by > >> + * the exiting or OOM-reaped process. This avoids redu= ndant > >> + * 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_referenc= es(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 > >> >