linux-kernel - Re: [PATCH v2 02/15] mm: pgtable: introduce generic p4d_alloc_one() and p4d

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: <0d22a66a-8c35-40f2-be1c-0a4a94885d9a@bytedance.com>
Date: Fri, 20 Dec 2024 11:19:13 +0800
From: Qi Zheng <zhengqi.arch@...edance.com>
To: Alexander Gordeev <agordeev@...ux.ibm.com>
Cc: peterz@...radead.org, tglx@...utronix.de, david@...hat.com,
 jannh@...gle.com, hughd@...gle.com, yuzhao@...gle.com, willy@...radead.org,
 muchun.song@...ux.dev, vbabka@...nel.org, lorenzo.stoakes@...cle.com,
 akpm@...ux-foundation.org, rientjes@...gle.com, vishal.moola@...il.com,
 linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 02/15] mm: pgtable: introduce generic p4d_alloc_one()
 and p4d_free()

Hi Alexander,

On 2024/12/20 00:32, Alexander Gordeev wrote:
> On Wed, Dec 18, 2024 at 09:04:38PM +0800, Qi Zheng wrote:
>> For these architectures, provide a generic implementation in
>> asm-generic/pgalloc.h and convert them to use it. And like other levels
>> of page tables, add statistics for P4D level page table.
> 
> FWIW, introducing the generic implementation and adding ctor|dtors
> look as separate changes to me.

OK, will separate it in v3.

> 
> ...
>>   static inline void p4d_free(struct mm_struct *mm, p4d_t *p4d)
>>   {
>> -	if (!pgtable_l5_enabled())
>> -		return;
>> -	BUG_ON((unsigned long)p4d & (PAGE_SIZE-1));
> 
> Or maybe?
> 
> 	BUG_ON(!IS_ALIGNED((unsigned long)p4d, PAGE_SIZE));

Do you mean to keep BUG_ON() and change it to the above form?

I would prefer to just drop BUG_ON() as is done with the other
page tables.

Thanks,
Qi

> 
>> -	free_page((unsigned long)p4d);
>> +	if (pgtable_l5_enabled())
>> +		__p4d_free(mm, p4d);
>>   }