Received-SPF: pass (google.com: domain of linux-kernel+bounces-105332-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240314141516.31747-1-liuhailong@oppo.com> <20240315081803.2223-1-liuhailong@oppo.com>
 <CAGsJ_4yMXE_4YuMTczF6u4YYp_TWksi+byszD09E2PT5pK8zfA@mail.gmail.com> <ba894762-d3ea-4773-b54c-ce063e1065e5@oppo.com>
In-Reply-To: <ba894762-d3ea-4773-b54c-ce063e1065e5@oppo.com>
From: Barry Song <21cnbao@gmail.com>
Date: Sun, 17 Mar 2024 18:47:42 +1300
Message-ID: <CAGsJ_4wD-NiquhnhK_6TGEF4reTGO9pVNGyYBnYZ1inVFc40WQ@mail.gmail.com>
Subject: Re: [PATCH v2] Revert "mm: skip CMA pages when they are not available"
To: =?UTF-8?B?5YiY5rW36b6ZKExhb0xpdSk=?= <liuhailong@oppo.com>
Cc: "akpm@linux-foundation.org" <akpm@linux-foundation.org>, "nathan@kernel.org" <nathan@kernel.org>, 
	"ndesaulniers@google.com" <ndesaulniers@google.com>, "trix@redhat.com" <trix@redhat.com>, 
	"linux-mm@kvack.org" <linux-mm@kvack.org>, 
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, 
	"llvm@lists.linux.dev" <llvm@lists.linux.dev>, "surenb@google.com" <surenb@google.com>, 
	"zhaoyang.huang@unisoc.com" <zhaoyang.huang@unisoc.com>, 
	"quic_charante@quicinc.com" <quic_charante@quicinc.com>, "yuzhao@google.com" <yuzhao@google.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Mar 15, 2024 at 11:44=E2=80=AFPM =E5=88=98=E6=B5=B7=E9=BE=99(LaoLiu=
) <liuhailong@oppo.com> wrote:
>
> On 2024/3/15 16:46, Barry Song wrote:
> > On Fri, Mar 15, 2024 at 9:18=E2=80=AFPM <liuhailong@oppo.com> wrote:
> >>
> >> From: "Hailong.Liu" <liuhailong@oppo.com>
> >>
> >> This reverts
> >> commit b7108d66318a ("Multi-gen LRU: skip CMA pages when they are not =
eligible")
> >> commit 5da226dbfce3 ("mm: skip CMA pages when they are not available")
> >
> > Reverting these changes seems inappropriate since the original patches
> > did improve most cases.
> > While doing direct reclamation without MOVABLE flags, scanning cma
> > folio doesn't help at all.
> >
>
> Absolutely, but without this patch  it give chance to reclaim cma pages t=
o freelist,
> other tasks without this flag would work normally without holding too muc=
h time
> on lru lock and lru size decrease=E3=80=82 it also trigger memory psi eve=
nt to allow
> admin do something to release memory.
>
> >>
> >> skip_cma may cause system not responding. if cma pages is large in lru=
_list
> >> and system is in lowmemory, many tasks would direct reclaim and waste
> >> cpu time to isolate_lru_pages and return.
> >>
> >> Test this patch on android-5.15 8G device
> >> reproducer:
> >> - cma_declare_contiguous 3G pages
> >> - set /proc/sys/vm/swappiness 0 to enable direct_reclaim reclaim file
> >>   only.
> >> - run a memleak process in userspace
> >
> >
> > I assume that scanning cma folio provides additional opportunities to r=
eclaim
> > anonymous memory, even though it is ineffective for allocating NON-MOVA=
BLE
> > folios. Consequently, we alleviate pressure for future allocations of a=
nonymous
> > folios. Moreover, setting swappiness=3D0 is a rare scenario. Instead of=
 entirely
> > removing the mechanism, could we detect such corner cases and handle th=
em
> > differently.
> >
> > Otherwise, I don't see any chance of this being acknowledged.
>
> It happens in internal aging test. open camera and record 8K video which =
use
> more than 2Gib dmabuf pages. without this patch the devices would kill ca=
mera process,
> but the system hang with it.
>
> In fact, in my opinion the issue of this patch increase the scanning lru =
time
> and reclaim nothing. Do you have a better solution ?

rather than aggressively reverting the original patch which might help lots=
 of
cases, could we just detect the corner cases and fix them , for example, by
something as the belows,

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 96147c4536e4..2f75ad9870c1 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -147,6 +147,9 @@ struct scan_control {
        /* Always discard instead of demoting to lower tier memory */
        unsigned int no_demotion:1;

+       /* no skipping cma */
+       unsigned int no_skip_cma:1;
+
        /* Allocation order */
        s8 order;

@@ -1595,7 +1598,7 @@ static __always_inline void
update_lru_sizes(struct lruvec *lruvec,
  */
 static bool skip_cma(struct folio *folio, struct scan_control *sc)
 {
-       return !current_is_kswapd() &&
+       return !current_is_kswapd() && !sc->no_skip_cma &&
                        gfp_migratetype(sc->gfp_mask) !=3D MIGRATE_MOVABLE =
&&
                        folio_migratetype(folio) =3D=3D MIGRATE_CMA;
 }
@@ -1636,7 +1639,7 @@ static unsigned long isolate_lru_folios(unsigned
long nr_to_scan,
        unsigned long nr_taken =3D 0;
        unsigned long nr_zone_taken[MAX_NR_ZONES] =3D { 0 };
        unsigned long nr_skipped[MAX_NR_ZONES] =3D { 0, };
-       unsigned long skipped =3D 0;
+       unsigned long skipped =3D 0, skipped_cma =3D 0;
        unsigned long scan, total_scan, nr_pages;
        LIST_HEAD(folios_skipped);

@@ -1645,6 +1648,7 @@ static unsigned long isolate_lru_folios(unsigned
long nr_to_scan,
        while (scan < nr_to_scan && !list_empty(src)) {
                struct list_head *move_to =3D src;
                struct folio *folio;
+               bool cma =3D false;

                folio =3D lru_to_folio(src);
                prefetchw_prev_lru_folio(folio, src, flags);
@@ -1652,10 +1656,11 @@ static unsigned long
isolate_lru_folios(unsigned long nr_to_scan,
                nr_pages =3D folio_nr_pages(folio);
                total_scan +=3D nr_pages;

-               if (folio_zonenum(folio) > sc->reclaim_idx ||
-                               skip_cma(folio, sc)) {
+               cma =3D skip_cma(folio, sc);
+               if (folio_zonenum(folio) > sc->reclaim_idx || cma) {
                        nr_skipped[folio_zonenum(folio)] +=3D nr_pages;
                        move_to =3D &folios_skipped;
+                       skipped_cma +=3D nr_pages;
                        goto move;
                }

@@ -1716,6 +1721,16 @@ static unsigned long
isolate_lru_folios(unsigned long nr_to_scan,
        *nr_scanned =3D total_scan;
        trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, nr_to_scan,
                                    total_scan, skipped, nr_taken, lru);
+
+       /*
+        * If the majority of folios are skipped due to contiguous
memory allocator (CMA),
+        * the least recently used (LRU) list might become overloaded
with CMA folios.
+        * This could result in numerous idle loops for non-movable
allocations. On the
+        * other hand, reclaiming CMA folios can alleviate the strain
on other processes
+        * that require anonymous pages.
+        */
+       sc->no_skip_cma =3D skipped_cma > total_scan / 2 ? 1 : 0;
+
        update_lru_sizes(lruvec, lru, nr_zone_taken);
        return nr_taken;
 }


This is merely a proof of concept (PoC), and I haven't even built it
yet. Additionally, LRU_GEN remains
unchanged. It will also need similar changes.

>
> Brs,
> Hailong.

Thanks
Barry