Thread (5 messages) 5 messages, 2 authors, 2021-04-15

Re: [PATCH] mm: Define ARCH_HAS_FIRST_USER_ADDRESS

From: Anshuman Khandual <hidden>
Date: 2021-04-15 04:40:01
Also in: linux-alpha, linux-arm-kernel, linux-m68k, linux-mips, linux-mm, linux-riscv, linux-s390, linux-sh, linux-um, lkml, sparclinux
Subsystem: arm port, memory management, memory management - core, the rest · Maintainers: Russell King, Andrew Morton, David Hildenbrand, Linus Torvalds

On 4/14/21 11:40 AM, Christophe Leroy wrote:

Le 14/04/2021 à 07:59, Anshuman Khandual a écrit :
quoted

On 4/14/21 10:52 AM, Christophe Leroy wrote:
quoted

Le 14/04/2021 à 04:54, Anshuman Khandual a écrit :
quoted
Currently most platforms define FIRST_USER_ADDRESS as 0UL duplicating the
same code all over. Instead define a new option ARCH_HAS_FIRST_USER_ADDRESS
for those platforms which would override generic default FIRST_USER_ADDRESS
value 0UL. This makes it much cleaner with reduced code.

Cc: linux-alpha@vger.kernel.org
Cc: linux-snps-arc@lists.infradead.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-csky@vger.kernel.org
Cc: linux-hexagon@vger.kernel.org
Cc: linux-ia64@vger.kernel.org
Cc: linux-m68k@lists.linux-m68k.org
Cc: linux-mips@vger.kernel.org
Cc: openrisc@lists.librecores.org
Cc: linux-parisc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-riscv@lists.infradead.org
Cc: linux-s390@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Cc: sparclinux@vger.kernel.org
Cc: linux-um@lists.infradead.org
Cc: linux-xtensa@linux-xtensa.org
Cc: x86@kernel.org
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <redacted>
---
   arch/alpha/include/asm/pgtable.h             | 1 -
   arch/arc/include/asm/pgtable.h               | 6 ------
   arch/arm/Kconfig                             | 1 +
   arch/arm64/include/asm/pgtable.h             | 2 --
   arch/csky/include/asm/pgtable.h              | 1 -
   arch/hexagon/include/asm/pgtable.h           | 3 ---
   arch/ia64/include/asm/pgtable.h              | 1 -
   arch/m68k/include/asm/pgtable_mm.h           | 1 -
   arch/microblaze/include/asm/pgtable.h        | 2 --
   arch/mips/include/asm/pgtable-32.h           | 1 -
   arch/mips/include/asm/pgtable-64.h           | 1 -
   arch/nds32/Kconfig                           | 1 +
   arch/nios2/include/asm/pgtable.h             | 2 --
   arch/openrisc/include/asm/pgtable.h          | 1 -
   arch/parisc/include/asm/pgtable.h            | 2 --
   arch/powerpc/include/asm/book3s/pgtable.h    | 1 -
   arch/powerpc/include/asm/nohash/32/pgtable.h | 1 -
   arch/powerpc/include/asm/nohash/64/pgtable.h | 2 --
   arch/riscv/include/asm/pgtable.h             | 2 --
   arch/s390/include/asm/pgtable.h              | 2 --
   arch/sh/include/asm/pgtable.h                | 2 --
   arch/sparc/include/asm/pgtable_32.h          | 1 -
   arch/sparc/include/asm/pgtable_64.h          | 3 ---
   arch/um/include/asm/pgtable-2level.h         | 1 -
   arch/um/include/asm/pgtable-3level.h         | 1 -
   arch/x86/include/asm/pgtable_types.h         | 2 --
   arch/xtensa/include/asm/pgtable.h            | 1 -
   include/linux/mm.h                           | 4 ++++
   mm/Kconfig                                   | 4 ++++
   29 files changed, 10 insertions(+), 43 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 8ba434287387..47098ccd715e 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -46,6 +46,10 @@ extern int sysctl_page_lock_unfairness;
     void init_mm_internals(void);
   +#ifndef ARCH_HAS_FIRST_USER_ADDRESS
