linux-kernel - Re: [RFC PATCH v2 3/8] mm: Hot page tracking and promotion

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <037112a2-7984-4c5b-8430-df5f167d7f84@amd.com>
Date: Mon, 6 Oct 2025 09:43:06 +0530
From: Bharata B Rao <bharata@....com>
To: <mgorman@...hsingularity.net>
CC: <linux-kernel@...r.kernel.org>, <linux-mm@...ck.org>,
	<dave.hansen@...el.com>, <gourry@...rry.net>, <hannes@...xchg.org>,
	<mingo@...hat.com>, <peterz@...radead.org>, <raghavendra.kt@....com>,
	<riel@...riel.com>, <rientjes@...gle.com>, <sj@...nel.org>,
	<weixugc@...gle.com>, <willy@...radead.org>, <ying.huang@...ux.alibaba.com>,
	<ziy@...dia.com>, <dave@...olabs.net>, <nifan.cxl@...il.com>,
	<xuezhengchu@...wei.com>, <yiannis@...corp.com>, <akpm@...ux-foundation.org>,
	<david@...hat.com>, <byungchul@...com>, <kinseyho@...gle.com>,
	<joshua.hahnjy@...il.com>, <yuanchu@...gle.com>, <balbirs@...dia.com>,
	<alok.rathore@...sung.com>
Subject: Re: [RFC PATCH v2 3/8] mm: Hot page tracking and promotion

On 03-Oct-25 4:47 PM, Jonathan Cameron wrote:
> On Wed, 10 Sep 2025 20:16:48 +0530
> Bharata B Rao <bharata@....com> wrote:
>> +
>> +struct max_heap {
>> +	size_t nr;
>> +	size_t size;
>> +	struct pghot_info **data;
>> +	DECLARE_FLEX_ARRAY(struct pghot_info *, preallocated);
> 
> That macro is all about use in unions rather than generally being needed.
> Do you need that here rather than
> 	struct pg_hot_info *preallocated[];
> 
> Can you add a __counted_by() marking?

I was using DEFINE_MIN_HEAP macro earlier which gave problems
when I had to define per-node instance of the heap with the
same name. The workaround for that resulted in the use of above
flex array.

Not needed, I will revert back to using array of pointers with
__counted_by() marking.

>> +
>> +	for (i = 0; i < hash_size; i++) {
>> +		INIT_HLIST_HEAD(&phi_hash[i].hash);
>> +		spin_lock_init(&phi_hash[i].lock);
>> +	}
>> +
>> +	phi_cache = KMEM_CACHE(pghot_info, 0);
>> +	if (unlikely(!phi_cache)) {
>> +		ret = -ENOMEM;
>> +		goto out;
> Whilst not strictly necessary I'd add multiple labels so only things
> that have been allocated are free rather than relying on them being
> NULL otherwise.  Whilst not a correctness thing it makes it slightly
> easier to check tear down paths are correct.

In general I agree but for freeing with a loop exit, the current
method appeared much simpler.

I will take care of rest of the review comments.

Regards,
Bharata.