Received-SPF: pass (google.com: domain of linux-kernel+bounces-41555-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Sun, 28 Jan 2024 06:35:36 +0000
From: Matthew Wilcox <willy@infradead.org>
To: zhiguojiang <justinjiang@vivo.com>
Cc: Andrew Morton <akpm@linux-foundation.org>, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, opensource.kernel@vivo.com
Subject: Re: [PATCH] mm:vmscan: shrink skip folios in the exiting task
Message-ID: <ZbX1uEbrN-lwnnaW@casper.infradead.org>
References: <20240124124308.461-1-justinjiang@vivo.com>
 <ZbEOrim8tjMXB5os@casper.infradead.org>
 <726d018c-01d0-48b3-870b-c63fb06eb0d0@vivo.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <726d018c-01d0-48b3-870b-c63fb06eb0d0@vivo.com>

On Thu, Jan 25, 2024 at 09:34:53AM +0800, zhiguojiang wrote:
> > > In the scenarios of the low memory system and mutil backed-applications,
> > > the time-consuming problem caused by shrinking the exiting task's folios
> > > will be more severe.
> > 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?)
> 1 Yes, this patch has been tested.
> 
> 2 When the exiting tasks and shrink_inactive_list occur at the same time,
> ?? the folios which shrink_inactive_list reclaims may be the exiting tasks's
> folios
> ?? in lruvecs. And when system is low memory, it more likely to occur,
> because
> ?? more backend applidatuions will be killed.
> ?? The shrink_inactive_list reclaims the exiting tasks's folios in lruvecs
> and
> ?? transforms the exiting tasks's anon folios into swap memory, which leads
> ?? to the increasing load of the current exiting tasks.

Ah, we're talking about an OOM scenario.  OK, that makes some sense.
You should have mentioned that in the patch description.

> 3 This patch can alleviate the load of the tasks exiting process. This patch
> ?? can make that the exiting tasks release its anon folios faster instead of
> ?? releasing its swap memory from its anon folios swap-in in
> shrink_inactive_list.
> 
> 4 "mutil backed-applications" means that there are a large number of
> ??? the backend applications in the system.
> 
> Thanks
> > 
> > And I do mean specifically of this patch, because to my eyes it
> > shouldn't even compile.
> Has been tested.

That's odd.  I thought GCC used to complain about

	long x = 0x100000000;

but it does the right thing.  Except on 32-bit where it'll say
"warning: integer constant out of range".

In any case, the number you chose is not going to work on 32-bit systems
or on ARM or x86.  It conflicts with protection keys on x86 and MTE on
ARM.

We can do it without any new magic numbers.  I'm not sure this is a
great approach, but this should work:

+++ b/mm/rmap.c
@@ -840,6 +840,12 @@ static bool folio_referenced_one(struct folio *folio,
        int referenced = 0;
        unsigned long start = address, ptes = 0;

+       /* Skip this folio if it's mapped by an exiting task */
+       if (test_bit(MMF_OOM_SKIP, &vma->vm_mm->flags)) {
+               pra->referenced = -1;
+               return false;
+       }
+
        while (page_vma_mapped_walk(&pvmw)) {
                address = pvmw.address;