Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Message-ID: <47198271414db19cecbfa1a6ea685577dad3a72c.camel@surriel.com>
Subject: Re: [PATCH v2 2/2] mm,thp,compaction,cma: allow THP migration for
 CMA allocations
From:   Rik van Riel <riel@surriel.com>
To:     Vlastimil Babka <vbabka@suse.cz>,
        Mike Kravetz <mike.kravetz@oracle.com>,
        linux-kernel@vger.kernel.org
Cc:     kernel-team@fb.com, akpm@linux-foundation.org, linux-mm@kvack.org,
        mhocko@kernel.org, mgorman@techsingularity.net,
        rientjes@google.com, aarcange@redhat.com, ziy@nvidia.com
Date:   Fri, 28 Feb 2020 09:32:04 -0500
In-Reply-To: <67185d77-87aa-400d-475c-4435d8b7be11@suse.cz>
References: <cover.1582321646.git.riel@surriel.com>
         <20200227213238.1298752-2-riel@surriel.com>
         <df83c62f-209f-b1fd-3a5c-c81c82cb2606@oracle.com>
         <7800e98e3688c124ac3672284b87d67321e1c29e.camel@surriel.com>
         <67185d77-87aa-400d-475c-4435d8b7be11@suse.cz>
Content-Type: multipart/signed; micalg="pgp-sha256";
        protocol="application/pgp-signature"; boundary="=-y/7ndlqWhwsZ6DQBptKq"
User-Agent: Evolution 3.34.3 (3.34.3-1.fc31) 
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


--=-y/7ndlqWhwsZ6DQBptKq
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, 2020-02-28 at 09:25 +0100, Vlastimil Babka wrote:
> On 2/28/20 2:21 AM, Rik van Riel wrote:
> > On Thu, 2020-02-27 at 15:41 -0800, Mike Kravetz wrote:
> > > On 2/27/20 1:32 PM, Rik van Riel wrote:
> > > > +++ b/mm/page_alloc.c
> > > > @@ -8253,14 +8253,19 @@ struct page *has_unmovable_pages(struct
> > > > zone *zone, struct page *page,
> > > > =20
> > > >  		/*
> > > >  		 * Hugepages are not in LRU lists, but they're
> > > > movable.
> > > > +		 * THPs are on the LRU, but need to be counted
> > > > as
> > > > #small pages.
> > > >  		 * We need not scan over tail pages because we
> > > > don't
> > > >  		 * handle each tail page individually in
> > > > migration.
> > > >  		 */
> > > > -		if (PageHuge(page)) {
> > > > +		if (PageHuge(page) || PageTransCompound(page))
> > > > {
> > > >  			struct page *head =3D
> > > > compound_head(page);
> > > >  			unsigned int skip_pages;
> > > > =20
> > > > -			if
> > > > (!hugepage_migration_supported(page_hstate(head)))
> > > > +			if (PageHuge(page) &&
> > > > +			    !hugepage_migration_supported(page_
> > > > hstate(h
> > > > ead)))
> > > > +				return page;
> > > > +
> > > > +			if (!PageLRU(head) &&
> > > > !__PageMovable(head))
> > >=20
> > > Pretty sure this is going to be true for hugetlb pages.  So, this
> > > will change
> > > behavior and make all hugetlb pages look unmovable.  Perhaps,
> > > only
> > > check this
> > > condition for THP pages?
>=20
> Oh right you are.
>=20
> > Does that need to be the following, then?
> >=20
> >      if (PageTransHuge(head) && !PageHuge(page) && !PageLRU(head)
> > &&
> > !__PageMovable(head))
> >                  return page;
>=20
> I would instead make it an "else if" to the "if (PageHuge(page)...)"
> above.

That was my first thought too, but that could break on
pages that are PageHuge when hugepage_migration_supported
returns true.

--=20
All Rights Reversed.

--=-y/7ndlqWhwsZ6DQBptKq
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: This is a digitally signed message part
Content-Transfer-Encoding: 7bit

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

iQEzBAABCAAdFiEEKR73pCCtJ5Xj3yADznnekoTE3oMFAl5ZJGQACgkQznnekoTE
3oNXLwgAqvXeuLmGzd0RhDnDwPGxGSbF9BXHxPUdd4pkgLQ19YJDy4KjiqJ4Qygn
OywW3UgFi9K9FwKZsbA3ZIID8+mm1H9qmTAaTMlzq7ItnexGARXvynJcmiUWt1lb
OgLKf4dnLT3k6naB2ebO6S+Qkxro08JwIhKbA3ATCZkJcGRSNZDqIsvL8PPHf0+3
Z+TLUyvEEQnmqRUCVcgrBPXsSig0Uh2zq3V0BmdCSK4q9/UdKgyd9xrxueJtkj3R
q4zyOEvB01b0lCR6Wue6FCDPLaEKq3NZ44dw/VYFFy+q6Qs+Poy7g1UGz22vAoWo
OnJFOG6XoOg2MbLlKhVuZvkIdDg7uQ==
=Lf9E
-----END PGP SIGNATURE-----

--=-y/7ndlqWhwsZ6DQBptKq--