Skip to contentSkip to conversation

mm/pgtable: define pte_index so that preprocessor could recognize it

PATCH In review Mike Rapoport 3 participants 4 messages Jan 11, 2022

Subsystems (3)
Stable branch Memory management MM core
lore
Share & tools
lore

Mike Rapoport

author reviewer
#0 lore raw

PATCH 1/1 mm/pgtable: define pte_index so that preprocessor could recognize it

From: Mike Rapoport <[email protected]>

Since commit 974b9b2c68f3 ("mm: consolidate pte_index() and pte_offset_*() definitions") pte_index is a static inline and there is no define for it that can be recognized by the preprocessor. As the result, vm_insert_pages() uses slower loop over vm_insert_page() instead of insert_pages() that amortizes the cost of spinlock operations when inserting multiple pages.

Fixes 974b9b2c68f3 ("mm: consolidate pte_index() and pte_offset_*() definitions")
Reported-by Christian Dietrich <[email protected]>
Signed-off-by Mike Rapoport <[email protected]>
Cc [email protected]
---
 include/linux/pgtable.h | 1 +
 1 file changed, 1 insertion(+)
include/linux/pgtable.h +1 -0
index e24d2c992b11..d468efcf48f4 100644
@@ -62,6 +62,7 @@ static inline unsigned long pte_index(unsigned long address)
 {
 	return (address >> PAGE_SHIFT) & (PTRS_PER_PTE - 1);
 }
+#define pte_index pte_index
 
 #ifndef pmd_index
 static inline unsigned long pmd_index(unsigned long address)

base-commit: 2585cf9dfaaddf00b069673f27bb3f8530e2039c

Signature · 1 line
2.28.0
#1 lore raw

Hello Mike!

Mike Rapoport <[email protected]> [11. Januar 2022]:

include/linux/pgtable.h static inline unsigned long pte_index(unsigned long address) · line 65

Quoted 9 lines
include/linux/pgtable.h +1 -0
index e24d2c992b11..d468efcf48f4 100644
@@ -62,6 +62,7 @@ static inline unsigned long pte_index(unsigned long address)
 {
 	return (address >> PAGE_SHIFT) & (PTRS_PER_PTE - 1);
 }
+#define pte_index pte_index

Wouldn't it make sense to remove the dead CPP blocks (#ifdef pte_index) from mm/memory.c? Or is there a case were pte_index is not defined for an architecture?

chris

Signature · 9 lines
Prof. Dr.-Ing. Christian Dietrich
Operating System Group (E-EXK4)
Technische Universität Hamburg
Am Schwarzenberg-Campus 3 (E), 4.092
21073 Hamburg

eMail:  [email protected]
Tel:    +49 40 42878 2188
WWW:    https://osg.tuhh.de/
author reviewer
#2 lore raw

Hi Christian,

Quoted 4 lines · Christian Dietrich

Hello Mike!

Mike Rapoport <[email protected]> [11. Januar 2022]:

Quoted 12 lines

include/linux/pgtable.h static inline unsigned long pte_index(unsigned long address) · line 65

Quoted 9 lines
include/linux/pgtable.h +1 -0
index e24d2c992b11..d468efcf48f4 100644
@@ -62,6 +62,7 @@ static inline unsigned long pte_index(unsigned long address)
 {
 	return (address >> PAGE_SHIFT) & (PTRS_PER_PTE - 1);
 }
+#define pte_index pte_index

Wouldn't it make sense to remove the dead CPP blocks (#ifdef pte_index) from mm/memory.c?

It does make sense to remove the dead code, but this cleanup does not need stable backporting so it'll be a separate patch.

Care to send a patch? ;-)

Quoted 1 line

Or is there a case were pte_index is not defined for an architecture?

Nope, the fix in include/linux/pgtable.h covers MMU architectures and NOMMU do not compile mm/memory.c anyway.

Quoted 2 lines

chris

Signature · 2 lines
Sincerely yours,
Mike.
Reviewed-by on v1 1/1
#3 lore raw
In reply to Mike Rapoport
Quoted 34 lines · Mike Rapoport

From: Mike Rapoport <[email protected]>

Since commit 974b9b2c68f3 ("mm: consolidate pte_index() and pte_offset_*()
definitions") pte_index is a static inline and there is no define for it
that can be recognized by the preprocessor. As the result, vm_insert_pages() uses slower loop over vm_insert_page() instead of insert_pages() that amortizes the cost of spinlock operations when inserting multiple pages.

Fixes 974b9b2c68f3 ("mm: consolidate pte_index() and pte_offset_*()

definitions")

Reported-by Christian Dietrich <[email protected]>
Signed-off-by Mike Rapoport <[email protected]>
Cc [email protected]

---
 include/linux/pgtable.h | 1 +

 1 file changed, 1 insertion(+)
include/linux/pgtable.h +1 -0
index e24d2c992b11..d468efcf48f4 100644
@@ -62,6 +62,7 @@ static inline unsigned long pte_index(unsigned long
 address)
  {
         return (address >> PAGE_SHIFT) & (PTRS_PER_PTE - 1);
  }
+#define pte_index pte_index


 #ifndef pmd_index
 static inline unsigned long pmd_index(unsigned long address)

Quoted 1 line

base-commit: 2585cf9dfaaddf00b069673f27bb3f8530e2039c

This is a good fix with positive performance impact.

Reviewed-by Khalid Aziz <[email protected]>