Re: [PATCH V1 05/10] arch/kmap_atomic: Consolidate duplicate code

2020-05-01 Thread Christoph Hellwig 
On Thu, Apr 30, 2020 at 01:38:40PM -0700, ira.we...@intel.com wrote:
> From: Ira Weiny 
> 
> Every arch has the same code to ensure atomic operations and a check for
> !HIGHMEM page.
> 
> Remove the duplicate code by defining a core kmap_atomic() which only
> calls the arch specific kmap_atomic_high() when the page is high memory.
> 
> Signed-off-by: Ira Weiny 

Looks good:

Reviewed-by: Christoph Hellwig 

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH V1 05/10] arch/kmap_atomic: Consolidate duplicate code

2020-04-30 Thread Al Viro 
On Thu, Apr 30, 2020 at 01:38:40PM -0700, ira.we...@intel.com wrote:
> From: Ira Weiny 
> 
> Every arch has the same code to ensure atomic operations and a check for
> !HIGHMEM page.
> 
> Remove the duplicate code by defining a core kmap_atomic() which only
> calls the arch specific kmap_atomic_high() when the page is high memory.

Err  AFAICS, you've just silently changed the semantics for
kmap_atomic_prot() here.  And while most of the callers are converted,
drivers/gpu/drm/ttm/ttm_bo_util.c one is not, so at the very least it's
a bisect hazard...

And I would argue that having kmap_atomic() differ from kmap_atomic_prot()
wrt disabling preempt is asking for trouble.

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

[PATCH V1 05/10] arch/kmap_atomic: Consolidate duplicate code

2020-04-30 Thread ira . weiny 
From: Ira Weiny 

Every arch has the same code to ensure atomic operations and a check for
!HIGHMEM page.

Remove the duplicate code by defining a core kmap_atomic() which only
calls the arch specific kmap_atomic_high() when the page is high memory.

Signed-off-by: Ira Weiny 

---
Changes from V0:
consolidate comments
Use a similar architecture to kmap() and define
kmap_atomic_high() for architecture specific
functionality
Fix 0-day build issue in arch/mips/mm/cache.c
---
 arch/arc/include/asm/highmem.h|  2 +-
 arch/arc/mm/highmem.c |  9 ++---
 arch/arm/include/asm/highmem.h|  2 +-
 arch/arm/mm/highmem.c |  9 ++---
 arch/csky/include/asm/highmem.h   |  2 +-
 arch/csky/mm/highmem.c|  9 ++---
 arch/microblaze/include/asm/highmem.h |  2 +-
 arch/microblaze/mm/highmem.c  |  6 --
 arch/mips/include/asm/highmem.h   |  2 +-
 arch/mips/mm/cache.c  |  2 +-
 arch/mips/mm/highmem.c| 18 ++
 arch/nds32/include/asm/highmem.h  |  2 +-
 arch/nds32/mm/highmem.c   |  9 ++---
 arch/powerpc/include/asm/highmem.h|  2 +-
 arch/powerpc/mm/highmem.c | 11 ---
 arch/sparc/include/asm/highmem.h  |  2 +-
 arch/sparc/mm/highmem.c   |  9 ++---
 arch/x86/include/asm/highmem.h|  7 +--
 arch/x86/mm/highmem_32.c  | 20 
 arch/xtensa/include/asm/highmem.h |  2 +-
 arch/xtensa/mm/highmem.c  |  9 ++---
 include/linux/highmem.h   | 22 ++
 22 files changed, 51 insertions(+), 107 deletions(-)

diff --git a/arch/arc/include/asm/highmem.h b/arch/arc/include/asm/highmem.h
index 8387a5596a91..75bd0fa77fe2 100644
--- a/arch/arc/include/asm/highmem.h
+++ b/arch/arc/include/asm/highmem.h
@@ -30,7 +30,7 @@
 
 #include 
 
