Christophe Leroy <christophe.le...@csgroup.eu> writes: > Le 21/02/2024 à 13:09, Michael Ellerman a écrit : >> Christophe Leroy <christophe.le...@csgroup.eu> writes: >>> set_memory_p() and set_memory_np() can fail. >>> >>> As mentioned in linux/mm.h: >>> >>> /* >>> * To support DEBUG_PAGEALLOC architecture must ensure that >>> * __kernel_map_pages() never fails >>> */ >>> >>> So panic in case set_memory_p() or set_memory_np() fail >>> in __kernel_map_pages(). >>> >>> Link: https://github.com/KSPP/linux/issues/7 >>> Signed-off-by: Christophe Leroy <christophe.le...@csgroup.eu> >>> --- >>> arch/powerpc/include/asm/book3s/64/hash.h | 2 +- >>> arch/powerpc/mm/book3s64/hash_utils.c | 3 ++- >>> arch/powerpc/mm/pageattr.c | 10 +++++++--- >>> 3 files changed, 10 insertions(+), 5 deletions(-) >>> >> ... >>> diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c >>> index 16b8d20d6ca8..62b678585878 100644 >>> --- a/arch/powerpc/mm/pageattr.c >>> +++ b/arch/powerpc/mm/pageattr.c >>> @@ -106,17 +106,21 @@ int change_memory_attr(unsigned long addr, int >>> numpages, long action) >>> #ifdef CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC >>> void __kernel_map_pages(struct page *page, int numpages, int enable) >>> { >>> + int err; >>> unsigned long addr = (unsigned long)page_address(page); >>> >>> if (PageHighMem(page)) >>> return; >>> >>> if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && !radix_enabled()) >>> - hash__kernel_map_pages(page, numpages, enable); >>> + err = hash__kernel_map_pages(page, numpages, enable); >>> else if (enable) >>> - set_memory_p(addr, numpages); >>> + err = set_memory_p(addr, numpages); >>> else >>> - set_memory_np(addr, numpages); >>> + err = set_memory_np(addr, numpages); >>> + >>> + if (err) >>> + panic("%s: set_memory_%sp() failed\n", enable ? "" : "n"); >> >> This doesn't compile, it's missing __func__ I guess. > > Damn, I was too quick when I took into account checkpatch's feedback, > sorry for that. > >> >> Seems like we could keep it simpler though, it should hopefully never >> happen anyway, eg: >> >> panic("%s: changing memory protections failed\n", __func__); > > Sure, let's do that. Do you want a v2 or you do the change directly ?
No need for a v2, I'll just fix it up when applying. cheers