linux-kernel - Re: [PATCH 11/12] mm: pgtable: introduce generic __tlb_remove

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: <9c6a4e46-7f98-4809-b4bb-fa83a4461474@bytedance.com>
Date: Mon, 16 Dec 2024 21:53:24 +0800
From: Qi Zheng <zhengqi.arch@...edance.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: 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, linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 11/12] mm: pgtable: introduce generic __tlb_remove_table()



On 2024/12/16 20:05, Peter Zijlstra wrote:
> On Mon, Dec 16, 2024 at 01:03:13PM +0100, Peter Zijlstra wrote:
>> On Mon, Dec 16, 2024 at 01:00:43PM +0100, Peter Zijlstra wrote:
>>> On Sat, Dec 14, 2024 at 05:02:57PM +0800, Qi Zheng wrote:
>>>
>>>> diff --git a/arch/s390/mm/pgalloc.c b/arch/s390/mm/pgalloc.c
>>>> index c73b89811a264..3e002dea6278f 100644
>>>> --- a/arch/s390/mm/pgalloc.c
>>>> +++ b/arch/s390/mm/pgalloc.c
>>>> @@ -193,13 +193,6 @@ void page_table_free(struct mm_struct *mm, unsigned long *table)
>>>>   	pagetable_dtor_free(ptdesc);
>>>>   }
>>>>   
>>>> -void __tlb_remove_table(void *table)
>>>> -{
>>>> -	struct ptdesc *ptdesc = virt_to_ptdesc(table);
>>>> -
>>>> -	pagetable_dtor_free(ptdesc);
>>>> -}
>>>> -
>>>>   #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>>>>   static void pte_free_now(struct rcu_head *head)
>>>>   {
>>>
>>>> diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h
>>>> index 709830274b756..939a813023d7e 100644
>>>> --- a/include/asm-generic/tlb.h
>>>> +++ b/include/asm-generic/tlb.h
>>>
>>>>   #define MAX_TABLE_BATCH		\
>>>>   	((PAGE_SIZE - sizeof(struct mmu_table_batch)) / sizeof(void *))
>>>>   
>>>> +#ifndef __HAVE_ARCH_TLB_REMOVE_TABLE
>>>> +static inline void __tlb_remove_table(void *_table)
>>>> +{
>>>> +	struct ptdesc *ptdesc = (struct ptdesc *)_table;
>>>> +
>>>> +	pagetable_dtor(ptdesc);
>>>> +	pagetable_free(ptdesc);
>>>> +}
>>>> +#endif
>>>
>>>
>>> Spot the fail...
>>>
>>> That said, all this ptdesc stuff is another giant trainwreck. Let me
>>> clean that up for you.
>>>
>>> ---
>>
>>> -static inline void __tlb_remove_table(void *_table)
>>> +static inline void __tlb_remove_table(void *table)
>>>   {
>>> -	struct ptdesc *ptdesc = (struct ptdesc *)_table;
>>> +	struct ptdesc *ptdesc = page_to_ptdesc(table);
>>>   
>>>   	pagetable_dtor(ptdesc);
>>>   	pagetable_free(ptdesc);
>>
>> And now observe that __tlb_remove_table() is identical to
>> asm-generic/pgalloc.h pte_free(), pmd_free(), __pud_free() and
>> __p4d_free().
>>
>> And I'm sure we don't need 5 copies of this :-), wdyt?
> 
> Argh, nearly, it has the whole page vs virt nonsense still going on.
> Bah.

Yes, maybe it can be unified into struct page parameter, which seems
to be more convenient for conversion:

static inline void pagtable_dtor_free(struct mm_struct *mm, void *table)
{
	struct ptdesc *ptdesc = page_to_ptdesc(table);

	pagetable_dtor(ptdesc);
	pagetable_free(ptdesc);
}