Thread (8 messages) 8 messages, 3 authors, 2024-02-29

Re: [PATCH] powerpc/mm: Code cleanup for __hash_page_thp

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2024-02-29 06:18:38
Also in: lkml

Aneesh Kumar K.V [off-list ref] writes:
quoted hunk ↗ jump to hunk
Michael Ellerman [off-list ref] writes:
quoted
Kunwu Chan [off-list ref] writes:
quoted
On 2024/2/26 18:49, Michael Ellerman wrote:
quoted
Kunwu Chan [off-list ref] writes:
quoted
This part was commented from commit 6d492ecc6489
("powerpc/THP: Add code to handle HPTE faults for hugepages")
in about 11 years before.

If there are no plans to enable this part code in the future,
we can remove this dead code.
I agree the code can go. But I'd like it to be replaced with a comment
explaining what the dead code was trying to say.
quoted
Thanks, i'll update a new patch with the following comment:
     /*
     * No CPU has hugepages but lacks no execute, so we
     * don't need to worry about cpu no CPU_FTR_COHERENT_ICACHE feature case
     */
Maybe wait until we can get some input from Aneesh. I'm not sure the
code/comment are really up to date.
How about?

modified   arch/powerpc/mm/book3s64/hash_hugepage.c
@@ -58,17 +58,13 @@ int __hash_page_thp(unsigned long ea, unsigned long access, unsigned long vsid,
 		return 0;
 
 	rflags = htab_convert_pte_flags(new_pmd, flags);
+	/*
+	 * THPs are only supported on platforms that can do mixed page size
+	 * segments (MPSS) and all such platforms have coherent icache. Hence we
+	 * don't need to do lazy icache flush (hash_page_do_lazy_icache()) on
+	 * noexecute fault.
+	 */
Yeah thanks that looks good.

It could say "see eg. __hash_page_4K()", but that's probably unnecessary
as it mentions hash_page_do_lazy_icache(), and anyone interested is just
going to grep for that anyway.

cheers
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help