Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Zi Yan <ziy@nvidia.com>
To:     Vlastimil Babka <vbabka@suse.cz>
CC:     Andrew Morton <akpm@linux-foundation.org>, <linux-mm@kvack.org>,
        Rik van Riel <riel@surriel.com>,
        <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] mm/compaction: count pages and stop correctly during page
 isolation.
Date:   Fri, 30 Oct 2020 11:18:07 -0400
Message-ID: <7C9FAF14-075D-4E29-B5F5-ADF15E687C3A@nvidia.com>
In-Reply-To: <16bdfad8-05f9-6ecf-0db6-c2dcf8e60309@suse.cz>
References: <20201029200435.3386066-1-zi.yan@sent.com>
 <16bdfad8-05f9-6ecf-0db6-c2dcf8e60309@suse.cz>
MIME-Version: 1.0
Content-Type: multipart/signed;
        boundary="=_MailMate_12CC4A4F-A68B-42A0-802B-75C263460CC2_=";
        micalg=pgp-sha512; protocol="application/pgp-signature"
Precedence: bulk

--=_MailMate_12CC4A4F-A68B-42A0-802B-75C263460CC2_=
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On 30 Oct 2020, at 10:50, Vlastimil Babka wrote:

> On 10/29/20 9:04 PM, Zi Yan wrote:
>> From: Zi Yan <ziy@nvidia.com>
>>
>> In isolate_migratepages_block, when cc->alloc_contig is true, we are
>> able to isolate compound pages, nr_migratepages and nr_isolated did no=
t
>> count compound pages correctly, causing us to isolate more pages than =
we
>> thought. Use thp_nr_pages to count pages. Otherwise, we might be trapp=
ed
>> in too_many_isolated while loop, since the actual isolated pages can g=
o
>> up to COMPACT_CLUSTER_MAX*512=3D16384, where COMPACT_CLUSTER_MAX is 32=
,
>> since we stop isolation after cc->nr_migratepages reaches to
>> COMPACT_CLUSTER_MAX.
>
> I wonder if a better fix would be to adjust the too_many_isolated() che=
ck so that if we have non-zero cc->nr_migratepages, we bail out from furt=
her isolation and migrate what we have immediately, instead of looping.

I just tested your fix and it works too. The difference is that with
your fix alloc_contig_range will fail quickly without killing the user
application mlocking THPs in the CMA region (for more context, please
see my other email to Andrew explaining how to reproduce in userspace),
whereas my fix will oom the user application and make alloc_contig_range
successful at the end.

Anyway, I will add your fix below and send v2:

diff --git a/mm/compaction.c b/mm/compaction.c
index ee1f8439369e..8fa11637ccfd 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -817,6 +817,9 @@ isolate_migratepages_block(struct compact_control *cc=
, unsigned long low_pfn,
         * delay for some time until fewer pages are isolated
         */
        while (unlikely(too_many_isolated(pgdat))) {
+               /* stop isolation if there are still pages not migrated *=
/
+               if (cc->nr_migratepages)
+                       return 0;
                /* async migration should just abort */
                if (cc->mode =3D=3D MIGRATE_ASYNC)
                        return 0;

>
> Because I can also imagine a hypothetical situation where multiple thre=
ads in parallel cause too_many_isolated() to be true, and will all loop t=
here forever. The proposed fix should prevent such situation as well, AFA=
ICT.

Yes. oom01 from ltp tests the multi-threaded situation and my fix works t=
here too.


=E2=80=94
Best Regards,
Yan Zi

--=_MailMate_12CC4A4F-A68B-42A0-802B-75C263460CC2_=
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="signature.asc"
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQJDBAEBCgAtFiEEh7yFAW3gwjwQ4C9anbJR82th+ooFAl+cLq8PHHppeUBudmlk
aWEuY29tAAoJEJ2yUfNrYfqK9D4QAKVpUdtG4B1p2K3qHffqsBxttPCXRVSU1e+N
9gSoxWs6/Q04FCkTMlOBwn34j9ZspH2pq/WngqKj8P//9UC/8z5WTnL6LU6E4NRY
btvjWlMA4xMeGgobB7N/1xYSg4Ejqw/m4Wh2sBBUHejZKfsRZBR4QLzAIwoTgqOb
5my/z7pSkrnczuw2cTCao+qJiJwpUZOXn47p8ymLqw8b70A3NDqh0iksbLM4uXsO
d+HG7u2P6JyqZIMeMKkFCMKT5ptXKx+cNY77nNUN15kktJ6gbTgSumzqU8fEyyIY
pnmMnHevwLWZzBX6pxGj9P8zVvqtq0w4FugYg3nDKCpnAKbqSxElshtwjql1fWq3
2u0w6QbOXIhGK57RlMzrhO3bhHYQ9tahboz/ln/wDqhDOjlrw5sCzg5HyZQAzR7G
PWPMHhXoCELIXre+LTtvOqpDLaGWBBeF4QjvgAkBBtF+0m4pM1relpGxjOPzcomG
t+zM25Ga1aTTqXEy/WrEf3SgkjZu14rP43go2W00qXAvGXkwtNF9sFbd7balvjyb
SreVTDZwjlT+QKh4pT7M9kfyZscPhIta/6HF/ZN5DLqfue1pY7tPm7kkSobZMs2J
7WtWT8gORb9C1kCsk9nPf2GlpPiyuMuG3hKSPiFD9GHRR9KTg0ly54FNFDSN8REY
QRose3Fq
=0VtA
-----END PGP SIGNATURE-----

--=_MailMate_12CC4A4F-A68B-42A0-802B-75C263460CC2_=--