Re: [PATCH V1 09/10] arch/kmap: Define kmap_atomic_prot() for all arch's
On Fri, May 01, 2020 at 04:20:20AM +0100, Al Viro wrote:
> On Fri, May 01, 2020 at 03:37:34AM +0100, Al Viro wrote:
> > On Thu, Apr 30, 2020 at 01:38:44PM -0700, ira.we...@intel.com wrote:
> >
> > > -static inline void *kmap_atomic(struct page *page)
> > > +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(page);
> > > + return kmap_atomic_high_prot(page, prot);
> > > }
> > > +#define kmap_atomic(page)kmap_atomic_prot(page, kmap_prot)
> >
> > OK, so it *was* just a bisect hazard - you return to original semantics
> > wrt preempt_disable()...
>
> FWIW, how about doing the following: just before #5/10 have a patch
> that would touch only microblaze, ppc and x86 splitting their
> kmap_atomic_prot() into an inline helper + kmap_atomic_high_prot().
> Then your #5 would leave their kmap_atomic_prot() as-is (it would
> use kmap_atomic_prot_high() instead). The rest of the series plays
> out pretty much the same way it does now, and wrappers on those
> 3 architectures would go away when an identical generic one is
> introduced in this commit (#9/10).
>
> AFAICS, that would avoid the bisect hazard and might even end
> up with less noise in the patches...
This works. V2 coming out shortly.
Thanks for catching this,
Ira
Re: [PATCH V1 09/10] arch/kmap: Define kmap_atomic_prot() for all arch's
On Thu, Apr 30, 2020 at 01:38:44PM -0700, ira.we...@intel.com wrote:
> From: Ira Weiny
>
> To support kmap_atomic_prot(), all architectures need to support
> protections passed to their kmap_atomic_high() function. Pass
> protections into kmap_atomic_high() and change the name to
> kmap_atomic_high_prot() to match.
>
> Then define kmap_atomic_prot() as a core function which calls
> kmap_atomic_high_prot() when needed.
>
> Finally, redefine kmap_atomic() as a wrapper of kmap_atomic_prot() with
> the default kmap_prot exported by the architectures.
Looks good,
Reviewed-by: Christoph Hellwig
But can you also consolidate the kmap_atomic_high_prot and
kunmap_atomic_high in linux/highmem.h instead of keeping the duplicates
in all arch headers?
>
> Signed-off-by: Ira Weiny
> ---
> arch/arc/include/asm/highmem.h| 2 +-
> arch/arc/mm/highmem.c | 6 +++---
> arch/arm/include/asm/highmem.h| 2 +-
> arch/arm/mm/highmem.c | 6 +++---
> arch/csky/include/asm/highmem.h | 2 +-
> arch/csky/mm/highmem.c| 6 +++---
> arch/microblaze/include/asm/highmem.h | 7 +--
> arch/microblaze/mm/highmem.c | 4 ++--
> arch/mips/include/asm/highmem.h | 2 +-
> arch/mips/mm/highmem.c| 6 +++---
> arch/nds32/include/asm/highmem.h | 2 +-
> arch/nds32/mm/highmem.c | 6 +++---
> arch/powerpc/include/asm/highmem.h| 8 +---
> arch/powerpc/mm/highmem.c | 4 ++--
> arch/sparc/include/asm/highmem.h | 2 +-
> arch/sparc/mm/highmem.c | 6 +++---
> arch/x86/include/asm/highmem.h| 6 +-
> arch/x86/mm/highmem_32.c | 4 ++--
> arch/xtensa/include/asm/highmem.h | 2 +-
> arch/xtensa/mm/highmem.c | 6 +++---
> include/linux/highmem.h | 5 +++--
> 21 files changed, 40 insertions(+), 54 deletions(-)
>
> diff --git a/arch/arc/include/asm/highmem.h b/arch/arc/include/asm/highmem.h
> index e16531495620..09f86bde6809 100644
> --- a/arch/arc/include/asm/highmem.h
> +++ b/arch/arc/include/asm/highmem.h
> @@ -30,7 +30,7 @@
>
> #include
>
> -extern void *kmap_atomic_high(struct page *page);
> +extern void *kmap_atomic_high_prot(struct page *page, pgprot_t prot);
> extern void kunmap_atomic_high(void *kvaddr);
>
> extern void kmap_init(void);
> diff --git a/arch/arc/mm/highmem.c b/arch/arc/mm/highmem.c
> index 5d3eab4ac0b0..479b0d72d3cf 100644
> --- a/arch/arc/mm/highmem.c
> +++ b/arch/arc/mm/highmem.c
> @@ -49,7 +49,7 @@
> extern pte_t * pkmap_page_table;
> static pte_t * fixmap_page_table;
>
> -void *kmap_atomic_high(struct page *page)
> +void *kmap_atomic_high_prot(struct page *page, pgprot_t prot)
> {
> int idx, cpu_idx;
> unsigned long vaddr;
> @@ -59,11 +59,11 @@ void *kmap_atomic_high(struct page *page)
> vaddr = FIXMAP_ADDR(idx);
>
> set_pte_at(_mm, vaddr, fixmap_page_table + idx,
> -mk_pte(page, kmap_prot));
> +mk_pte(page, prot));
>
> return (void *)vaddr;
> }
> -EXPORT_SYMBOL(kmap_atomic_high);
> +EXPORT_SYMBOL(kmap_atomic_high_prot);
>
> void kunmap_atomic_high(void *kv)
> {
> diff --git a/arch/arm/include/asm/highmem.h b/arch/arm/include/asm/highmem.h
> index a9d5e9bce1cc..e35f2f73f6aa 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_high(struct page *page);
> +extern void *kmap_atomic_high_prot(struct page *page, pgprot_t prot);
> extern void kunmap_atomic_high(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 ac8394655a6e..e013f6b81328 100644
> --- a/arch/arm/mm/highmem.c
> +++ b/arch/arm/mm/highmem.c
> @@ -31,7 +31,7 @@ static inline pte_t get_fixmap_pte(unsigned long vaddr)
> return *ptep;
> }
>
> -void *kmap_atomic_high(struct page *page)
> +void *kmap_atomic_high_prot(struct page *page, pgprot_t prot)
> {
> unsigned int idx;
> unsigned long vaddr;
> @@ -67,11 +67,11 @@ void *kmap_atomic_high(struct page *page)
>* in place, so the contained TLB flush ensures the TLB is updated
>* with the new mapping.
>*/
> - set_fixmap_pte(idx, mk_pte(page, kmap_prot));
> + set_fixmap_pte(idx, mk_pte(page, prot));
>
> return (void *)vaddr;
> }
> -EXPORT_SYMBOL(kmap_atomic_high);
> +EXPORT_SYMBOL(kmap_atomic_high_prot);
>
> void kunmap_atomic_high(void *kvaddr)
> {
> diff --git a/arch/csky/include/asm/highmem.h b/arch/csky/include/asm/highmem.h
> index 5bbbe59e60a9..59854c7ccf78 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
Re: [PATCH V1 09/10] arch/kmap: Define kmap_atomic_prot() for all arch's
On Fri, May 01, 2020 at 03:37:34AM +0100, Al Viro wrote:
> On Thu, Apr 30, 2020 at 01:38:44PM -0700, ira.we...@intel.com wrote:
>
> > -static inline void *kmap_atomic(struct page *page)
> > +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(page);
> > + return kmap_atomic_high_prot(page, prot);
> > }
> > +#define kmap_atomic(page) kmap_atomic_prot(page, kmap_prot)
>
> OK, so it *was* just a bisect hazard - you return to original semantics
> wrt preempt_disable()...
FWIW, how about doing the following: just before #5/10 have a patch
that would touch only microblaze, ppc and x86 splitting their
kmap_atomic_prot() into an inline helper + kmap_atomic_high_prot().
Then your #5 would leave their kmap_atomic_prot() as-is (it would
use kmap_atomic_prot_high() instead). The rest of the series plays
out pretty much the same way it does now, and wrappers on those
3 architectures would go away when an identical generic one is
introduced in this commit (#9/10).
AFAICS, that would avoid the bisect hazard and might even end
up with less noise in the patches...
Re: [PATCH V1 09/10] arch/kmap: Define kmap_atomic_prot() for all arch's
On Thu, Apr 30, 2020 at 01:38:44PM -0700, ira.we...@intel.com wrote:
> -static inline void *kmap_atomic(struct page *page)
> +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(page);
> + return kmap_atomic_high_prot(page, prot);
> }
> +#define kmap_atomic(page)kmap_atomic_prot(page, kmap_prot)
OK, so it *was* just a bisect hazard - you return to original semantics
wrt preempt_disable()...
[PATCH V1 09/10] arch/kmap: Define kmap_atomic_prot() for all arch's
From: Ira Weiny
To support kmap_atomic_prot(), all architectures need to support
protections passed to their kmap_atomic_high() function. Pass
protections into kmap_atomic_high() and change the name to
kmap_atomic_high_prot() to match.
Then define kmap_atomic_prot() as a core function which calls
kmap_atomic_high_prot() when needed.
Finally, redefine kmap_atomic() as a wrapper of kmap_atomic_prot() with
the default kmap_prot exported by the architectures.
Signed-off-by: Ira Weiny
---
arch/arc/include/asm/highmem.h| 2 +-
arch/arc/mm/highmem.c | 6 +++---
arch/arm/include/asm/highmem.h| 2 +-
arch/arm/mm/highmem.c | 6 +++---
arch/csky/include/asm/highmem.h | 2 +-
arch/csky/mm/highmem.c| 6 +++---
arch/microblaze/include/asm/highmem.h | 7 +--
arch/microblaze/mm/highmem.c | 4 ++--
arch/mips/include/asm/highmem.h | 2 +-
arch/mips/mm/highmem.c| 6 +++---
arch/nds32/include/asm/highmem.h | 2 +-
arch/nds32/mm/highmem.c | 6 +++---
arch/powerpc/include/asm/highmem.h| 8 +---
arch/powerpc/mm/highmem.c | 4 ++--
arch/sparc/include/asm/highmem.h | 2 +-
arch/sparc/mm/highmem.c | 6 +++---
arch/x86/include/asm/highmem.h| 6 +-
arch/x86/mm/highmem_32.c | 4 ++--
arch/xtensa/include/asm/highmem.h | 2 +-
arch/xtensa/mm/highmem.c | 6 +++---
include/linux/highmem.h | 5 +++--
21 files changed, 40 insertions(+), 54 deletions(-)
diff --git a/arch/arc/include/asm/highmem.h b/arch/arc/include/asm/highmem.h
index e16531495620..09f86bde6809 100644
--- a/arch/arc/include/asm/highmem.h
+++ b/arch/arc/include/asm/highmem.h
@@ -30,7 +30,7 @@
#include
-extern void *kmap_atomic_high(struct page *page);
+extern void *kmap_atomic_high_prot(struct page *page, pgprot_t prot);
extern void kunmap_atomic_high(void *kvaddr);
extern void kmap_init(void);
diff --git a/arch/arc/mm/highmem.c b/arch/arc/mm/highmem.c
index 5d3eab4ac0b0..479b0d72d3cf 100644
--- a/arch/arc/mm/highmem.c
+++ b/arch/arc/mm/highmem.c
@@ -49,7 +49,7 @@
extern pte_t * pkmap_page_table;
static pte_t * fixmap_page_table;
-void *kmap_atomic_high(struct page *page)
+void *kmap_atomic_high_prot(struct page *page, pgprot_t prot)
{
int idx, cpu_idx;
unsigned long vaddr;
@@ -59,11 +59,11 @@ void *kmap_atomic_high(struct page *page)
vaddr = FIXMAP_ADDR(idx);
set_pte_at(_mm, vaddr, fixmap_page_table + idx,
- mk_pte(page, kmap_prot));
+ mk_pte(page, prot));
return (void *)vaddr;
}
-EXPORT_SYMBOL(kmap_atomic_high);
+EXPORT_SYMBOL(kmap_atomic_high_prot);
void kunmap_atomic_high(void *kv)
{
diff --git a/arch/arm/include/asm/highmem.h b/arch/arm/include/asm/highmem.h
index a9d5e9bce1cc..e35f2f73f6aa 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_high(struct page *page);
+extern void *kmap_atomic_high_prot(struct page *page, pgprot_t prot);
extern void kunmap_atomic_high(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 ac8394655a6e..e013f6b81328 100644
--- a/arch/arm/mm/highmem.c
+++ b/arch/arm/mm/highmem.c
@@ -31,7 +31,7 @@ static inline pte_t get_fixmap_pte(unsigned long vaddr)
return *ptep;
}
-void *kmap_atomic_high(struct page *page)
+void *kmap_atomic_high_prot(struct page *page, pgprot_t prot)
{
unsigned int idx;
unsigned long vaddr;
@@ -67,11 +67,11 @@ void *kmap_atomic_high(struct page *page)
* in place, so the contained TLB flush ensures the TLB is updated
* with the new mapping.
*/
- set_fixmap_pte(idx, mk_pte(page, kmap_prot));
+ set_fixmap_pte(idx, mk_pte(page, prot));
return (void *)vaddr;
}
-EXPORT_SYMBOL(kmap_atomic_high);
+EXPORT_SYMBOL(kmap_atomic_high_prot);
void kunmap_atomic_high(void *kvaddr)
{
diff --git a/arch/csky/include/asm/highmem.h b/arch/csky/include/asm/highmem.h
index 5bbbe59e60a9..59854c7ccf78 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_high(struct page *page);
+extern void *kmap_atomic_high_prot(struct page *page, pgprot_t prot);
extern void kunmap_atomic_high(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 b/arch/csky/mm/highmem.c
index f4311669b5bb..3ae5c8cd7619 100644
--- a/arch/csky/mm/highmem.c
+++ b/arch/csky/mm/highmem.c
@@ -21,7 +21,7 @@
