Thread (24 messages) 24 messages, 3 authors, 2021-04-19

Re: [PATCH v1 3/5] mm: ptdump: Provide page size to notepage()

From: Steven Price <steven.price@arm.com>
Date: 2021-04-16 10:51:27
Also in: linux-arch, linux-arm-kernel, linux-mm, linux-riscv, linux-s390, lkml

On 16/04/2021 11:38, Christophe Leroy wrote:

Le 16/04/2021 à 11:28, Steven Price a écrit :
quoted
On 15/04/2021 18:18, Christophe Leroy wrote:
quoted
In order to support large pages on powerpc, notepage()
needs to know the page size of the page.

Add a page_size argument to notepage().

Signed-off-by: Christophe Leroy <redacted>
---
  arch/arm64/mm/ptdump.c         |  2 +-
  arch/riscv/mm/ptdump.c         |  2 +-
  arch/s390/mm/dump_pagetables.c |  3 ++-
  arch/x86/mm/dump_pagetables.c  |  2 +-
  include/linux/ptdump.h         |  2 +-
  mm/ptdump.c                    | 16 ++++++++--------
  6 files changed, 14 insertions(+), 13 deletions(-)
[...]
quoted
diff --git a/mm/ptdump.c b/mm/ptdump.c
index da751448d0e4..61cd16afb1c8 100644
--- a/mm/ptdump.c
+++ b/mm/ptdump.c
@@ -17,7 +17,7 @@ static inline int note_kasan_page_table(struct 
mm_walk *walk,
  {
      struct ptdump_state *st = walk->private;
-    st->note_page(st, addr, 4, pte_val(kasan_early_shadow_pte[0]));
+    st->note_page(st, addr, 4, pte_val(kasan_early_shadow_pte[0]), 
PAGE_SIZE);
I'm not completely sure what the page_size is going to be used for, 
but note that KASAN presents an interesting case here. We short-cut by 
detecting it's a KASAN region at a high level (PGD/P4D/PUD/PMD) and 
instead of walking the tree down just call note_page() *once* but with 
level==4 because we know KASAN sets up the page table like that.

However the one call actually covers a much larger region - so while 
PAGE_SIZE matches the level it doesn't match the region covered. 
AFAICT this will lead to odd results if you enable KASAN on powerpc.
Hum .... I successfully tested it with KASAN, I now realise that I 
tested it with CONFIG_KASAN_VMALLOC selected. In this situation, since 
https://github.com/torvalds/linux/commit/af3d0a686 we don't have any 
common shadow page table anymore.

I'll test again without CONFIG_KASAN_VMALLOC.
quoted
To be honest I don't fully understand why powerpc requires the 
page_size - it appears to be using it purely to find "holes" in the 
calls to note_page(), but I haven't worked out why such holes would 
occur.
I was indeed introduced for KASAN. We have a first commit 
https://github.com/torvalds/linux/commit/cabe8138 which uses page size 
to detect whether it is a KASAN like stuff.

Then came https://github.com/torvalds/linux/commit/b00ff6d8c as a fix. I 
can't remember what the problem was exactly, something around the use of 
hugepages for kernel memory, came as part of the series 
https://patchwork.ozlabs.org/project/linuxppc-dev/cover/cover.1589866984.git.christophe.leroy@csgroup.eu/ 
Ah, that's useful context. So it looks like powerpc took a different 
route to reducing the KASAN output to x86.

Given the generic ptdump code has handling for KASAN already it should 
be possible to drop that from the powerpc arch code, which I think means 
we don't actually need to provide page size to notepage(). Hopefully 
that means more code to delete ;)

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