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 3DEF0C2BD09 for ; Wed, 10 Jul 2024 02:42:08 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CE7B16B008A; Tue, 9 Jul 2024 22:42:07 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C701A6B0092; Tue, 9 Jul 2024 22:42:07 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AEA346B0095; Tue, 9 Jul 2024 22:42:07 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 8C2896B008A for ; Tue, 9 Jul 2024 22:42:07 -0400 (EDT) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 30CAE1A1A10 for ; Wed, 10 Jul 2024 02:42:07 +0000 (UTC) X-FDA: 82322293494.18.352C8F9 Received: from mail-ua1-f48.google.com (mail-ua1-f48.google.com [209.85.222.48]) by imf22.hostedemail.com (Postfix) with ESMTP id 6B74DC0017 for ; Wed, 10 Jul 2024 02:42:05 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=none; spf=pass (imf22.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.222.48 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=1720579301; 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=RLQcepiuA2+08kDxrKNHP4HLyj6kWoZhkBHfDgRJmlk=; b=u7SlqDwT6EngHmwerQn2G++UWgTZY0Gp8Y8aO8SgZ7OTfihZ2dc7VVS4+xsOohMCF+6/DF wulszTP12eXXbGXjlOIq/9fMWmSbEoPYWXkeT2nNmwK6qwV5C+s6Z8K12yTvusQ0XgM+WF y6oof5ZpJS0OaFVeOdcIf4LP9D3Su6w= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=none; spf=pass (imf22.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.222.48 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=1720579301; a=rsa-sha256; cv=none; b=JQV5JbXZcaFWhWj0FwLmDj8ePph6PwP/AqTO7IY9ZgkiQO8aG/hk0c05fPISDLbYCTEoeC dqwE0ejxB+HOWVquoW3cTp/7blWeUlDsFI7oAiymE7Z1G8obs2j616rVF4GIJpLOcdzPmp Akz5JgceiZss2xHdWBdr7fcsKo3vjK4= Received: by mail-ua1-f48.google.com with SMTP id a1e0cc1a2514c-81013580bd5so1520670241.0 for ; Tue, 09 Jul 2024 19:42:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1720579324; x=1721184124; 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=RLQcepiuA2+08kDxrKNHP4HLyj6kWoZhkBHfDgRJmlk=; b=LtqwsyzPRSwvZ20fk99FJfNZLzH8ChFIYnw7UQ9V5+2Z89IlMzWnRc4sTdwgdD9wAL nkU3/hZnPIYubYiTtJu51YLPsLfcmyFfsRiv1f3aIfMwYLShVHAJcckQjuQ2RIbKcxzs aBgt/TmwZSSy/Dot3cvJPEFSg7cY78V+mzImDIhcqn0dQRc0OFuCa+wDECyzRsVSKJVQ h44X+TWaMrDPeI96pjYOnz2WSLcQps3L9d9GR+qvd7p9uK0Fjk5x3+nvjuvc9xk2p4qt gj8lwxjavOR5k4uQz6eLZoP1I+bolK9rs1zRcKckjJMbRrkrFvI8jaEImAJobIA3K1Ei /2NA== X-Forwarded-Encrypted: i=1; AJvYcCXLrN5MYcfpP2BMfrIpJPRMfoyqFEXfLALZnMUZvttKssgjB30ae8jvT5Y1kHmgxdbV9qNmwMvUpB1GnhfE9DXbXe0= X-Gm-Message-State: AOJu0YzWcNFsbUwgd5qr/YDRVsUcuLCX379rkvZnVoQOkP3ZGIAbjcTv zdt/2Z41pUcAStc6RlRTkaz875edrSa44QIQ/XQ/Gxz1HT+04U2eigmLKNoXSCLtfbEuW0rP0x6 x03848J5h0hVkPdQTuOsDq38CDIo= X-Google-Smtp-Source: AGHT+IHe87NT6SDA8L+fAdauoyqinBJbnOsL3YD7Zho+pUVlaGiRZV4Cnph5YnlZfbiFhG5ERnXCFjD2JUxJu6baM58= X-Received: by 2002:a05:6102:508d:b0:48f:96a8:fa33 with SMTP id ada2fe7eead31-4903210cd2fmr4384623137.11.1720579324202; Tue, 09 Jul 2024 19:42:04 -0700 (PDT) MIME-Version: 1.0 References: <20240710023901.1624-1-justinjiang@vivo.com> In-Reply-To: <20240710023901.1624-1-justinjiang@vivo.com> From: Barry Song Date: Wed, 10 Jul 2024 14:41:53 +1200 Message-ID: Subject: Re: [PATCH v8] 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-Stat-Signature: bzzm5bxhytn38m984kxixm3k3tpa9u4j X-Rspam-User: X-Rspamd-Queue-Id: 6B74DC0017 X-Rspamd-Server: rspam02 X-HE-Tag: 1720579325-332444 X-HE-Meta: U2FsdGVkX19pM7O0lEdCfgRTNIDvu0ajmMYMFh017FzQqCFlI9RJrVd+KluF0RktN//RzSjoxBUlGfhRvN0lJ/mT8lg5y0nXhqRY+KVtg0Xk0jQoFJBjpqxfwGqnDwWfy+Ybh50lcCMetft1aXBYHFFQzWWrqnylsUZbTzDAeEuWNHD0Jzo34XVrwRkzjBEfTGQ9YVAvEa0xoMRs0UZsbSUXltazRRZvIO5rmOZ3kxetkt7TQ8NpI7aFePEYHjSd7caK+Lahw4O32GFeKnef3d6c+H/nlwKEW6c/LehFCWOKS7q18XkBjBSJZyA/mmOegs6OH25oUKzQ3EmIIZ1+qpiHp40ARrL1jW3XjT5kwUFAtLXAVae8x1zJvVpFA16gsxX+gYlO5BJr51KWIuVXnAulONiKQ4owkmGabxEdpbtaGnGBhkah0tiTRbxA6pc9ZxhQ5GONGavQCJ2ZplknioGi4n0jVRqT3X86wIglkdogwNIGFqGuPjnvk10SsXTBLmF14WS8B9KfJIrDtFUD93cZNmaT0FK6pVXpc0bN/DtBMuGWfwghOsvikILby87qggm3tuYdZ1G6+RJluGY4oxcashiI9X7B3wph7jxmjkpB6jnfgiaeOEiu7/D7Jd7VImMv5bzCRiP+TAG2Op7ckQyoE78A1Jl2tfh1GOEGPWVBb276Q4qAp8c5egmynAoHFsosxEiMOzH5EUjjsh1V3G6C5jOt0fctcUMhh1YhBuQQhi+75zlP64w7PWdDHMqK/4UG5LphjxIgv7ZpD58kzaNRL9ys1Pcx1POkhP0h3iK3J1Bb+yqCPHBJ+fwvU4oJnoxUvRaxbFtc249TGhwIXYw2mJ37VIQmxxCrGzVMdApCMWMfS+2VjI6YHCttOolNmK26DWGrsa1/Mco59HKmDdyoifLkOFuM7e58yPUXmK2LOWt6MmPVdp7UOchCedDQPPREtJ/YsAkSJRJxm8i Dw2TLdMg ygwK9IsNlPLTLV4Zl1azt6afPvztgxSDFJ8HSe1PkeDWYfwBDY3YsTXbMkMzMhQ7Cci4qht5+pK+ZkC4zypSzdqzfpzNHBJOSLjgQUo2DODbyKoOw/qDOAOVsFn/r4aybPk4RNSHPJsjwBWPEByraBp98O7UyJkK+cOakqHDbX05U3Je63vCnAcIN9tFAYpB8EMJCcIQrazmUb+/Qn3DGTon0ggYUSl2Kev+g5KddOvw3/D+AJX//A834kYwxodBzvyhrI/Sos79ohCEBKk7PMjsxJcxGx2cRvYbLCBptR9zdLTQ+51TtHXs1qBYIr+XhihQhTtqsJwJzGcDh7AGbTNBUB93A+gHqDus4vdKB3RM+kz/R2JN+bmgrIOGTTA08Uz5svi2eeWI0Tj/pdmeFJRIAS1gI7WuKgibUvNtkzkeYpeZ1p9m1G/SQQPlVlWbtVHawVhEID9scQEymAmoqNOFuRNPFu+mwmOyHnoiDE4YsWmlE6m9U1+onx5uTcnLH3BcV 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 2:39=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. > > Reviewed-by: Matthew Wilcox > Reviewed-by: David Hildenbrand No, this is a disaster. Please ask someone for help before you send it. Neither Willy nor David has ever posted any Reviewed-by tags. Please do get someone to help you. Stop posting like this! > Acked-by: Barry Song > Signed-off-by: Zhiguo Jiang > --- > > Change log: > v7->v8: > 1.Add tags of Reviewed-by and Acked-by. > 2.Add #include to solve compilation issue. > 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: > [v7->v8]: > 1.Barry Song > You should have collected tags such as reviewed-by, acked-by you got in > v6 while sending v7. > --> > Added in patch v8. > > You didn't even pass the compilation stage because you're missing > 'linux/oom.h'. It's quite disappointing because I believe in your idea, > but you didn't even build it before sending. > --> > Sorry, I overlooked the compilation of folio_likely_mapped_shared() added > in patch v5. Compiled and Updated have been compeleted in patch v8. > > [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 | 15 +++++++++++++++ > mm/vmscan.c | 7 ++++++- > 2 files changed, 21 insertions(+), 1 deletion(-) > mode change 100644 =3D> 100755 mm/rmap.c > > diff --git a/mm/rmap.c b/mm/rmap.c > index 26806b49a86f..5b92c3dadcc2 100644 > --- a/mm/rmap.c > +++ b/mm/rmap.c > @@ -75,6 +75,7 @@ > #include > #include > #include > +#include > > #include > > @@ -870,6 +871,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 >