I guess you didn't test it ..... :)
In fact I did :) Though just booted it on arm64 and cross compiled on
multiple others platforms.
I guess for all platforms, ARCH_HAS_FIRST_USER_ADDRESS would have just
evaluated to be false hence falling back on the generic definition. So
this never complained during build any where or during boot on arm64.
quoted
quoted
should be #ifndef CONFIG_ARCH_HAS_FIRST_USER_ADDRESS
Right, meant that instead.
quoted
quoted
+#define FIRST_USER_ADDRESS    0UL
+#endif
But why do we need a config option at all for that ?

Why not just:

#ifndef FIRST_USER_ADDRESS
#define FIRST_USER_ADDRESS    0UL
#endif
This sounds simpler. But just wondering, would not there be any possibility
of build problems due to compilation sequence between arch and generic code ?
For sure it has to be addresses carefully, but there are already a lot of stuff like that around pgtables.h

For instance, pte_offset_kernel() has a generic definition in linux/pgtables.h based on whether it is already defined or not.

Taking into account that FIRST_USER_ADDRESS is today in the architectures's asm/pgtables.h, I think putting the fallback definition in linux/pgtable.h would do the trick.
Agreed, <linux/pgtable.h> includes <asm/pgtable.h> at the beginning and
if the arch defines FIRST_USER_ADDRESS, the generic one afterwards would
be skipped. The following change builds on multiple platforms.
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index ad086e6d7155..5da96f5df48f 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -7,7 +7,6 @@ config ARM
 	select ARCH_HAS_DEBUG_VIRTUAL if MMU
 	select ARCH_HAS_DMA_WRITE_COMBINE if !ARM_DMA_MEM_BUFFERABLE
 	select ARCH_HAS_ELF_RANDOMIZE
-	select ARCH_HAS_FIRST_USER_ADDRESS
 	select ARCH_HAS_FORTIFY_SOURCE
 	select ARCH_HAS_KEEPINITRD
 	select ARCH_HAS_KCOV
diff --git a/arch/nds32/Kconfig b/arch/nds32/Kconfig
index 23ec4fcc0d0f..62313902d75d 100644
--- a/arch/nds32/Kconfig
+++ b/arch/nds32/Kconfig
@@ -8,7 +8,6 @@ config NDS32
 	def_bool y
 	select ARCH_32BIT_OFF_T
 	select ARCH_HAS_DMA_PREP_COHERENT
-	select ARCH_HAS_FIRST_USER_ADDRESS
 	select ARCH_HAS_SYNC_DMA_FOR_CPU
 	select ARCH_HAS_SYNC_DMA_FOR_DEVICE
 	select ARCH_WANT_FRAME_POINTERS if FTRACE
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 47098ccd715e..8ba434287387 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -46,10 +46,6 @@ extern int sysctl_page_lock_unfairness;
 
 void init_mm_internals(void);
 
-#ifndef ARCH_HAS_FIRST_USER_ADDRESS
-#define FIRST_USER_ADDRESS	0UL
-#endif
-
 #ifndef CONFIG_NEED_MULTIPLE_NODES	/* Don't use mapnrs, do it properly */
 extern unsigned long max_mapnr;
 
diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
index 5e772392a379..f3da6a5cc35a 100644
--- a/include/linux/pgtable.h
+++ b/include/linux/pgtable.h
@@ -28,6 +28,10 @@
 #define USER_PGTABLES_CEILING	0UL
 #endif
 
+#ifndef FIRST_USER_ADDRESS
+#define FIRST_USER_ADDRESS	0UL
+#endif
+
 /*
  * A page table page can be thought of an array like this: pXd_t[PTRS_PER_PxD]
  *
diff --git a/mm/Kconfig b/mm/Kconfig
index 373fbe377075..4494501aa403 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -807,9 +807,6 @@ config VMAP_PFN
 config ARCH_USES_HIGH_VMA_FLAGS
 	bool
 
-config ARCH_HAS_FIRST_USER_ADDRESS
-	bool
-
 config ARCH_HAS_PKEYS
 	bool
 
-- 
2.20.1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help