Received-SPF: pass (google.com: domain of linux-kernel+bounces-67636-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
From: Zi Yan <ziy@nvidia.com>
To: Vlastimil Babka <vbabka@suse.cz>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
 "\"Huang, Ying\"" <ying.huang@intel.com>,
 Ryan Roberts <ryan.roberts@arm.com>,
 Andrew Morton <akpm@linux-foundation.org>,
 "\"Matthew Wilcox (Oracle)\"" <willy@infradead.org>,
 David Hildenbrand <david@redhat.com>,
 "\"Yin, Fengwei\"" <fengwei.yin@intel.com>, Yu Zhao <yuzhao@google.com>,
 "\"Kirill A . Shutemov\"" <kirill.shutemov@linux.intel.com>,
 Johannes Weiner <hannes@cmpxchg.org>,
 Baolin Wang <baolin.wang@linux.alibaba.com>,
 Kemeng Shi <shikemeng@huaweicloud.com>,
 Mel Gorman <mgorman@techsingularity.net>,
 Rohan Puri <rohan.puri15@gmail.com>, Mcgrof Chamberlain <mcgrof@kernel.org>,
 Adam Manzanares <a.manzanares@samsung.com>,
 "\"Vishal Moola (Oracle)\"" <vishal.moola@gmail.com>
Subject: Re: [PATCH v5 2/3] mm/compaction: add support for >0 order folio
 memory compaction.
Date: Thu, 15 Feb 2024 15:04:02 -0500
Message-ID: <8FFA6A77-59C9-40E1-B999-C5C9EB5A2643@nvidia.com>
In-Reply-To: <25a80b95-93e6-4d68-a3b1-35cd3ef9c421@suse.cz>
References: <20240214220420.1229173-1-zi.yan@sent.com>
 <20240214220420.1229173-3-zi.yan@sent.com>
 <6a011e7f-4e21-4e31-a9a2-52cba3181337@suse.cz>
 <CA1FC23E-A652-4DD9-8E79-4956B2E11921@nvidia.com>
 <25a80b95-93e6-4d68-a3b1-35cd3ef9c421@suse.cz>
Content-Type: multipart/signed;
 boundary="=_MailMate_B0F46FDC-2DBD-49E2-B04D-46C9691AD48E_=";
 micalg=pgp-sha512; protocol="application/pgp-signature"
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?LpQ75dr+Rz2Ysy3CeF7cglTp+kB9HlVrXFQHiuGUWDgcGlQJGLeGbSoNKC7I?=
 =?us-ascii?Q?bLcJ5tlfSsx/Rkqr3oMhlkeJxCy8ssMMGd+WVeU6oi16cLQr1rMCJqk/Q0CX?=
 =?us-ascii?Q?VTR0+l8syAP75x3HB5haAwEcFJO5tp035/s87tZGUhg68XKWhW5oQfwuEcek?=
 =?us-ascii?Q?k90rSttHrSRrFVZz757Ur3m/570bf6zq29Iy34w20W+EHvQ79ub6qBCgMDBa?=
 =?us-ascii?Q?5lB1QsW9dNz23j52FNe5BsRB9SxCQ9fyrOhwDwrkhRiPecgojQ8LIl33xZ/L?=
 =?us-ascii?Q?T9HwPUX0dSywFFyCvEBa4tfrUHwhiyr80K418iJ/y03/Ra5O7Vgl8lB49syk?=
 =?us-ascii?Q?D+iM71Ok1HkPOgb+3p2ubBLETc/2bASoP/e+NDvBArHdPMapOE/4gfzMmfaK?=
 =?us-ascii?Q?ie7wLuvRT3aRLweTY8Jn3TyeWq/7I/i3Gmh6hS9ewU1KsLiGmbtHdPY/8U5Y?=
 =?us-ascii?Q?lJLxnLFlHNLBLsYebNDt6r5bOjVauGqjzoaUiwLXx+Zc5B+VVZ/e4LGJdDZg?=
 =?us-ascii?Q?GzVTKyXd+w+EkGi3VvVLxlVhXyi0olGDLKtE4DuHaFttQVLx+pYtaACd4hoj?=
 =?us-ascii?Q?8Q2AO58lXvSwOHmusfJDTEzhkmSMP8qWNLHSYlCLtRofRH88rRb2Mlx8VAUM?=
 =?us-ascii?Q?c2ut6RG1V7rQgrW+E/ZF/uLw+6FVDc62krNWQmtTVjVxfoNPk/L6Whzs8Y26?=
 =?us-ascii?Q?LGC781cK3w6SZ32TDPiAC+mhpF8aXuYMFhpVIYT2OsGkIuKK3BbLXN0ViUBW?=
 =?us-ascii?Q?v4jsOHTz5Hzop7OnnWAFKt+qLYRr2i+JN7k2sT0pRPAD7wqnGHVDHwTR8lrR?=
 =?us-ascii?Q?cByzK7eXEr8OIuU9dgAKSw8hwwdeogHYZQRXzMWgTmRHpKuq/jA+Pu51YXB8?=
 =?us-ascii?Q?e3vqdTllATGgZskoOHU7wei+UPY2WZAwTHMKmp++jncgRl/SWcr/fqb3a4F7?=
 =?us-ascii?Q?Luqu+L6JyH7Jwo8scKpNiNXp5VcB++CzL4Xgl8SUvEKqZwM6OoR1dfA+1t+U?=
 =?us-ascii?Q?GrK5eXobib5lkfcDqwr2CO0ufJK7mVFyHnCyMKmJ/+Qo7JuWmMJQtcIRGS9Z?=
 =?us-ascii?Q?AmBk+6iCjfhfAmSrEGgTKb5vKg5sKga4WfU+TVXCLV7kH9yw6uXuUnv4sYjV?=
 =?us-ascii?Q?A+e+3CG1O0E4iVpLMdudg6HCrUZSELPcNtnvReNzRMB+0oJpTpfZghboStkT?=
 =?us-ascii?Q?LLvWcyj8jdriemuu6SPTbH5SJCLCSALUgH9fR8XNdU00oJ0Xu38AnFupFUYL?=
 =?us-ascii?Q?/qTHnsgpcFibQmjQwPCdwn+8PjbpffxSk/+F1cgwp6AxvGiWhCpd3tuQknnE?=
 =?us-ascii?Q?5Us5j6NdfFQguYdC5bnflZzF5sc0Ee5SZ9fVZe09SYg3hM16cIFa6YiLMybJ?=
 =?us-ascii?Q?FcVT4xuN5aLxoHz5FKo5mgMB3Wzz1c9tR7vSK130f/WJMyPyD9uRBVEjQkaR?=
 =?us-ascii?Q?A9GtVzHFsjgdIkbfU/kTZNPxZGS5zgi624y7UDMMrwrNus6PBr0zge3UGqyf?=
 =?us-ascii?Q?NSn3xLE/A07ZF3wCCcMhPywKYvZuJ73zHfl7W+Mki7ti50Cq5XuEuyjt0izR?=
 =?us-ascii?Q?MVZgUq0V45UHFEwEkZA=3D?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 319520ca-b749-49e3-e4e0-08dc2e61430e
X-MS-Exchange-CrossTenant-AuthSource: DS7PR12MB5744.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Feb 2024 20:04:05.3139
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: YyzpSxljkvbJPCUH9218L8waJoO4SIrGRXhtiIKGBAFht/RoT7AISbFdEGDcJSN7
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4249

--=_MailMate_B0F46FDC-2DBD-49E2-B04D-46C9691AD48E_=
Content-Type: text/plain
Content-Transfer-Encoding: quoted-printable

On 15 Feb 2024, at 15:02, Vlastimil Babka wrote:

> On 2/15/24 18:32, Zi Yan wrote:
>> On 15 Feb 2024, at 11:57, Vlastimil Babka wrote:
>>
>>> On 2/14/24 23:04, Zi Yan wrote:
>>>> @@ -1849,10 +1857,22 @@ static struct folio *compaction_alloc(struct=
 folio *src, unsigned long data)
>>>>  static void compaction_free(struct folio *dst, unsigned long data)
>>>>  {
>>>>  	struct compact_control *cc =3D (struct compact_control *)data;
>>>> +	int order =3D folio_order(dst);
>>>> +	struct page *page =3D &dst->page;
>>>> +
>>>> +	if (folio_put_testzero(dst)) {
>>>> +		free_pages_prepare_fpi_none(page, order);
>>>> +
>>>> +		INIT_LIST_HEAD(&dst->lru);
>>>
>>> (is this even needed? I think the state of first parameter of list_ad=
d() is
>>> never expected to be in particular state?)
>>
>> There is a __list_add_valid() performing list corruption checks.
>
> Yes, but dst->lru becomes "new" in list_add() and __list_add_valid() an=
d
> those never check the contents of new, i.e. new->next or new->prev. We =
could
> have done list_del(&dst->lru) which puts poison values there and then a=

> list_add() is fine. So dst->lru does not need the init, it's just confu=
sing.
> Init is for the list's list_head, not for the list entry.

Got it. Will remove it.

>>>>
>>>> -	list_add(&dst->lru, &cc->freepages);
>>>> -	cc->nr_freepages++;
>>>> -	cc->nr_migratepages +=3D 1 << folio_order(dst);
>>>> +		list_add(&dst->lru, &cc->freepages[order]);
>>>> +		cc->nr_freepages +=3D 1 << order;
>>>> +		cc->nr_migratepages +=3D 1 << order;
>>>
>>> Hm actually this increment of nr_migratepages should happen even if w=
e lost
>>> the free page.
>>
>> Because compaction_free() indicates the page is not migrated and nr_mi=
gratepages
>> should be increased regardless.
>
> Yes.
>
>> Will fix it. Thanks.
>>
>>>> +	}
>>>> +	/*
>>>> +	 * someone else has referenced the page, we cannot take it back to=
 our
>>>> +	 * free list.
>>>> +	 */
>>>>  }
>>
>>
>> --
>> Best Regards,
>> Yan, Zi


--
Best Regards,
Yan, Zi

--=_MailMate_B0F46FDC-2DBD-49E2-B04D-46C9691AD48E_=
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename=signature.asc
Content-Type: application/pgp-signature; name=signature.asc

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

iQJDBAEBCgAtFiEE6rR4j8RuQ2XmaZol4n+egRQHKFQFAmXObjIPHHppeUBudmlk
aWEuY29tAAoJEOJ/noEUByhUbRIQAJSqRDuq9E608suyLANOBkwzoy91swtR/YN6
cnfQ2PSPGvcddeapP+w85P5v5/CZm0pgA3cKU8y34nu6c95CLSvDSwNf3ZtSvvfc
KwVNdz6uAxcOEEkjkEkx6HBCg+4jH7eEBbYuRAjgwpyZGYuosOEaZ2AcLv2VmGAz
rX78GFykIE0ATq7tygglibHFbC4AF2GQ/fFL1XehwMXy+MrKr3FQahV4PnXXY2i3
LzqQgmHJmTE/qHhHorYHLR1SoDk8Om/yluRkvlD8I+6u1hqOSfuIgvG9wf/K2vVm
lVMnUTz+5rwPq9W3oABn04NwgSt42ZtQXbYamTzuXLOQyj9CLaWJO3B06fV219+M
b6d0VJf/tXdCuxQjCUogjjgE80YDs9WBdLLWYaWgU2/yNV+KXowyRBmzQRDKb9AK
LUJQjURm/3eNnD83YQ3ybKQyaV6cRLhM071IEHJPxbJK7uJpHAahZkMF6xyTV8bl
QPTrFkkYs+jcq9k07UIjfbT9ZKiszyJSzRnvyhS5qc/g3gvvVfPr4gfY9LNcRXhy
rEPxup4qkKuwbxk5W3j1LeDQDJTQkl2Y8YqcehKzpZwvftnWOZScXtUNayiF3Kj3
vNXaAnGIuChFFEqFcECGSDNXZUXbt++JZ0U3kNyIzMJMcC+GomWKu2Gk1680rZ6c
9fv3ob5Z
=QopK
-----END PGP SIGNATURE-----

--=_MailMate_B0F46FDC-2DBD-49E2-B04D-46C9691AD48E_=--