kasan: call clear_page with a match-all tag instead of changing page tag

PATCH Applied Peter Collingbourne 3 participants 3 messages Feb 16, 2023 – Feb 20, 2023

Subsystems (2)

Memory management MM core

lore

Share & tools

lore

Peter Collingbourne

author

2023-02-16 19:59 UTC #0 lore raw

PATCH v2 1/1 kasan: call clear_page with a match-all tag instead of changing page tag

Instead of changing the page's tag solely in order to obtain a pointer with a match-all tag and then changing it back again, just convert the pointer that we get from kmap_atomic() into one with a match-all tag before passing it to clear_page().

On a certain microarchitecture, this has been observed to cause a measurable improvement in microbenchmark performance, presumably as a result of being able to avoid the atomic operations on the page tag.

Signed-off-by Peter Collingbourne <[email protected]>

Link https://linux-review.googlesource.com/id/I0249822cc29097ca7a04ad48e8eb14871f80e711

---
v2:
- switched to kmap_local_page()

 include/linux/highmem.h | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

include/linux/highmem.h +3 -5

index 44242268f53b..212fd081b227 100644
@@ -245,12 +245,10 @@ static inline void clear_highpage(struct page *page)
 
 static inline void clear_highpage_kasan_tagged(struct page *page)
 {
-	u8 tag;
+	void *kaddr = kmap_local_page(page);
 
-	tag = page_kasan_tag(page);
-	page_kasan_tag_reset(page);
-	clear_highpage(page);
-	page_kasan_tag_set(page, tag);
+	clear_page(kasan_reset_tag(kaddr));
+	kunmap_local(kaddr);
 }
 
 #ifndef __HAVE_ARCH_TAG_CLEAR_HIGHPAGE

Signature · 1 line

2.39.2.637.g21b0678d19-goog

Andrey Konovalov

Reviewed-by on v2 1/1

2023-02-17 09:56 UTC #1 lore raw

Quoted 40 lines · Peter Collingbourne

Instead of changing the page's tag solely in order to obtain a pointer with a match-all tag and then changing it back again, just convert the pointer that we get from kmap_atomic() into one with a match-all tag before passing it to clear_page().
On a certain microarchitecture, this has been observed to cause a measurable improvement in microbenchmark performance, presumably as a result of being able to avoid the atomic operations on the page tag.
Signed-off-by Peter Collingbourne <[email protected]>
Link https://linux-review.googlesource.com/id/I0249822cc29097ca7a04ad48e8eb14871f80e711
---
v2:
- switched to kmap_local_page()
 include/linux/highmem.h | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
include/linux/highmem.h +3 -6
index 44242268f53b..212fd081b227 100644
@@ -245,12 +245,10 @@ static inline void clear_highpage(struct page *page)
 
 static inline void clear_highpage_kasan_tagged(struct page *page)
 {
-       u8 tag;
+       void *kaddr = kmap_local_page(page);
 
-       tag = page_kasan_tag(page);
-       page_kasan_tag_reset(page);
-       clear_highpage(page);
-       page_kasan_tag_set(page, tag);
+       clear_page(kasan_reset_tag(kaddr));
+       kunmap_local(kaddr);
 }
 
 #ifndef __HAVE_ARCH_TAG_CLEAR_HIGHPAGE
--

Quoted 2 lines

2.39.2.637.g21b0678d19-goog

Reviewed-by Andrey Konovalov <[email protected]>

Thanks!

Catalin Marinas

Reviewed-by on v2 1/1

2023-02-20 15:55 UTC #2 lore raw

In reply to Peter Collingbourne

Quoted 11 lines · Peter Collingbourne

Instead of changing the page's tag solely in order to obtain a pointer with a match-all tag and then changing it back again, just convert the pointer that we get from kmap_atomic() into one with a match-all tag before passing it to clear_page().
On a certain microarchitecture, this has been observed to cause a measurable improvement in microbenchmark performance, presumably as a result of being able to avoid the atomic operations on the page tag.
Signed-off-by Peter Collingbourne <[email protected]>
Link https://linux-review.googlesource.com/id/I0249822cc29097ca7a04ad48e8eb14871f80e711

Reviewed-by Catalin Marinas <[email protected]>

Not sure how this will go upstream, maybe the mm tree? Otherwise happy to take it through the arm64 tree.

Thanks.

Signature · 1 line

Catalin