mm/highmem: Preemptible variant of kmap_atomic &#38; friends

Cc Peter Zijlstra <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Juri Lelli <[email protected]>

Cc Vincent Guittot <[email protected]>

Cc Dietmar Eggemann <[email protected]>

Cc Steven Rostedt <[email protected]>

Cc Ben Segall <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Daniel Bristot de Oliveira <[email protected]>

---
 include/linux/preempt.h |   38 +++-----------------------------------
 include/linux/sched.h   |    2 +-
 kernel/sched/core.c     |   12 ++----------
 kernel/sched/sched.h    |    2 +-
 lib/smp_processor_id.c  |    2 +-
 5 files changed, 8 insertions(+), 48 deletions(-)

include/linux/preempt.h +3 ~~-35~~

@@ -322,7 +322,7 @@ static inline void preempt_notifier_init
 
 #endif
 
-#if defined(CONFIG_SMP) && defined(CONFIG_PREEMPT_RT)
+#ifdef CONFIG_SMP
 
 /*
  * Migrate-Disable and why it is undesired.

@@ -382,43 +382,11 @@ static inline void preempt_notifier_init
 extern void migrate_disable(void);
 extern void migrate_enable(void);
 
-#elif defined(CONFIG_PREEMPT_RT)
+#else
 
 static inline void migrate_disable(void) { }
 static inline void migrate_enable(void) { }
 
-#else /* !CONFIG_PREEMPT_RT */
-
-/**
- * migrate_disable - Prevent migration of the current task
- *
- * Maps to preempt_disable() which also disables preemption. Use
- * migrate_disable() to annotate that the intent is to prevent migration,
- * but not necessarily preemption.
- *
- * Can be invoked nested like preempt_disable() and needs the corresponding
- * number of migrate_enable() invocations.
- */
-static __always_inline void migrate_disable(void)
-{
-	preempt_disable();
-}
-
-/**
- * migrate_enable - Allow migration of the current task
- *
- * Counterpart to migrate_disable().
- *
- * As migrate_disable() can be invoked nested, only the outermost invocation
- * reenables migration.
- *
- * Currently mapped to preempt_enable().
- */
-static __always_inline void migrate_enable(void)
-{
-	preempt_enable();
-}
-
-#endif /* CONFIG_SMP && CONFIG_PREEMPT_RT */
+#endif /* CONFIG_SMP */
 
 #endif /* __LINUX_PREEMPT_H */

include/linux/sched.h +1 -1