-extern void *kmap_atomic(struct page *page);
+extern void *kmap_atomic_high(struct page *page);
 extern void __kunmap_atomic(void *kvaddr);
 
 extern void kmap_init(void);
diff --git a/arch/arc/mm/highmem.c b/arch/arc/mm/highmem.c
index 4db13a6b9f3b..0964b011c29f 100644
--- a/arch/arc/mm/highmem.c
+++ b/arch/arc/mm/highmem.c
@@ -49,16 +49,11 @@
 extern pte_t * pkmap_page_table;
 static pte_t * fixmap_page_table;
 
-void *kmap_atomic(struct page *page)
+void *kmap_atomic_high(struct page *page)
 {
int idx, cpu_idx;
unsigned long vaddr;
 
-   preempt_disable();
-   pagefault_disable();
-   if (!PageHighMem(page))
-   return page_address(page);
-
cpu_idx = kmap_atomic_idx_push();
idx = cpu_idx + KM_TYPE_NR * smp_processor_id();
vaddr = FIXMAP_ADDR(idx);
@@ -68,7 +63,7 @@ void *kmap_atomic(struct page *page)
 
return (void *)vaddr;
 }
-EXPORT_SYMBOL(kmap_atomic);
+EXPORT_SYMBOL(kmap_atomic_high);
 
 void __kunmap_atomic(void *kv)
 {
diff --git a/arch/arm/include/asm/highmem.h b/arch/arm/include/asm/highmem.h
index 736f65283e7b..4edb6db3a5c8 100644
--- a/arch/arm/include/asm/highmem.h
+++ b/arch/arm/include/asm/highmem.h
@@ -60,7 +60,7 @@ static inline void *kmap_high_get(struct page *page)
  * when CONFIG_HIGHMEM is not set.
  */
 #ifdef CONFIG_HIGHMEM
-extern void *kmap_atomic(struct page *page);
+extern void *kmap_atomic_high(struct page *page);
 extern void __kunmap_atomic(void *kvaddr);
 extern void *kmap_atomic_pfn(unsigned long pfn);
 #endif
diff --git a/arch/arm/mm/highmem.c b/arch/arm/mm/highmem.c
index c700b32350ee..075fdc235091 100644
--- a/arch/arm/mm/highmem.c
+++ b/arch/arm/mm/highmem.c
@@ -31,18 +31,13 @@ static inline pte_t get_fixmap_pte(unsigned long vaddr)
return *ptep;
 }
 
-void *kmap_atomic(struct page *page)
+void *kmap_atomic_high(struct page *page)
 {
unsigned int idx;
unsigned long vaddr;
void *kmap;
int type;
 
-   preempt_disable();
-   pagefault_disable();
-   if (!PageHighMem(page))
-   return page_address(page);
-
 #ifdef CONFIG_DEBUG_HIGHMEM
/*
 * There is no cache coherency issue when non VIVT, so force the
@@ -76,7 +71,7 @@ void *kmap_atomic(struct page *page)
 
return (void *)vaddr;
 }
-EXPORT_SYMBOL(kmap_atomic);
+EXPORT_SYMBOL(kmap_atomic_high);
 
 void __kunmap_atomic(void *kvaddr)
 {
diff --git a/arch/csky/include/asm/highmem.h b/arch/csky/include/asm/highmem.h
index be11c5b67122..6807df1232f3 100644
--- a/arch/csky/include/asm/highmem.h
+++ b/arch/csky/include/asm/highmem.h
@@ -32,7 +32,7 @@ extern pte_t *pkmap_page_table;
 
 #define ARCH_HAS_KMAP_FLUSH_TLB
 extern void kmap_flush_tlb(unsigned long addr);
-extern void *kmap_atomic(struct page *page);
+extern void *kmap_atomic_high(struct page *page);
 extern void __kunmap_atomic(void *kvaddr);
 extern void *kmap_atomic_pfn(unsigned long pfn);
 extern struct page *kmap_atomic_to_page(void *ptr);
diff --git a/arch/csky/mm/highmem.c