@@ -715,7 +715,7 @@ struct task_struct {
 	const cpumask_t			*cpus_ptr;
 	cpumask_t			cpus_mask;
 	void				*migration_pending;
-#if defined(CONFIG_SMP) && defined(CONFIG_PREEMPT_RT)
+#ifdef CONFIG_SMP
 	unsigned short			migration_disabled;
 #endif
 	unsigned short			migration_flags;

kernel/sched/core.c +2 ~~-10~~

@@ -1696,8 +1696,6 @@ void check_preempt_curr(struct rq *rq, s
 
 #ifdef CONFIG_SMP
 
-#ifdef CONFIG_PREEMPT_RT
-
 static void
 __do_set_cpus_allowed(struct task_struct *p, const struct cpumask *new_mask, u32 flags);

@@ -1772,8 +1770,6 @@ static inline bool rq_has_pinned_tasks(s
 	return rq->nr_pinned;
 }
 
-#endif
-
 /*
  * Per-CPU kthreads are allowed to run on !active && online CPUs, see
  * __set_cpus_allowed_ptr() and select_fallback_rq().

@@ -2841,7 +2837,7 @@ void sched_set_stop_task(int cpu, struct
 	}
 }
 
-#else
+#else /* CONFIG_SMP */
 
 static inline int __set_cpus_allowed_ptr(struct task_struct *p,
 					 const struct cpumask *new_mask,

@@ -2850,10 +2846,6 @@ static inline int __set_cpus_allowed_ptr
 	return set_cpus_allowed_ptr(p, new_mask);
 }
 
-#endif /* CONFIG_SMP */
-
-#if !defined(CONFIG_SMP) || !defined(CONFIG_PREEMPT_RT)
-
 static inline void migrate_disable_switch(struct rq *rq, struct task_struct *p) { }
 
 static inline bool rq_has_pinned_tasks(struct rq *rq)

@@ -2861,7 +2853,7 @@ static inline bool rq_has_pinned_tasks(s
 	return false;
 }
 
-#endif
+#endif /* !CONFIG_SMP */
 
 static void
 ttwu_stat(struct task_struct *p, int cpu, int wake_flags)

kernel/sched/sched.h +1 -1

@@ -1056,7 +1056,7 @@ struct rq {
 	struct cpuidle_state	*idle_state;
 #endif
 
-#if defined(CONFIG_PREEMPT_RT) && defined(CONFIG_SMP)
+#if CONFIG_SMP
 	unsigned int		nr_pinned;
 #endif
 	unsigned int		push_busy;

lib/smp_processor_id.c +1 -1

@@ -26,7 +26,7 @@ unsigned int check_preemption_disabled(c
 	if (current->nr_cpus_allowed = 1)
 		goto out;
 
-#if defined(CONFIG_SMP) && defined(CONFIG_PREEMPT_RT)
+#ifdef CONFIG_SMP
 	if (current->migration_disabled)
 		goto out;
 #endif

Thomas Gleixner

author

2020-10-29 22:18 UTC #2 lore raw

PATCH v2 2/18 mm/highmem: Un-EXPORT __kmap_atomic_idx()

Nothing in modules can use that.

Reviewed-by Christoph Hellwig <[email protected]>

Cc Andrew Morton <[email protected]>

---
 mm/highmem.c |    2 --
 1 file changed, 2 deletions(-)

mm/highmem.c +0 -2

@@ -108,8 +108,6 @@ static inline wait_queue_head_t *get_pkm
 atomic_long_t _totalhigh_pages __read_mostly;
 EXPORT_SYMBOL(_totalhigh_pages);
 
-EXPORT_PER_CPU_SYMBOL(__kmap_atomic_idx);
-
 unsigned int nr_free_highpages (void)
 {
 	struct zone *zone;

Thomas Gleixner

author

2020-10-29 22:18 UTC #3 lore raw

PATCH v2 3/18 highmem: Provide generic variant of kmap_atomic*

The kmap_atomic* interfaces in all architectures are pretty much the same except for post map operations (flush) and pre- and post unmap operations.

Provide a generic variant for that.

Cc Andrew Morton <[email protected]>

---
V2: Address review comments from Christoph (style and EXPORT variant)

---
 include/linux/highmem.h |   79 ++++++++++++++++++++++++++------
 mm/Kconfig              |    3 +
 mm/highmem.c            |  118 +++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 183 insertions(+), 17 deletions(-)

include/linux/highmem.h +64 ~~-15~~

@@ -31,9 +31,16 @@ static inline void invalidate_kernel_vma
 
 #include <asm/kmap_types.h>
 
+/*
+ * Outside of CONFIG_HIGHMEM to support X86 32bit iomap_atomic() cruft.
+ */
+#ifdef CONFIG_KMAP_LOCAL
+void *__kmap_local_pfn_prot(unsigned long pfn, pgprot_t prot);
+void *__kmap_local_page_prot(struct page *page, pgprot_t prot);
+void kunmap_local_indexed(void *vaddr);
+#endif
+
 #ifdef CONFIG_HIGHMEM
-extern void *kmap_atomic_high_prot(struct page *page, pgprot_t prot);
-extern void kunmap_atomic_high(void *kvaddr);
 #include <asm/highmem.h>
 
 #ifndef ARCH_HAS_KMAP_FLUSH_TLB

@@ -81,6 +88,11 @@ static inline void kunmap(struct page *p
  * be used in IRQ contexts, so in some (very limited) cases we need
  * it.
  */
+
+#ifndef CONFIG_KMAP_LOCAL
+void *kmap_atomic_high_prot(struct page *page, pgprot_t prot);
+void kunmap_atomic_high(void *kvaddr);
+
 static inline void *kmap_atomic_prot(struct page *page, pgprot_t prot)
 {
 	preempt_disable();

@@ -89,7 +101,38 @@ static inline void *kmap_atomic_prot(str
 		return page_address(page);
 	return kmap_atomic_high_prot(page, prot);
 }
-#define kmap_atomic(page)	kmap_atomic_prot(page, kmap_prot)
+
+static inline void __kunmap_atomic(void *vaddr)
+{
+	kunmap_atomic_high(vaddr);
+}
+#else /* !CONFIG_KMAP_LOCAL */
+
+static inline void *kmap_atomic_prot(struct page *page, pgprot_t prot)
+{
+	preempt_disable();
+	pagefault_disable();
+	return __kmap_local_page_prot(page, prot);
+}
+
+static inline void *kmap_atomic_pfn(unsigned long pfn)
+{
+	preempt_disable();
+	pagefault_disable();
+	return __kmap_local_pfn_prot(pfn, kmap_prot);
+}
+
+static inline void __kunmap_atomic(void *addr)
+{
+	kunmap_local_indexed(addr);
+}
+
+#endif /* CONFIG_KMAP_LOCAL */
+
+static inline void *kmap_atomic(struct page *page)
+{
+	return kmap_atomic_prot(page, kmap_prot);
+}
 
 /* declarations for linux/mm/highmem.c */
 unsigned int nr_free_highpages(void);

@@ -157,21 +200,28 @@ static inline void *kmap_atomic(struct p
 	pagefault_disable();
 	return page_address(page);
 }
-#define kmap_atomic_prot(page, prot)	kmap_atomic(page)
 
-static inline void kunmap_atomic_high(void *addr)
+static inline void *kmap_atomic_prot(struct page *page, pgprot_t prot)
+{
+	return kmap_atomic(page);
+}
+
+static inline void *kmap_atomic_pfn(unsigned long pfn)
+{
+	return kmap_atomic(pfn_to_page(pfn));
+}
+
+static inline void __kunmap_atomic(void *addr)
 {
 	/*
 	 * Mostly nothing to do in the CONFIG_HIGHMEM=n case as kunmap_atomic()
-	 * handles re-enabling faults + preemption
+	 * handles re-enabling faults and preemption
 	 */
 #ifdef ARCH_HAS_FLUSH_ON_KUNMAP
 	kunmap_flush_on_unmap(addr);
 #endif
 }
 
-#define kmap_atomic_pfn(pfn)	kmap_atomic(pfn_to_page(pfn))
-
 #define kmap_flush_unused()	do {} while(0)
 
 #endif /* CONFIG_HIGHMEM */

@@ -213,15 +263,14 @@ static inline void kmap_atomic_idx_pop(v
  * Prevent people trying to call kunmap_atomic() as if it were kunmap()
  * kunmap_atomic() should get the return value of kmap_atomic, not the page.
  */
-#define kunmap_atomic(addr)                                     \
-do {                                                            \
-	BUILD_BUG_ON(__same_type((addr), struct page *));       \
-	kunmap_atomic_high(addr);                                  \
-	pagefault_enable();                                     \
-	preempt_enable();                                       \
+#define kunmap_atomic(__addr)					\
+do {								\
+	BUILD_BUG_ON(__same_type((__addr), struct page *));	\
+	__kunmap_atomic(__addr);				\
+	pagefault_enable();					\
+	preempt_enable();					\
 } while (0)
 
-
 /* when CONFIG_HIGHMEM is not set these will be plain clear/copy_page */
 #ifndef clear_user_highpage
 static inline void clear_user_highpage(struct page *page, unsigned long vaddr)

mm/Kconfig +3 -0

@@ -872,4 +872,7 @@ config ARCH_HAS_HUGEPD
 config MAPPING_DIRTY_HELPERS
         bool
 
+config KMAP_LOCAL
+	bool
+
 endmenu

mm/highmem.c +116 -2

131 lines not on this page · raw message

[email protected]

2020-10-29 23:51 UTC #4 lore raw

Test Results: RE: [V2,03/18] highmem: Provide generic variant of kmap_atomic*

Thanks for your contribution, unfortunately we've found some issues.

Your patch failed to apply to any branch.

Thomas Gleixner

author

2020-10-29 22:18 UTC #5 lore raw

PATCH v2 4/18 x86/mm/highmem: Use generic kmap atomic implementation

Convert X86 to the generic kmap atomic implementation and make the iomap_atomic() naming convention consistent while at it.

---
 arch/x86/Kconfig               |    3 +-
 arch/x86/include/asm/fixmap.h  |    1 
 arch/x86/include/asm/highmem.h |   12 ++++++--
 arch/x86/include/asm/iomap.h   |   18 ++++++------
 arch/x86/mm/highmem_32.c       |   59 -----------------------------------------
 arch/x86/mm/init_32.c          |   15 ----------
 arch/x86/mm/iomap_32.c         |   59 +++--------------------------------------
 include/linux/io-mapping.h     |    2 -
 8 files changed, 27 insertions(+), 142 deletions(-)

arch/x86/Kconfig +2 -1

@@ -14,10 +14,11 @@ config X86_32
 	select ARCH_WANT_IPC_PARSE_VERSION
 	select CLKSRC_I8253
 	select CLONE_BACKWARDS
+	select GENERIC_VDSO_32
 	select HAVE_DEBUG_STACKOVERFLOW
+	select KMAP_LOCAL
 	select MODULES_USE_ELF_REL
 	select OLD_SIGACTION
-	select GENERIC_VDSO_32
 
 config X86_64
 	def_bool y

arch/x86/include/asm/fixmap.h +0 -1

@@ -151,7 +151,6 @@ extern void reserve_top_address(unsigned
 
 extern int fixmaps_set;
 
-extern pte_t *kmap_pte;
 extern pte_t *pkmap_page_table;
 
 void __native_set_fixmap(enum fixed_addresses idx, pte_t pte);

arch/x86/include/asm/highmem.h +9 -3

@@ -58,11 +58,17 @@ extern unsigned long highstart_pfn, high
 #define PKMAP_NR(virt)  ((virt-PKMAP_BASE) >> PAGE_SHIFT)
 #define PKMAP_ADDR(nr)  (PKMAP_BASE + ((nr) << PAGE_SHIFT))
 
-void *kmap_atomic_pfn(unsigned long pfn);
-void *kmap_atomic_prot_pfn(unsigned long pfn, pgprot_t prot);
-
 #define flush_cache_kmaps()	do { } while (0)
 
+#define	arch_kmap_local_post_map(vaddr, pteval)		\
+	arch_flush_lazy_mmu_mode()
+
+#define	arch_kmap_local_post_unmap(vaddr)		\
+	do {						\
+		flush_tlb_one_kernel((vaddr));		\
+		arch_flush_lazy_mmu_mode();		\
+	} while (0)
+
 extern void add_highpages_with_active_regions(int nid, unsigned long start_pfn,
 					unsigned long end_pfn);

arch/x86/include/asm/iomap.h +10 -8

@@ -9,19 +9,21 @@
 #include <linux/fs.h>
 #include <linux/mm.h>
 #include <linux/uaccess.h>
+#include <linux/highmem.h>
 #include <asm/cacheflush.h>
 #include <asm/tlbflush.h>
 
-void __iomem *
-iomap_atomic_prot_pfn(unsigned long pfn, pgprot_t prot);
+void __iomem *iomap_atomic_pfn_prot(unsigned long pfn, pgprot_t prot);
 
-void
-iounmap_atomic(void __iomem *kvaddr);
+static inline void iounmap_atomic(void __iomem *vaddr)
+{
+	kunmap_local_indexed((void __force *)vaddr);
+	pagefault_enable();
+	preempt_enable();
+}
 
-int
-iomap_create_wc(resource_size_t base, unsigned long size, pgprot_t *prot);
+int iomap_create_wc(resource_size_t base, unsigned long size, pgprot_t *prot);
 
-void
-iomap_free(resource_size_t base, unsigned long size);
+void iomap_free(resource_size_t base, unsigned long size);
 
 #endif /* _ASM_X86_IOMAP_H */

arch/x86/mm/highmem_32.c +0 ~~-59~~

65 lines not on this page · raw message

arch/x86/mm/init_32.c +0 ~~-15~~

@@ -394,19 +394,6 @@ kernel_physical_mapping_init(unsigned lo
 	return last_map_addr;
 }
 
-pte_t *kmap_pte;
-
-static void __init kmap_init(void)
-{
-	unsigned long kmap_vstart;
-
-	/*
-	 * Cache the first kmap pte:
-	 */
-	kmap_vstart = __fix_to_virt(FIX_KMAP_BEGIN);
-	kmap_pte = virt_to_kpte(kmap_vstart);
-}
-
 #ifdef CONFIG_HIGHMEM
 static void __init permanent_kmaps_init(pgd_t *pgd_base)
 {

@@ -712,8 +699,6 @@ void __init paging_init(void)
 
 	__flush_tlb_all();
 
-	kmap_init();
-
 	/*
 	 * NOTE: at this point the bootmem allocator is fully available.
 	 */

arch/x86/mm/iomap_32.c +5 ~~-54~~

69 lines not on this page · raw message

include/linux/io-mapping.h +1 -1

@@ -69,7 +69,7 @@ io_mapping_map_atomic_wc(struct io_mappi
 
 	BUG_ON(offset >= mapping->size);
 	phys_addr = mapping->base + offset;
-	return iomap_atomic_prot_pfn(PHYS_PFN(phys_addr), mapping->prot);
+	return iomap_atomic_pfn_prot(PHYS_PFN(phys_addr), mapping->prot);
 }
 
 static inline void

Thomas Gleixner

author

2020-10-29 22:18 UTC #6 lore raw

PATCH v2 5/18 arc/mm/highmem: Use generic kmap atomic implementation

Adopt the map ordering to match the other architectures and the generic code.

Cc Vineet Gupta <[email protected]>

---
 arch/arc/Kconfig               |    1 
 arch/arc/include/asm/highmem.h |    8 ++++++-
 arch/arc/mm/highmem.c          |   44 -----------------------------------------
 3 files changed, 9 insertions(+), 44 deletions(-)

arch/arc/Kconfig +1 -0

@@ -507,6 +507,7 @@ config LINUX_RAM_BASE
 config HIGHMEM
 	bool "High Memory Support"
 	select ARCH_DISCONTIGMEM_ENABLE
+	select KMAP_LOCAL
 	help
 	  With ARC 2G:2G address split, only upper 2G is directly addressable by
 	  kernel. Enable this to potentially allow access to rest of 2G and PAE

arch/arc/include/asm/highmem.h +7 -1

@@ -15,7 +15,10 @@
 #define FIXMAP_BASE		(PAGE_OFFSET - FIXMAP_SIZE - PKMAP_SIZE)
 #define FIXMAP_SIZE		PGDIR_SIZE	/* only 1 PGD worth */
 #define KM_TYPE_NR		((FIXMAP_SIZE >> PAGE_SHIFT)/NR_CPUS)
-#define FIXMAP_ADDR(nr)		(FIXMAP_BASE + ((nr) << PAGE_SHIFT))
+
+#define FIX_KMAP_BEGIN		(0)
+#define FIX_KMAP_END		((FIXMAP_SIZE >> PAGE_SHIFT) - 1)
+#define FIXADDR_TOP		(FIXMAP_BASE + FIXMAP_SIZE - PAGE_SIZE)
 
 /* start after fixmap area */
 #define PKMAP_BASE		(FIXMAP_BASE + FIXMAP_SIZE)

@@ -29,6 +32,9 @@
 
 extern void kmap_init(void);
 
+#define arch_kmap_local_post_unmap(vaddr)			\
+	local_flush_tlb_kernel_range(vaddr, vaddr + PAGE_SIZE)
+
 static inline void flush_cache_kmaps(void)
 {
 	flush_cache_all();

arch/arc/mm/highmem.c +1 ~~-43~~

54 lines not on this page · raw message

[email protected]

2020-10-29 23:49 UTC #7 lore raw

Test Results: RE: [V2, 05/18] arc/mm/highmem: Use generic kmap atomic implementation

Thanks for your contribution, unfortunately we've found some issues.

Your patch failed to apply to any branch.

Thomas Gleixner

author

2020-10-29 22:18 UTC #8 lore raw

PATCH v2 6/18 ARM: highmem: Switch to generic kmap atomic

No reason having the same code in every architecture.

Cc Russell King <[email protected]>

Cc Arnd Bergmann <[email protected]>

arch/arm/Kconfig +1 -0

index e00d94b16658..410235e350cc 100644
@@ -1499,6 +1499,7 @@ config HAVE_ARCH_PFN_VALID
 config HIGHMEM
 	bool "High Memory Support"
 	depends on MMU
+	select KMAP_LOCAL
 	help
 	  The address space of ARM processors is only 4 Gigabytes large
 	  and it has to accommodate user address space, kernel address

arch/arm/include/asm/highmem.h +22 -9

index 31811be38d78..99a99862c474 100644
@@ -46,19 +46,32 @@ extern pte_t *pkmap_page_table;
 
 #ifdef ARCH_NEEDS_KMAP_HIGH_GET
 extern void *kmap_high_get(struct page *page);
-#else
+
+static inline void *arch_kmap_local_high_get(struct page *page)
+{
+	if (IS_ENABLED(CONFIG_DEBUG_HIGHMEM) && !cache_is_vivt())
+		return NULL;
+	return kmap_high_get(page);
+}
+#define arch_kmap_local_high_get arch_kmap_local_high_get
+
+#else /* ARCH_NEEDS_KMAP_HIGH_GET */
 static inline void *kmap_high_get(struct page *page)
 {
 	return NULL;
 }
-#endif
+#endif /* !ARCH_NEEDS_KMAP_HIGH_GET */
 
-/*
- * The following functions are already defined by <linux/highmem.h>
- * when CONFIG_HIGHMEM is not set.
- */
-#ifdef CONFIG_HIGHMEM
-extern void *kmap_atomic_pfn(unsigned long pfn);
-#endif
+#define arch_kmap_local_post_map(vaddr, pteval)				\
+	local_flush_tlb_kernel_page(vaddr)
+
+#define arch_kmap_local_pre_unmap(vaddr)				\
+do {									\
+	if (cache_is_vivt())						\
+		__cpuc_flush_dcache_area((void *)vaddr, PAGE_SIZE);	\
+} while (0)
+
+#define arch_kmap_local_post_unmap(vaddr)				\
+	local_flush_tlb_kernel_page(vaddr)
 
 #endif

arch/arm/mm/Makefile +0 -1

index 7cb1699fbfc4..c4ce477c5261 100644
@@ -19,7 +19,6 @@ obj-$(CONFIG_MODULES)		+= proc-syms.o
 obj-$(CONFIG_DEBUG_VIRTUAL)	+= physaddr.o
 
 obj-$(CONFIG_ALIGNMENT_TRAP)	+= alignment.o
-obj-$(CONFIG_HIGHMEM)		+= highmem.o
 obj-$(CONFIG_HUGETLB_PAGE)	+= hugetlbpage.o
 obj-$(CONFIG_ARM_PV_FIXUP)	+= pv-fixup-asm.o

arch/arm/mm/highmem.c deleted +0 ~~-121~~

121 lines not on this page · raw message

[email protected]

2020-10-29 23:45 UTC #9 lore raw

Test Results: RE: [V2,06/18] ARM: highmem: Switch to generic kmap atomic

Thanks for your contribution, unfortunately we've found some issues.

Your patch failed to apply to any branch.

Thomas Gleixner

author

2020-10-29 22:18 UTC #10 lore raw

PATCH v2 7/18 csky/mm/highmem: Switch to generic kmap atomic

No reason having the same code in every architecture.

Acked-by Guo Ren <[email protected]>

---
 arch/csky/Kconfig               |    1 
 arch/csky/include/asm/highmem.h |    4 +-
 arch/csky/mm/highmem.c          |   75 ----------------------------------------
 3 files changed, 5 insertions(+), 75 deletions(-)

arch/csky/Kconfig +1 -0

@@ -286,6 +286,7 @@ config NR_CPUS
 config HIGHMEM
 	bool "High Memory Support"
 	depends on !CPU_CK610
+	select KMAP_LOCAL
 	default y
 
 config FORCE_MAX_ZONEORDER

arch/csky/include/asm/highmem.h +3 -1

@@ -32,10 +32,12 @@ extern pte_t *pkmap_page_table;
 
 #define ARCH_HAS_KMAP_FLUSH_TLB
 extern void kmap_flush_tlb(unsigned long addr);
-extern void *kmap_atomic_pfn(unsigned long pfn);
 
 #define flush_cache_kmaps() do {} while (0)
 
+#define arch_kmap_local_post_map(vaddr, pteval)	kmap_flush_tlb(vaddr)
+#define arch_kmap_local_post_unmap(vaddr)	kmap_flush_tlb(vaddr)
+
 extern void kmap_init(void);
 
 #endif /* __KERNEL__ */

arch/csky/mm/highmem.c +1 ~~-74~~

90 lines not on this page · raw message

[email protected]

2020-10-29 23:48 UTC #11 lore raw

Test Results: RE: [V2, 07/18] csky/mm/highmem: Switch to generic kmap atomic

Thanks for your contribution, unfortunately we've found some issues.

Your patch failed to apply to any branch.

Thomas Gleixner

author

2020-10-29 22:18 UTC #12 lore raw

PATCH v2 8/18 microblaze/mm/highmem: Switch to generic kmap atomic

No reason having the same code in every architecture.

Cc Michal Simek <[email protected]>

arch/microblaze/Kconfig +1 -0

index d262ac0c8714..186a0526564c 100644
@@ -170,6 +170,7 @@ config XILINX_UNCACHED_SHADOW
 config HIGHMEM
 	bool "High memory support"
 	depends on MMU
+	select KMAP_LOCAL
 	help
 	  The address space of Microblaze processors is only 4 Gigabytes large
 	  and it has to accommodate user address space, kernel address

arch/microblaze/include/asm/highmem.h +5 -1

index 284ca8fb54c1..4418633fb163 100644
@@ -25,7 +25,6 @@
 #include <linux/uaccess.h>
 #include <asm/fixmap.h>
 
-extern pte_t *kmap_pte;
 extern pte_t *pkmap_page_table;
 
 /*

@@ -52,6 +51,11 @@ extern pte_t *pkmap_page_table;
 
 #define flush_cache_kmaps()	{ flush_icache(); flush_dcache(); }
 
+#define arch_kmap_local_post_map(vaddr, pteval)	\
+	local_flush_tlb_page(NULL, vaddr);
+#define arch_kmap_local_post_unmap(vaddr)	\
+	local_flush_tlb_page(NULL, vaddr);
+
 #endif /* __KERNEL__ */
 
 #endif /* _ASM_HIGHMEM_H */

arch/microblaze/mm/Makefile +0 -1

index 1b16875cea70..8ced71100047 100644
@@ -6,4 +6,3 @@
 obj-y := consistent.o init.o
 
 obj-$(CONFIG_MMU) += pgtable.o mmu_context.o fault.o
-obj-$(CONFIG_HIGHMEM) += highmem.o

arch/microblaze/mm/highmem.c deleted +0 ~~-78~~

78 lines not on this page · raw message

arch/microblaze/mm/init.c +0 -6

index 3344d4a1fe89..3f4e41787a4e 100644
@@ -49,17 +49,11 @@ unsigned long lowmem_size;
 EXPORT_SYMBOL(min_low_pfn);
 EXPORT_SYMBOL(max_low_pfn);
 
-#ifdef CONFIG_HIGHMEM
-pte_t *kmap_pte;
-EXPORT_SYMBOL(kmap_pte);
-
 static void __init highmem_init(void)
 {
 	pr_debug("%x\n", (u32)PKMAP_BASE);
 	map_page(PKMAP_BASE, 0, 0);	/* XXX gross */
 	pkmap_page_table = virt_to_kpte(PKMAP_BASE);
-
-	kmap_pte = virt_to_kpte(__fix_to_virt(FIX_KMAP_BEGIN));
 }
 
 static void highmem_setup(void)

[email protected]

2020-10-29 23:43 UTC #13 lore raw

Test Results: RE: [V2,08/18] microblaze/mm/highmem: Switch to generic kmap atomic

Thanks for your contribution, unfortunately we've found some issues.

Your patch failed to apply to any branch.

Thomas Gleixner

author

2020-10-29 22:18 UTC #14 lore raw

PATCH v2 9/18 mips/mm/highmem: Switch to generic kmap atomic

No reason having the same code in every architecture

Cc Thomas Bogendoerfer <[email protected]>

arch/mips/Kconfig +1 -0

index 8f328298f8cc..ed6b3de944a8 100644
@@ -2654,6 +2654,7 @@ config MIPS_CRC_SUPPORT
 config HIGHMEM
 	bool "High Memory Support"
 	depends on 32BIT && CPU_SUPPORTS_HIGHMEM && SYS_SUPPORTS_HIGHMEM && !CPU_MIPS32_3_5_EVA
+	select KMAP_LOCAL
 
 config CPU_SUPPORTS_HIGHMEM
 	bool

arch/mips/include/asm/highmem.h +2 -2

index f1f788b57166..cb2e0fb8483b 100644
@@ -48,11 +48,11 @@ extern pte_t *pkmap_page_table;
 
 #define ARCH_HAS_KMAP_FLUSH_TLB
 extern void kmap_flush_tlb(unsigned long addr);
-extern void *kmap_atomic_pfn(unsigned long pfn);
 
 #define flush_cache_kmaps()	BUG_ON(cpu_has_dc_aliases)
 
-extern void kmap_init(void);
+#define arch_kmap_local_post_map(vaddr, pteval)	local_flush_tlb_one(vaddr)
+#define arch_kmap_local_post_unmap(vaddr)	local_flush_tlb_one(vaddr)
 
 #endif /* __KERNEL__ */

arch/mips/mm/highmem.c +0 ~~-77~~

86 lines not on this page · raw message

arch/mips/mm/init.c +0 -3

index 6c7bbfe35ba3..e5de8e9c2ede 100644
@@ -402,9 +402,6 @@ void __init paging_init(void)
 
 	pagetable_init();
 
-#ifdef CONFIG_HIGHMEM
-	kmap_init();
-#endif
 #ifdef CONFIG_ZONE_DMA
 	max_zone_pfns[ZONE_DMA] = MAX_DMA_PFN;
 #endif

[email protected]

2020-10-29 23:41 UTC #15 lore raw

Test Results: RE: [V2, 09/18] mips/mm/highmem: Switch to generic kmap atomic

Thanks for your contribution, unfortunately we've found some issues.

Your patch failed to apply to any branch.

Thomas Gleixner

author

2020-10-29 22:18 UTC #16 lore raw

PATCH v2 10/18 nds32/mm/highmem: Switch to generic kmap atomic

The mapping code is odd and looks broken. See FIXME in the comment.

Cc Nick Hu <[email protected]>

Cc Greentime Hu <[email protected]>

Cc Vincent Chen <[email protected]>

arch/nds32/Kconfig.cpu +1 -0

index f88a12fdf0f3..c7add11ea36e 100644
@@ -157,6 +157,7 @@ config HW_SUPPORT_UNALIGNMENT_ACCESS
 config HIGHMEM
 	bool "High Memory Support"
 	depends on MMU && !CPU_CACHE_ALIASING
+        select KMAP_LOCAL
 	help
 	  The address space of Andes processors is only 4 Gigabytes large
 	  and it has to accommodate user address space, kernel address

arch/nds32/include/asm/highmem.h +16 -5

index fe986d0e6e3f..d844c282c090 100644
@@ -45,11 +45,22 @@ extern pte_t *pkmap_page_table;
 extern void kmap_init(void);
 
 /*
- * The following functions are already defined by <linux/highmem.h>
- * when CONFIG_HIGHMEM is not set.
+ * FIXME: The below looks broken vs. a kmap_atomic() in task context which
+ * is interupted and another kmap_atomic() happens in interrupt context.
+ * But what do I know about nds32. -- tglx
  */
-#ifdef CONFIG_HIGHMEM
-extern void *kmap_atomic_pfn(unsigned long pfn);
-#endif
+#define arch_kmap_local_post_map(vaddr, pteval)			\
+	do {							\
+		__nds32__tlbop_inv(vaddr);			\
+		__nds32__mtsr_dsb(vaddr, NDS32_SR_TLB_VPN);	\
+		__nds32__tlbop_rwr(pteval);			\
+		__nds32__isb();					\
+	} while (0)
+
+#define arch_kmap_local_pre_unmap(vaddr, pte)			\
+	do {							\
+		__nds32__tlbop_inv(vaddr);			\
+		__nds32__isb();					\
+	} while (0)
 
 #endif

arch/nds32/mm/Makefile +0 -1

index 897ecaf5cf54..14fb2e8eb036 100644
@@ -3,7 +3,6 @@ obj-y				:= extable.o tlb.o fault.o init.o mmap.o \
                                    mm-nds32.o cacheflush.o proc.o
 
 obj-$(CONFIG_ALIGNMENT_TRAP)	+= alignment.o
-obj-$(CONFIG_HIGHMEM)           += highmem.o
 
 ifdef CONFIG_FUNCTION_TRACER
 CFLAGS_REMOVE_proc.o     = $(CC_FLAGS_FTRACE)

arch/nds32/mm/highmem.c deleted +0 ~~-48~~

48 lines not on this page · raw message

[email protected]

2020-10-29 23:40 UTC #17 lore raw

Test Results: RE: [V2,10/18] nds32/mm/highmem: Switch to generic kmap atomic

Thanks for your contribution, unfortunately we've found some issues.

Your patch failed to apply to any branch.

Thomas Gleixner

author

2020-10-29 22:18 UTC #18 lore raw

PATCH v2 11/18 powerpc/mm/highmem: Switch to generic kmap atomic

No reason having the same code in every architecture

Cc Michael Ellerman <[email protected]>

Cc Benjamin Herrenschmidt <[email protected]>

Cc Paul Mackerras <[email protected]>

---
 arch/powerpc/Kconfig               |    1 
 arch/powerpc/include/asm/highmem.h |    6 ++-
 arch/powerpc/mm/Makefile           |    1 
 arch/powerpc/mm/highmem.c          |   67 -------------------------------------
 arch/powerpc/mm/mem.c              |    7 ---
 5 files changed, 6 insertions(+), 76 deletions(-)

arch/powerpc/Kconfig +1 -0

@@ -409,6 +409,7 @@ menu "Kernel options"
 config HIGHMEM
 	bool "High memory support"
 	depends on PPC32
+        select KMAP_LOCAL
 
 source "kernel/Kconfig.hz"

arch/powerpc/include/asm/highmem.h +5 -1

@@ -29,7 +29,6 @@
 #include <asm/page.h>
 #include <asm/fixmap.h>
 
-extern pte_t *kmap_pte;
 extern pte_t *pkmap_page_table;
 
 /*

@@ -60,6 +59,11 @@ extern pte_t *pkmap_page_table;
 
 #define flush_cache_kmaps()	flush_cache_all()
 
+#define arch_kmap_local_post_map(vaddr, pteval)	\
+	local_flush_tlb_page(NULL, vaddr)
+#define arch_kmap_local_post_unmap(vaddr)	\
+	local_flush_tlb_page(NULL, vaddr)
+
 #endif /* __KERNEL__ */
 
 #endif /* _ASM_HIGHMEM_H */

arch/powerpc/mm/Makefile +0 -1

@@ -16,7 +16,6 @@ obj-$(CONFIG_NEED_MULTIPLE_NODES) += num
 obj-$(CONFIG_PPC_MM_SLICES)	+= slice.o
 obj-$(CONFIG_HUGETLB_PAGE)	+= hugetlbpage.o
 obj-$(CONFIG_NOT_COHERENT_CACHE) += dma-noncoherent.o
-obj-$(CONFIG_HIGHMEM)		+= highmem.o
 obj-$(CONFIG_PPC_COPRO_BASE)	+= copro_fault.o
 obj-$(CONFIG_PPC_PTDUMP)	+= ptdump/
 obj-$(CONFIG_KASAN)		+= kasan/

arch/powerpc/mm/highmem.c deleted +0 ~~-67~~

67 lines not on this page · raw message

arch/powerpc/mm/mem.c +0 -7

@@ -61,11 +61,6 @@
 unsigned long long memory_limit;
 bool init_mem_is_free;
 
-#ifdef CONFIG_HIGHMEM
-pte_t *kmap_pte;
-EXPORT_SYMBOL(kmap_pte);
-#endif
-
 pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
 			      unsigned long size, pgprot_t vma_prot)
 {

@@ -235,8 +230,6 @@ void __init paging_init(void)
 
 	map_kernel_page(PKMAP_BASE, 0, __pgprot(0));	/* XXX gross */
 	pkmap_page_table = virt_to_kpte(PKMAP_BASE);
-
-	kmap_pte = virt_to_kpte(__fix_to_virt(FIX_KMAP_BEGIN));
 #endif /* CONFIG_HIGHMEM */
 
 	printk(KERN_DEBUG "Top of RAM: 0x%llx, Total RAM: 0x%llx\n",

[email protected]

2020-10-29 23:36 UTC #19 lore raw

Test Results: RE: [V2,11/18] powerpc/mm/highmem: Switch to generic kmap atomic

Thanks for your contribution, unfortunately we've found some issues.

Your patch failed to apply to any branch.

Thomas Gleixner

author

2020-10-29 22:18 UTC #20 lore raw

PATCH v2 12/18 sparc/mm/highmem: Switch to generic kmap atomic

No reason having the same code in every architecture

Cc "David S. Miller" <[email protected]>

---
 arch/sparc/Kconfig               |    1 
 arch/sparc/include/asm/highmem.h |    7 +-
 arch/sparc/mm/Makefile           |    3 -
 arch/sparc/mm/highmem.c          |  115 ---------------------------------------
 arch/sparc/mm/srmmu.c            |    2 
 5 files changed, 6 insertions(+), 122 deletions(-)

arch/sparc/Kconfig +1 -0

@@ -139,6 +139,7 @@ config MMU
 config HIGHMEM
 	bool
 	default y if SPARC32
+        select KMAP_LOCAL
 
 config ZONE_DMA
 	bool

arch/sparc/include/asm/highmem.h +5 -2

@@ -33,8 +33,6 @@ extern unsigned long highstart_pfn, high
 #define kmap_prot __pgprot(SRMMU_ET_PTE | SRMMU_PRIV | SRMMU_CACHE)
 extern pte_t *pkmap_page_table;
 
-void kmap_init(void) __init;
-
 /*
  * Right now we initialize only a single pte table. It can be extended
  * easily, subsequent pte tables have to be allocated in one physical

@@ -53,6 +51,11 @@ void kmap_init(void) __init;
 
 #define flush_cache_kmaps()	flush_cache_all()
 
+/* FIXME: Use __flush_tlb_one(vaddr) instead of flush_cache_all() -- Anton */
+#define arch_kmap_local_post_map(vaddr, pteval)	flush_cache_all()
+#define arch_kmap_local_post_unmap(vaddr)	flush_cache_all()
+
+
 #endif /* __KERNEL__ */
 
 #endif /* _ASM_HIGHMEM_H */

arch/sparc/mm/Makefile +0 -3

@@ -15,6 +15,3 @@ obj-$(CONFIG_SPARC32)   += leon_mm.o
 
 # Only used by sparc64
 obj-$(CONFIG_HUGETLB_PAGE) += hugetlbpage.o
-
-# Only used by sparc32
-obj-$(CONFIG_HIGHMEM)   += highmem.o

arch/sparc/mm/highmem.c deleted +0 ~~-115~~

115 lines not on this page · raw message

arch/sparc/mm/srmmu.c +0 -2

@@ -971,8 +971,6 @@ void __init srmmu_paging_init(void)
 
 	sparc_context_init(num_contexts);
 
-	kmap_init();
-
 	{
 		unsigned long max_zone_pfn[MAX_NR_ZONES] = { 0 };

[email protected]

2020-10-29 23:34 UTC #21 lore raw

Test Results: RE: [V2,12/18] sparc/mm/highmem: Switch to generic kmap atomic

Thanks for your contribution, unfortunately we've found some issues.

Your patch failed to apply to any branch.

Thomas Gleixner

author

2020-10-29 22:18 UTC #22 lore raw

PATCH v2 13/18 xtensa/mm/highmem: Switch to generic kmap atomic

No reason having the same code in every architecture

Cc Chris Zankel <[email protected]>

Cc Max Filippov <[email protected]>

---
 arch/xtensa/Kconfig               |    1 
 arch/xtensa/include/asm/highmem.h |    9 +++++++
 arch/xtensa/mm/highmem.c          |   44 +++-----------------------------------
 3 files changed, 14 insertions(+), 40 deletions(-)

arch/xtensa/Kconfig +1 -0

@@ -666,6 +666,7 @@ endchoice
 config HIGHMEM
 	bool "High Memory Support"
 	depends on MMU
+        select KMAP_LOCAL
 	help
 	  Linux can use the full amount of RAM in the system by
 	  default. However, the default MMUv2 setup only maps the

arch/xtensa/include/asm/highmem.h +9 -0

@@ -68,6 +68,15 @@ static inline void flush_cache_kmaps(voi
 	flush_cache_all();
 }
 
+enum fixed_addresses kmap_local_map_idx(int type, unsigned long pfn);
+#define arch_kmap_local_map_idx		kmap_local_map_idx
+
+enum fixed_addresses kmap_local_unmap_idx(int type, unsigned long addr);
+#define arch_kmap_local_unmap_idx	kmap_local_unmap_idx
+
+#define arch_kmap_local_post_unmap(vaddr)	\
+	local_flush_tlb_kernel_range(vaddr, vaddr + PAGE_SIZE)
+
 void kmap_init(void);
 
 #endif

arch/xtensa/mm/highmem.c +4 ~~-40~~

67 lines not on this page · raw message

[email protected]

2020-10-29 23:32 UTC #23 lore raw

Test Results: RE: [V2,13/18] xtensa/mm/highmem: Switch to generic kmap atomic

Thanks for your contribution, unfortunately we've found some issues.

Your patch failed to apply to any branch.

Thomas Gleixner

author

2020-10-29 22:18 UTC #24 lore raw

PATCH v2 14/18 mm/highmem: Remove the old kmap_atomic cruft

All users gone.

---
 include/linux/highmem.h |   61 ++----------------------------------------------
 mm/highmem.c            |   28 ++++++++++++++++++----
 2 files changed, 27 insertions(+), 62 deletions(-)

include/linux/highmem.h +3 ~~-58~~

@@ -88,31 +88,16 @@ static inline void kunmap(struct page *p
  * be used in IRQ contexts, so in some (very limited) cases we need
  * it.
  */
-
-#ifndef CONFIG_KMAP_LOCAL
-void *kmap_atomic_high_prot(struct page *page, pgprot_t prot);
-void kunmap_atomic_high(void *kvaddr);
-
 static inline void *kmap_atomic_prot(struct page *page, pgprot_t prot)
 {
 	preempt_disable();
 	pagefault_disable();
-	if (!PageHighMem(page))
-		return page_address(page);
-	return kmap_atomic_high_prot(page, prot);
-}
-
-static inline void __kunmap_atomic(void *vaddr)
-{
-	kunmap_atomic_high(vaddr);
+	return __kmap_local_page_prot(page, prot);
 }
-#else /* !CONFIG_KMAP_LOCAL */
 
-static inline void *kmap_atomic_prot(struct page *page, pgprot_t prot)
+static inline void *kmap_atomic(struct page *page)
 {
-	preempt_disable();
-	pagefault_disable();
-	return __kmap_local_page_prot(page, prot);
+	return kmap_atomic_prot(page, kmap_prot);
 }
 
 static inline void *kmap_atomic_pfn(unsigned long pfn)

@@ -127,13 +112,6 @@ static inline void __kunmap_atomic(void
 	kunmap_local_indexed(addr);
 }
 
-#endif /* CONFIG_KMAP_LOCAL */
-
-static inline void *kmap_atomic(struct page *page)
-{
-	return kmap_atomic_prot(page, kmap_prot);
-}
-
 /* declarations for linux/mm/highmem.c */
 unsigned int nr_free_highpages(void);
 extern atomic_long_t _totalhigh_pages;

@@ -226,39 +204,6 @@ static inline void __kunmap_atomic(void
 
 #endif /* CONFIG_HIGHMEM */
 
-#if defined(CONFIG_HIGHMEM) || defined(CONFIG_X86_32)
-
-DECLARE_PER_CPU(int, __kmap_atomic_idx);
-
-static inline int kmap_atomic_idx_push(void)
-{
-	int idx = __this_cpu_inc_return(__kmap_atomic_idx) - 1;
-
-#ifdef CONFIG_DEBUG_HIGHMEM
-	WARN_ON_ONCE(in_irq() && !irqs_disabled());
-	BUG_ON(idx >= KM_TYPE_NR);
-#endif
-	return idx;
-}
-
-static inline int kmap_atomic_idx(void)
-{
-	return __this_cpu_read(__kmap_atomic_idx) - 1;
-}
-
-static inline void kmap_atomic_idx_pop(void)
-{
-#ifdef CONFIG_DEBUG_HIGHMEM
-	int idx = __this_cpu_dec_return(__kmap_atomic_idx);
-
-	BUG_ON(idx < 0);
-#else
-	__this_cpu_dec(__kmap_atomic_idx);
-#endif
-}
-
-#endif
-
 /*
  * Prevent people trying to call kunmap_atomic() as if it were kunmap()
  * kunmap_atomic() should get the return value of kmap_atomic, not the page.

mm/highmem.c +24 -4

@@ -32,10 +32,6 @@
 #include <linux/vmalloc.h>
 #include <asm/fixmap.h>
 
-#if defined(CONFIG_HIGHMEM) || defined(CONFIG_X86_32)
-DEFINE_PER_CPU(int, __kmap_atomic_idx);
-#endif
-
 /*
  * Virtual_count is not a pure "count".
  *  0 means that it is not mapped, and has not been mapped

@@ -370,6 +366,30 @@ EXPORT_SYMBOL(kunmap_high);
 #endif /* CONFIG_HIGHMEM */
 
 #ifdef CONFIG_KMAP_LOCAL
+
+static DEFINE_PER_CPU(int, __kmap_atomic_idx);
+
+static inline int kmap_atomic_idx_push(void)
+{
+	int idx = __this_cpu_inc_return(__kmap_atomic_idx) - 1;
+
+	WARN_ON_ONCE(in_irq() && !irqs_disabled());
+	BUG_ON(idx >= KM_TYPE_NR);
+	return idx;
+}
+
+static inline int kmap_atomic_idx(void)
+{
+	return __this_cpu_read(__kmap_atomic_idx) - 1;
+}
+
+static inline void kmap_atomic_idx_pop(void)
+{
+	int idx = __this_cpu_dec_return(__kmap_atomic_idx);
+
+	BUG_ON(idx < 0);
+}
+
 #ifndef arch_kmap_local_post_map
 # define arch_kmap_local_post_map(vaddr, pteval)	do { } while (0)
 #endif

[email protected]

2020-10-29 23:29 UTC #25 lore raw

Test Results: RE: [V2,14/18] mm/highmem: Remove the old kmap_atomic cruft

Thanks for your contribution, unfortunately we've found some issues.

Your patch failed to apply to any branch.

Thomas Gleixner

author

2020-10-29 22:18 UTC #26 lore raw

PATCH v2 15/18 io-mapping: Cleanup atomic iomap

Switch the atomic iomap implementation over to kmap_local and stick the preempt/pagefault mechanics into the generic code similar to the kmap_atomic variants.

Rename the x86 map function in preparation for a non-atomic variant.

---
V2: New patch to make review easier

---
 arch/x86/include/asm/iomap.h |    9 +--------
 arch/x86/mm/iomap_32.c       |    6 ++----
 include/linux/io-mapping.h   |    8 ++++++--
 3 files changed, 9 insertions(+), 14 deletions(-)

arch/x86/include/asm/iomap.h +1 -8

@@ -13,14 +13,7 @@
 #include <asm/cacheflush.h>
 #include <asm/tlbflush.h>
 
-void __iomem *iomap_atomic_pfn_prot(unsigned long pfn, pgprot_t prot);
-
-static inline void iounmap_atomic(void __iomem *vaddr)
-{
-	kunmap_local_indexed((void __force *)vaddr);
-	pagefault_enable();
-	preempt_enable();
-}
+void __iomem *__iomap_local_pfn_prot(unsigned long pfn, pgprot_t prot);
 
 int iomap_create_wc(resource_size_t base, unsigned long size, pgprot_t *prot);

arch/x86/mm/iomap_32.c +2 -4

@@ -44,7 +44,7 @@ void iomap_free(resource_size_t base, un
 }
 EXPORT_SYMBOL_GPL(iomap_free);
 
-void __iomem *iomap_atomic_pfn_prot(unsigned long pfn, pgprot_t prot)
+void __iomem *__iomap_local_pfn_prot(unsigned long pfn, pgprot_t prot)
 {
 	/*
 	 * For non-PAT systems, translate non-WB request to UC- just in

@@ -60,8 +60,6 @@ void __iomem *iomap_atomic_pfn_prot(unsi
 	/* Filter out unsupported __PAGE_KERNEL* bits: */
 	pgprot_val(prot) &= __default_kernel_pte_mask;
 
-	preempt_disable();
-	pagefault_disable();
 	return (void __force __iomem *)__kmap_local_pfn_prot(pfn, prot);
 }
-EXPORT_SYMBOL_GPL(iomap_atomic_pfn_prot);
+EXPORT_SYMBOL_GPL(__iomap_local_pfn_prot);

include/linux/io-mapping.h +6 -2

@@ -69,13 +69,17 @@ io_mapping_map_atomic_wc(struct io_mappi
 
 	BUG_ON(offset >= mapping->size);
 	phys_addr = mapping->base + offset;
-	return iomap_atomic_pfn_prot(PHYS_PFN(phys_addr), mapping->prot);
+	preempt_disable();
+	pagefault_disable();
+	return __iomap_local_pfn_prot(PHYS_PFN(phys_addr), mapping->prot);
 }
 
 static inline void
 io_mapping_unmap_atomic(void __iomem *vaddr)
 {
-	iounmap_atomic(vaddr);
+	kunmap_local_indexed((void __force *)vaddr);
+	pagefault_enable();
+	preempt_enable();
 }
 
 static inline void __iomem *

[email protected]

2020-10-29 23:27 UTC #27 lore raw

Test Results: RE: [V2,15/18] io-mapping: Cleanup atomic iomap

Thanks for your contribution, unfortunately we've found some issues.

Your patch failed to apply to any branch.

Thomas Gleixner

author

2020-10-29 22:18 UTC #28 lore raw

PATCH v2 16/18 sched: highmem: Store local kmaps in task struct

Instead of storing the map per CPU provide and use per task storage. That prepares for local kmaps which are preemptible.

The context switch code is preparatory and not yet in use because kmap_atomic() runs with preemption disabled. Will be made usable in the next step.

The context switch logic is safe even when an interrupt happens after clearing or before restoring the kmaps. The kmap index in task struct is not modified so any nesting kmap in an interrupt will use unused indices and on return the counter is the same as before.

Also add an assert into the return to user space code. Going back to user space with an active kmap local is a nono.

---
 include/linux/highmem.h |   10 +++++
 include/linux/sched.h   |    9 ++++
 kernel/entry/common.c   |    2 +
 kernel/fork.c           |    1 
 kernel/sched/core.c     |   18 +++++++++
 mm/highmem.c            |   96 +++++++++++++++++++++++++++++++++++++++++-------
 6 files changed, 123 insertions(+), 13 deletions(-)

include/linux/highmem.h +10 -0

@@ -38,6 +38,16 @@ static inline void invalidate_kernel_vma
 void *__kmap_local_pfn_prot(unsigned long pfn, pgprot_t prot);
 void *__kmap_local_page_prot(struct page *page, pgprot_t prot);
 void kunmap_local_indexed(void *vaddr);
+void kmap_local_fork(struct task_struct *tsk);
+void __kmap_local_sched_out(void);
+void __kmap_local_sched_in(void);
+static inline void kmap_assert_nomap(void)
+{
+	DEBUG_LOCKS_WARN_ON(current->kmap_ctrl.idx);
+}
+#else
+static inline void kmap_local_fork(struct task_struct *tsk) { }
+static inline void kmap_assert_nomap(void) { }
 #endif
 
 #ifdef CONFIG_HIGHMEM

include/linux/sched.h +9 -0

@@ -34,6 +34,7 @@
 #include <linux/rseq.h>
 #include <linux/seqlock.h>
 #include <linux/kcsan.h>
+#include <asm/kmap_types.h>
 
 /* task_struct member predeclarations (sorted alphabetically): */
 struct audit_context;

@@ -629,6 +630,13 @@ struct wake_q_node {
 	struct wake_q_node *next;
 };
 
+struct kmap_ctrl {
+#ifdef CONFIG_KMAP_LOCAL
+	int				idx;
+	pte_t				pteval[KM_TYPE_NR];
+#endif
+};
+
 struct task_struct {
 #ifdef CONFIG_THREAD_INFO_IN_TASK
 	/*

@@ -1294,6 +1302,7 @@ struct task_struct {
 	unsigned int			sequential_io;
 	unsigned int			sequential_io_avg;
 #endif
+	struct kmap_ctrl		kmap_ctrl;
 #ifdef CONFIG_DEBUG_ATOMIC_SLEEP
 	unsigned long			task_state_change;
 #endif

kernel/entry/common.c +2 -0

@@ -2,6 +2,7 @@
 
 #include <linux/context_tracking.h>
 #include <linux/entry-common.h>
+#include <linux/highmem.h>
 #include <linux/livepatch.h>
 #include <linux/audit.h>

@@ -194,6 +195,7 @@ static void exit_to_user_mode_prepare(st
 
 	/* Ensure that the address limit is intact and no locks are held */
 	addr_limit_user_check();
+	kmap_assert_nomap();
 	lockdep_assert_irqs_disabled();
 	lockdep_sys_exit();
 }

kernel/fork.c +1 -0

@@ -930,6 +930,7 @@ static struct task_struct *dup_task_stru
 	account_kernel_stack(tsk, 1);
 
 	kcov_task_init(tsk);
+	kmap_local_fork(tsk);
 
 #ifdef CONFIG_FAULT_INJECTION
 	tsk->fail_nth = 0;

kernel/sched/core.c +18 -0

@@ -4053,6 +4053,22 @@ static inline void finish_lock_switch(st
 # define finish_arch_post_lock_switch()	do { } while (0)
 #endif
 
+static inline void kmap_local_sched_out(void)
+{
+#ifdef CONFIG_KMAP_LOCAL
+	if (unlikely(current->kmap_ctrl.idx))
+		__kmap_local_sched_out();
+#endif
+}
+
+static inline void kmap_local_sched_in(void)
+{
+#ifdef CONFIG_KMAP_LOCAL
+	if (unlikely(current->kmap_ctrl.idx))
+		__kmap_local_sched_in();
+#endif
+}
+
 /**
  * prepare_task_switch - prepare to switch tasks
  * @rq: the runqueue preparing to switch

@@ -4075,6 +4091,7 @@ prepare_task_switch(struct rq *rq, struc
 	perf_event_task_sched_out(prev, next);
 	rseq_preempt(prev);
 	fire_sched_out_preempt_notifiers(prev, next);
+	kmap_local_sched_out();
 	prepare_task(next);
 	prepare_arch_switch(next);
 }

@@ -4141,6 +4158,7 @@ static struct rq *finish_task_switch(str
 	finish_lock_switch(rq);
 	finish_arch_post_lock_switch();
 	kcov_finish_switch(current);
+	kmap_local_sched_in();
 
 	fire_sched_in_preempt_notifiers(current);
 	/*

mm/highmem.c +83 ~~-13~~

138 lines not on this page · raw message

[email protected]

2020-10-29 23:30 UTC #29 lore raw

Test Results: RE: [V2,16/18] sched: highmem: Store local kmaps in task struct

Thanks for your contribution, unfortunately we've found some issues.

Your patch failed to apply to any branch.

Thomas Gleixner

author

2020-10-29 22:18 UTC #30 lore raw

PATCH v2 17/18 mm/highmem: Provide kmap_local*

Now that the kmap atomic index is stored in task struct provide a preemptible variant. On context switch the maps of an outgoing task are removed and the map of the incoming task are restored. That's obviously slow, but highmem is slow anyway.

The kmap_local.*() functions can be invoked from both preemptible and atomic context. kmap local sections disable migration to keep the resulting virtual mapping address correct, but disable neither pagefaults nor preemption.

A wholesale conversion of kmap_atomic to be fully preemptible is not possible because some of the usage sites might rely on the preemption disable for serialization or on the implicit pagefault disable. Needs to be done on a case by case basis.

---
V2: Make it more consistent and add commentry

---
 include/linux/highmem.h |  115 +++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 100 insertions(+), 15 deletions(-)

include/linux/highmem.h +100 ~~-15~~

@@ -86,17 +86,56 @@ static inline void kunmap(struct page *p
 }
 
 /*
- * kmap_atomic/kunmap_atomic is significantly faster than kmap/kunmap because
- * no global lock is needed and because the kmap code must perform a global TLB
- * invalidation when the kmap pool wraps.
- *
- * However when holding an atomic kmap it is not legal to sleep, so atomic
- * kmaps are appropriate for short, tight code paths only.
- *
- * The use of kmap_atomic/kunmap_atomic is discouraged - kmap/kunmap
- * gives a more generic (and caching) interface. But kmap_atomic can
- * be used in IRQ contexts, so in some (very limited) cases we need
- * it.
+ * For highmem systems it is required to temporarily map pages
+ * which reside in the portion of memory which is not covered
+ * by the permanent kernel mapping.
+ *
+ * This comes in three flavors:
+ *
+ * 1) kmap/kunmap:
+ *
+ *    An interface to acquire longer term mappings with no restrictions
+ *    on preemption and migration. This comes with an overhead as the
+ *    mapping space is restricted and protected by a global lock. It
+ *    also requires global TLB invalidation when the kmap pool wraps.
+ *
+ *    kmap() might block when the mapping space is fully utilized until a
+ *    slot becomes available. Only callable from preemptible thread
+ *    context.
+ *
+ * 2) kmap_local.*()/kunmap_local.*()
+ *
+ *    An interface to acquire short term mappings. Can be invoked from any
+ *    context including interrupts. The mapping is per thread, CPU local
+ *    and not globaly visible. It can only be used in the context which
+ *    acquried the mapping. Nesting kmap_local.*() and kmap_atomic.*()
+ *    mappings is allowed to a certain extent (up to KMAP_TYPE_NR).
+ *
+ *    Nested kmap_local.*() and kunmap_local.*() invocations have to be
+ *    strictly ordered because the map implementation is stack based.
+ *
+ *    kmap_local.*() disables migration, but keeps preemption enabled. It's
+ *    valid to take pagefaults in a kmap_local region unless the context in
+ *    which the local kmap is acquired does not allow it for other reasons.
+ *
+ *    If a task holding local kmaps is preempted, the maps are removed on
+ *    context switch and restored when the task comes back on the CPU. As
+ *    the maps are strictly CPU local it is guaranteed that the task stays
+ *    on the CPU and the CPU cannot be unplugged until the local kmaps are
+ *    released.
+ *
+ * 3) kmap_atomic.*()/kunmap_atomic.*()
+ *
+ *    Based on the same mechanism as kmap local. Atomic kmap disables
+ *    preemption and pagefaults. Only use if absolutely required, use
+ *    the corresponding kmap_local variant if possible.
+ *
+ * Local and atomic kmaps are faster than kmap/kunmap, but impose
+ * restrictions. Only use them when required.
+ *
+ * For !HIGHMEM enabled systems the kmap flavours are not doing any mapping
+ * operation and kmap() won't sleep, but the kmap local and atomic variants
+ * still disable migration resp. pagefaults and preemption.
  */
 static inline void *kmap_atomic_prot(struct page *page, pgprot_t prot)
 {

@@ -122,6 +161,28 @@ static inline void __kunmap_atomic(void
 	kunmap_local_indexed(addr);
 }
 
+static inline void *kmap_local_page_prot(struct page *page, pgprot_t prot)
+{
+	migrate_disable();
+	return __kmap_local_page_prot(page, prot);
+}
+
+static inline void *kmap_local_page(struct page *page)
+{
+	return kmap_local_page_prot(page, kmap_prot);
+}
+
+static inline void *kmap_local_pfn(unsigned long pfn)
+{
+	migrate_disable();
+	return __kmap_local_pfn_prot(pfn, kmap_prot);
+}
+
+static inline void __kunmap_local(void *vaddr)
+{
+	kunmap_local_indexed(vaddr);
+}
+
 /* declarations for linux/mm/highmem.c */
 unsigned int nr_free_highpages(void);
 extern atomic_long_t _totalhigh_pages;

@@ -201,10 +262,27 @@ static inline void *kmap_atomic_pfn(unsi
 
 static inline void __kunmap_atomic(void *addr)
 {
-	/*
-	 * Mostly nothing to do in the CONFIG_HIGHMEM=n case as kunmap_atomic()
-	 * handles re-enabling faults and preemption
-	 */
+	__kunmap_local(addr);
+}
+
+static inline void *kmap_local_page(struct page *page)
+{
+	migrate_disable();
+	return page_address(page);
+}
+
+static inline void *kmap_local_page_prot(struct page *page, pgprot_t prot)
+{
+	return kmap_local_page(page);
+}
+
+static inline void *kmap_local_pfn(unsigned long pfn)
+{
+	return kmap_local_page(pfn_to_page(pfn));
+}
+
+static inline void __kunmap_local(void *addr)
+{
 #ifdef ARCH_HAS_FLUSH_ON_KUNMAP
 	kunmap_flush_on_unmap(addr);
 #endif

@@ -226,6 +304,13 @@ do {								\
 	preempt_enable();					\
 } while (0)
 
+#define kunmap_local(__addr)					\
+do {								\
+	BUILD_BUG_ON(__same_type((__addr), struct page *));	\
+	__kunmap_local(__addr);					\
+	migrate_enable();					\
+} while (0)
+
 /* when CONFIG_HIGHMEM is not set these will be plain clear/copy_page */
 #ifndef clear_user_highpage
 static inline void clear_user_highpage(struct page *page, unsigned long vaddr)

[email protected]

2020-10-29 23:21 UTC #31 lore raw

Test Results: RE: [V2,17/18] mm/highmem: Provide kmap_local*

Thanks for your contribution, unfortunately we've found some issues.

Your patch failed to apply to any branch.

Thomas Gleixner

author

2020-10-29 22:18 UTC #32 lore raw

PATCH v2 18/18 io-mapping: Provide iomap_local variant

Similar to kmap local provide a iomap local variant which only disables migration, but neither disables pagefaults nor preemption.

---
V2: Split out from the large combo patch and add the !IOMAP_ATOMIC variants

---
 include/linux/io-mapping.h |   34 ++++++++++++++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

include/linux/io-mapping.h +32 -2

@@ -83,6 +83,23 @@ io_mapping_unmap_atomic(void __iomem *va
 }
 
 static inline void __iomem *
+io_mapping_map_local_wc(struct io_mapping *mapping, unsigned long offset)
+{
+	resource_size_t phys_addr;
+
+	BUG_ON(offset >= mapping->size);
+	phys_addr = mapping->base + offset;
+	migrate_disable();
+	return __iomap_local_pfn_prot(PHYS_PFN(phys_addr), mapping->prot);
+}
+
+static inline void io_mapping_unmap_local(void __iomem *vaddr)
+{
+	kunmap_local_indexed((void __force *)vaddr);
+	migrate_enable();
+}
+
+static inline void __iomem *
 io_mapping_map_wc(struct io_mapping *mapping,
 		  unsigned long offset,
 		  unsigned long size)

@@ -101,7 +118,7 @@ io_mapping_unmap(void __iomem *vaddr)
 	iounmap(vaddr);
 }
 
-#else
+#else  /* HAVE_ATOMIC_IOMAP */
 
 #include <linux/uaccess.h>

@@ -166,7 +183,20 @@ io_mapping_unmap_atomic(void __iomem *va
 	preempt_enable();
 }
 
-#endif /* HAVE_ATOMIC_IOMAP */
+static inline void __iomem *
+io_mapping_map_local_wc(struct io_mapping *mapping, unsigned long offset)
+{
+	migrate_disable();
+	return io_mapping_map_wc(mapping, offset, PAGE_SIZE);
+}
+
+static inline void io_mapping_unmap_local(void __iomem *vaddr)
+{
+	io_mapping_unmap(vaddr);
+	migrate_enable();
+}
+
+#endif /* !HAVE_ATOMIC_IOMAP */
 
 static inline struct io_mapping *
 io_mapping_create_wc(resource_size_t base,

[email protected]

2020-10-29 23:17 UTC #33 lore raw

Test Results: RE: [V2,18/18] io-mapping: Provide iomap_local variant

Thanks for your contribution, unfortunately we've found some issues.

Your patch failed to apply to any branch.

Linus Torvalds

2020-10-29 23:11 UTC #34 lore raw

https://lore.kernel.org/r/[email protected]

Quoted 5 lines · Thomas Gleixner

Though I wanted to share the current state of affairs before investigating that further. If there is consensus in going forward with this, I'll have a deeper look into this issue.

Me likee. I think this looks like the right thing to do.

I didn't actually apply the patches, but just from reading them it _looks_ to me like you do the migrate_disable() unconditionally, even if it's not a highmem page..

That sounds like it might be a good thing for debugging, but not necessarily great in general.

Or am I misreading things?

Linus

Thomas Gleixner

author

2020-10-29 23:41 UTC #35 lore raw

Quoted 8 lines · Linus Torvalds

Quoted 4 lines · Thomas Gleixner
Though I wanted to share the current state of affairs before investigating that further. If there is consensus in going forward with this, I'll have a deeper look into this issue.
Me likee. I think this looks like the right thing to do.

Quoted 8 lines

I didn't actually apply the patches, but just from reading them it _looks_ to me like you do the migrate_disable() unconditionally, even if it's not a highmem page..
That sounds like it might be a good thing for debugging, but not necessarily great in general.
Or am I misreading things?

No, you're not misreading it, but doing it conditionally would be a complete semantical disaster. kmap_atomic*() also disables preemption and pagefaults unconditionaly. If that wouldn't be the case then every caller would have to have conditionals like 'if (CONFIG_HIGHMEM)' or worse 'if (PageHighMem(page)'.

Let's not go there.

Migrate disable is a less horrible plague than preempt and pagefault disable even if the scheduler people disagree due to the lack of theory backing that up :)

The charm of the new interface is that users still can rely on per cpuness independent of being on a highmem plagued system. For non highmem systems the extra migrate disable/enable is really a minor nuissance.

Thanks,

tglx

Thomas Gleixner

author

2020-10-30 09:39 UTC #36 lore raw

Quoted 9 lines · Thomas Gleixner

On Thu, Oct 29 2020 at 16:11, Linus Torvalds wrote:
No, you're not misreading it, but doing it conditionally would be a complete semantical disaster. kmap_atomic*() also disables preemption and pagefaults unconditionaly. If that wouldn't be the case then every caller would have to have conditionals like 'if (CONFIG_HIGHMEM)' or worse 'if (PageHighMem(page)'.
Let's not go there.

Quoted 8 lines

Migrate disable is a less horrible plague than preempt and pagefault disable even if the scheduler people disagree due to the lack of theory backing that up :)
The charm of the new interface is that users still can rely on per cpuness independent of being on a highmem plagued system. For non highmem systems the extra migrate disable/enable is really a minor nuissance.

thinking about it some more after having sleep and coffee, we actually could hide the migrate disable in the actual highmem part.

But then we really should not name it kmap_local. 'local' suggests locality, think local_irq*, local_bh* ... kmap_task would be more accurate then.

Toughts?

tglx

Linus Torvalds

2020-10-30 20:28 UTC #37 lore raw

Quoted 4 lines · Thomas Gleixner

But then we really should not name it kmap_local. 'local' suggests locality, think local_irq*, local_bh* ... kmap_task would be more accurate then.

So the main reason I'd like to see it is because I think on a non-highmem machine, the new kmap should be a complete no-op. IOW, we'd make sure that there are no costs, no need to increment any "restrict migration" counts etc.

It's been a bit of a pain to have kmap_atomic() have magical side semantics that people might then depend on.

I think "local" could still work as a name, because it would have to be thread-local (and maybe we'd want a debug mode where that gets verified, as actual HIGHMEM machines are getting rare).

I'd avoid "task", because that implies (to me, at least) that it wouldn't be good for interrupts etc that don't have a task context.

I think the main issue is that it has to be released in the same context as it was created (ie no passing those things around to other contexts). I think "local" is fine for that, but I could imagine other names. The ones that come to mind are somewhat cumbersome, though ("shortterm" or "local_ctx" or something along those lines).

I dunno.

Linus

Thomas Gleixner

author

2020-10-30 22:26 UTC #38 lore raw

Quoted 10 lines · Linus Torvalds

Quoted 4 lines · Thomas Gleixner
But then we really should not name it kmap_local. 'local' suggests locality, think local_irq*, local_bh* ... kmap_task would be more accurate then.
So the main reason I'd like to see it is because I think on a non-highmem machine, the new kmap should be a complete no-op. IOW, we'd make sure that there are no costs, no need to increment any "restrict migration" counts etc.

Fair enough.

Quoted 2 lines

It's been a bit of a pain to have kmap_atomic() have magical side semantics that people might then depend on.

kmap_atomic() will still have the side semantics :)

Quoted 4 lines

I think "local" could still work as a name, because it would have to be thread-local (and maybe we'd want a debug mode where that gets verified, as actual HIGHMEM machines are getting rare).

Quoted 8 lines

I'd avoid "task", because that implies (to me, at least) that it wouldn't be good for interrupts etc that don't have a task context.
I think the main issue is that it has to be released in the same context as it was created (ie no passing those things around to other contexts). I think "local" is fine for that, but I could imagine other names. The ones that come to mind are somewhat cumbersome, though ("shortterm" or "local_ctx" or something along those lines).

Yeah, not really intuitive either.

Let's stick with _local and add proper function documentation which clearly says, that the side effect of non-migratability applies only for the 32bit highmem case in order to make it work at all.

So code which needs CPU locality cannot rely on it and we have enough debug stuff to catch something like:

    kmap_local()
    this_cpu_write(....)
    kunmap_local()

Let me redo the pile.

While at it I might have a look at that debug request from Willy in the other end of this thread. Any comment on that?

Thanks,

tglx

Linus Torvalds

2020-10-30 22:46 UTC #39 lore raw

Quoted 5 lines · Thomas Gleixner

While at it I might have a look at that debug request from Willy in the other end of this thread. Any comment on that?
https://lore.kernel.org/r/[email protected]

I do think that it would be nice to have a debug mode, particularly since over the last few years we've really lost a lot of HIGHMEM coverage (to the point that I've wondered how worthwhile it really is to support at all any more - I think it's Arnd who argued that it's mainly some embedded ARM variants that will want it for the forseeable future).

So I'm honestly somewhat torn. I think HIGHMEM is dying, and yes that argues for "non-HIGHMEM had better have some debug coverage", but at the same time I think it doesn't even really matter any more. At some point those embedded ARM platforms just aren't even interesting - they might as well use older kernels if they are the only thing really arguing for HIGHMEM at all.

This is one reason why I'd like the new kmap_local() to be a no-op, and I'd prefer for it to have no other side effects - because I want to be ready to remove it entirely some day. And if we end up having some transition where people start rewriting "kmap_atomic()" to be "kmap_local() + explicit preemption disable", then I think that would be a good step on that whole "kmap will eventually go away" path.

But I do *not* believe that we need to add _so_ much debug support that we'd catch Willy's "more than one page" case. And I absolutely do not believe for a second that we should start caring about compound pages. NO. kmap() is almost dead already, we're not making it worse.

To me, your patch series has two big advantages:

- more common code

- kmap_local() becomes more of a no-op

and the last thing we want is to expand on kmap.

Linus

Thomas Gleixner

author

2020-10-30 23:26 UTC #40 lore raw

Quoted 8 lines · Linus Torvalds

On Fri, Oct 30, 2020 at 3:26 PM Thomas Gleixner <[email protected]> wrote:
To me, your patch series has two big advantages:
- more common code
- kmap_local() becomes more of a no-op
and the last thing we want is to expand on kmap.

Happy to go with that.

While trying to document the mess, I just stumbled over the abuse of kmap_atomic_prot() in

drivers/gpu/drm/ttm/ttm_bo_util.c:      dst = kmap_atomic_prot(d, prot);
drivers/gpu/drm/ttm/ttm_bo_util.c:      src = kmap_atomic_prot(s, prot);
drivers/gpu/drm/vmwgfx/vmwgfx_blit.c:   kmap_atomic_prot(d->dst_pages[dst_page],
drivers/gpu/drm/vmwgfx/vmwgfx_blit.c:   kmap_atomic_prot(d->src_pages[src_page],

For !HIGHMEM kmap_atomic_prot() just ignores the 'prot' argument and returns the page address.

Moar patches to be written ... Sigh!

Thanks,

tglx

Thomas Gleixner

author

2020-10-30 23:53 UTC #41 lore raw

Quoted 20 lines · Thomas Gleixner

Quoted 8 lines · Linus Torvalds
On Fri, Oct 30, 2020 at 3:26 PM Thomas Gleixner <[email protected]> wrote:
To me, your patch series has two big advantages:
- more common code
- kmap_local() becomes more of a no-op
and the last thing we want is to expand on kmap.
Happy to go with that.
While trying to document the mess, I just stumbled over the abuse of kmap_atomic_prot() in
drivers/gpu/drm/ttm/ttm_bo_util.c:      dst = kmap_atomic_prot(d, prot);
drivers/gpu/drm/ttm/ttm_bo_util.c:      src = kmap_atomic_prot(s, prot);
drivers/gpu/drm/vmwgfx/vmwgfx_blit.c:   kmap_atomic_prot(d->dst_pages[dst_page],
drivers/gpu/drm/vmwgfx/vmwgfx_blit.c:   kmap_atomic_prot(d->src_pages[src_page],

Quoted 4 lines

For !HIGHMEM kmap_atomic_prot() just ignores the 'prot' argument and returns the page address.
Moar patches to be written ... Sigh!

Or not. This is actually correct by some definition of correct. For the non highmem case pgprot is set via the set_memory_*() functions and this just handles the highmem case.

Comments are overrrated...

Arnd Bergmann

2020-10-31 13:37 UTC #42 lore raw

In reply to Linus Torvalds

Quoted 15 lines · Linus Torvalds

Quoted 5 lines · Thomas Gleixner
While at it I might have a look at that debug request from Willy in the other end of this thread. Any comment on that?
https://lore.kernel.org/r/[email protected]
I do think that it would be nice to have a debug mode, particularly since over the last few years we've really lost a lot of HIGHMEM coverage (to the point that I've wondered how worthwhile it really is to support at all any more - I think it's Arnd who argued that it's mainly some embedded ARM variants that will want it for the forseeable future).

Quoted 3 lines

So I'm honestly somewhat torn. I think HIGHMEM is dying, and yes that argues for "non-HIGHMEM had better have some debug coverage", but at the same time I think it doesn't even really matter any more.

Shifting the testing of highmem code to the actual users of highmem sounds reasonable to me. This means it will get broken more often, but if it doesn't happen all the time, it might serve as a canary:
once a bug survives for long enough, we have a good indication that users stopped caring ;-)

Quoted 4 lines

At some
point those embedded ARM platforms just aren't even interesting - they might as well use older kernels if they are the only thing really arguing for HIGHMEM at all.

Agreed, but it does need a little time to get there. My best guess is three to five years from now we can remove it for good, provided a few things happen first:

1. The remaining users of TI Keystone2, NXP i.MX6 Quad(Plus) and Renesas R8A7790/R8A7791/R8A7742/R8A7743 that use the largest memory configuration must have stopped requiring kernel version updates.
These were all introduced at the peak of 32-bit Arm embedded systems around 2013, but they have long (15+ year) product life cycles and users pick these because they do promise kernel updates, unlike other SoC families that get stuck on old vendor kernels much earlier.

2. The plan to add a CONFIG_VMSPLIT_4G_4G option on arch/arm/ must work out. We don't have all the code yet, and so far it looks like it's going to be a bit ugly and a bit slow but not nearly as ugly or slow as it was on x86 20 years ago.
This would cover Cortex-A7/A15 systems from 2GB to 4GB, which are still fairly common and need to keep using mainline kernels for much longer than the system in point 1.
It won't help on Cortex-A9 machines with 2GB, which I hope can migrate CONFIG_VMSPLIT_2G_OPT as a fallback.

3. No new systems with larger memory must appear. I noticed that e.g. the newly introduced Rockchips RV1109 and Allwinner A50/R328/V316 support LP-DDR4 on a dual Cortex-A7, but I hope that nobody will in practice combine a $2 SoC with a $15 memory chip.
Most other 32-bit chips use DDR3, which tends to prohibit configurations over 4GB in new designs, with the cheapest ones limited to 512MB (a single 256Mx16 chip) and the high end having already moved on to 64 bit.

Regarding 32-bit non-Arm systems, support for the MIPS-based Baikal T1 was merged earlier this year and is used in Russian PC style systems with up to 8GB.
There are also some users on 10+ year old 32-bit netbooks or business laptops, both x86 and Apple G4.
The longest-lived 32-bit embedded systems with large memory (other than Arm) are probably NXP QorIQ P20xx/P40xx used in military VME bus systems, and low-end embedded systems based on Vortex86.
I'm less worried about all of these because upstream kernel support for ppc32 and x86-32 is already bitrotting and they will likely get stuck on the last working kernel before the TI/Renesas/NXP Arm systems do.

Arnd

Christophe Leroy

2020-10-31 15:03 UTC #43 lore raw

Quoted 12 lines

There are also some users on 10+ year old 32-bit netbooks or business laptops, both x86 and Apple G4.
The longest-lived 32-bit embedded systems with large memory (other than Arm) are probably NXP QorIQ P20xx/P40xx used in military VME bus systems, and low-end embedded systems based on Vortex86.
I'm less worried about all of these because upstream kernel support for ppc32 and x86-32 is already bitrotting and they will likely get stuck on the last working kernel before the TI/Renesas/NXP Arm systems do.

Upstream kernel support for ppc32 is bitrotting, seriously ? What do you mean exactly ?

ppc32 is actively supported, with recent addition of support of hugepages, kasan, uaccess protection, VMAP stack, etc ...

Christophe

Arnd Bergmann

2020-10-31 21:33 UTC #44 lore raw

Quoted 12 lines · Christophe Leroy

Quoted 11 lines
There are also some users on 10+ year old 32-bit netbooks or business laptops, both x86 and Apple G4.
The longest-lived 32-bit embedded systems with large memory (other than Arm) are probably NXP QorIQ P20xx/P40xx used in military VME bus systems, and low-end embedded systems based on Vortex86.
I'm less worried about all of these because upstream kernel support for ppc32 and x86-32 is already bitrotting and they will likely get stuck on the last working kernel before the TI/Renesas/NXP Arm systems do.

Quoted 2 lines

Upstream kernel support for ppc32 is bitrotting, seriously ? What do you mean exactly ?

I was thinking more of the platform support: out of the twelve 32-bit platforms in arch/powerpc/platforms/, your 8xx is the only one listed as 'maintained' or 'supported' in the maintainers list, and that seems to accurately describe the current state.

Freescale seems to have practically stopped contributing to any of their 32-bit platforms in 2016 after the NXP acquisition and no longer employing either of the maintainers. Similarly, Ben seems to have stopped working on powermac in 2016, which was ten years after the last 32-bit hardware shipped for that platform.

Quoted 2 lines

ppc32 is actively supported, with recent addition of support of hugepages, kasan, uaccess protection, VMAP stack, etc ...

That is good to hear, I didn't know about these additions. What platforms are people using to develop these? Is this mainly your 8xx work, or is there ongoing development for platforms that need highmem?

Arnd

Christoph Hellwig

2020-10-30 07:25 UTC #45 lore raw

Quoted 1 line · Thomas Gleixner

This is achieved by:

...

Quoted 2 lines

- Consolidating all kmap atomic implementations in generic code

...

Quoted 3 lines

Though I wanted to share the current state of affairs before investigating that further. If there is consensus in going forward with this, I'll have a deeper look into this issue.

I think the consolidation is a winner no matter where we go next. Maybe split it out in a prep series so we can get it in ASAP?

Thomas Gleixner

author

2020-10-30 09:39 UTC #46 lore raw

Quoted 5 lines · Christoph Hellwig

Quoted 1 line · Thomas Gleixner
- Consolidating all kmap atomic implementations in generic code
I think the consolidation is a winner no matter where we go next. Maybe split it out in a prep series so we can get it in ASAP?

Yes, patch 2-15 can just go without any dependency. The only thing which needs a bit of thought is naming. See the other reply to Linus.

Thanks,

tglx

Matthew Wilcox

2020-10-30 13:06 UTC #47 lore raw

Quoted 5 lines · Thomas Gleixner

This series provides kmap_local.* iomap_local variants which only disable migration to keep the virtual mapping address stable accross preemption, but do neither disable pagefaults nor preemption. The new functions can be used in any context, but if used in atomic context the caller has to take care of eventually disabling pagefaults.

Could I ask for a CONFIG_KMAP_DEBUG which aliases all the kmap variants to vmap()? I think we currently have a problem in iov_iter on HIGHMEM configs:

copy_page_to_iter() calls page_copy_sane() which checks:

        head = compound_head(page);
        if (likely(n <= v && v <= page_size(head)))
                return true;

but then:

                void *kaddr = kmap_atomic(page);
                size_t wanted = copy_to_iter(kaddr + offset, bytes, i);
                kunmap_atomic(kaddr);

so if offset to offset+bytes is larger than PAGE_SIZE, this is going to work for lowmem pages and fail miserably for highmem pages. I suggest vmap() because vmap has a PAGE_SIZE gap between each allocation.

Alternatively if we could have a kmap_atomic_compound(), that would be awesome, but probably not realistic to implement. I've more or less resigned myself to having to map things one page at a time.

Thomas Gleixner

author

2020-10-30 19:35 UTC #48 lore raw

Quoted 10 lines · Matthew Wilcox

Quoted 5 lines · Thomas Gleixner
This series provides kmap_local.* iomap_local variants which only disable migration to keep the virtual mapping address stable accross preemption, but do neither disable pagefaults nor preemption. The new functions can be used in any context, but if used in atomic context the caller has to take care of eventually disabling pagefaults.
Could I ask for a CONFIG_KMAP_DEBUG which aliases all the kmap variants to vmap()? I think we currently have a problem in iov_iter on HIGHMEM configs:

For kmap() that would work, but for kmap_atomic() not so much when it is called in non-preemptible context because vmap() might sleep.

Quoted 8 lines

copy_page_to_iter() calls page_copy_sane() which checks:
        head = compound_head(page);
        if (likely(n <= v && v <= page_size(head)))
                return true;
but then:

Quoted 7 lines

                void *kaddr = kmap_atomic(page);
                size_t wanted = copy_to_iter(kaddr + offset, bytes, i);
                kunmap_atomic(kaddr);
so if offset to offset+bytes is larger than PAGE_SIZE, this is going to work for lowmem pages and fail miserably for highmem pages. I suggest vmap() because vmap has a PAGE_SIZE gap between each allocation.

On 32bit highmem the kmap_atomic() case is easy: Double the number of mapping slots and only use every second one, which gives you a guard page between the maps.

For 64bit we could do something ugly: Enable the highmem kmap_atomic() crud and enforce an alias mapping (at least on the architectures where this is reasonable). Then you get the same as for 32bit.

Quoted 3 lines

Alternatively if we could have a kmap_atomic_compound(), that would be awesome, but probably not realistic to implement. I've more or less resigned myself to having to map things one page at a time.

That might be horribly awesome on 32bit :)

Thanks,

tglx

Thomas Gleixner

author

2020-11-02 01:08 UTC #49 lore raw