Skip to contentSkip to conversation

mm: uncached vma support with writenotify

RFC PATCH Superseded Magnus Damm 5 participants 12 messages Jun 14, 2009 – Jun 23, 2009

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 mm: uncached vma support with writenotify

From: Magnus Damm <[email protected]>

This patch modifies the generic mmap() code to keep the cache attribute in vma->vm_page_prot regardless if writenotify is enabled or not. Without this patch the cache configuration selected by f_op->mmap() is overwritten if writenotify is enabled, making it impossible to keep the vma uncached.

Needed by drivers such as drivers/video/sh_mobile_lcdcfb.c which uses deferred io together with uncached memory.

---

Everyone: Maybe there is a cleaner way to do this?

Jaya: Will post a patch for deferred io to use fb_pgprotect() in the non-vmalloc mmap() case. That patch together with this allow architectures to use uncached memory with deferred io.

 mm/mmap.c |   14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)
mm/mmap.c +13 -1
@@ -1209,8 +1209,20 @@ munmap_back:
 	pgoff = vma->vm_pgoff;
 	vm_flags = vma->vm_flags;
 
-	if (vma_wants_writenotify(vma))
+	if (vma_wants_writenotify(vma)) {
+		pgprot_t pprot = vma->vm_page_prot;
+
+		/* Can vma->vm_page_prot have changed??
+		 *
+		 * Answer: Yes, drivers may have changed it in their
+		 *         f_op->mmap method.
+		 *
+		 * Ensures that vmas marked as uncached stay that way.
+		 */
 		vma->vm_page_prot = vm_get_page_prot(vm_flags & ~VM_SHARED);
+		if (pgprot_val(pprot) == pgprot_val(pgprot_noncached(pprot)))
+			vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
+	}
 
 	vma_link(mm, vma, prev, rb_link, rb_parent);
 	file = vma->vm_file;

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#1 lore raw
Quoted 15 lines · Magnus Damm
mm/mmap.c +7 -1
@@ -1209,8 +1209,20 @@ munmap_back:
 	pgoff = vma->vm_pgoff;
 	vm_flags = vma->vm_flags;
 
-	if (vma_wants_writenotify(vma))
+	if (vma_wants_writenotify(vma)) {
+		pgprot_t pprot = vma->vm_page_prot;
+
+		/* Can vma->vm_page_prot have changed??
+		 *
+		 * Answer: Yes, drivers may have changed it in their
+		 *         f_op->mmap method.

mm/mmap.c munmap_back: · line 1228

Quoted 10 lines
mm/mmap.c +6 -0
+		 *
+		 * Ensures that vmas marked as uncached stay that way.
+		 */
 		vma->vm_page_prot = vm_get_page_prot(vm_flags & ~VM_SHARED);
+		if (pgprot_val(pprot) == pgprot_val(pgprot_noncached(pprot)))
+			vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
+	}
 
 	vma_link(mm, vma, prev, rb_link, rb_parent);
 	file = vma->vm_file;

I guess the only real issue here is that we presently have no generic interface in the kernel for setting a VMA uncached. pgprot_noncached() is the closest approximation we have, but there are still architectures that do not implement it.

Given that this comes up at least once a month, perhaps it makes sense to see which platforms are still outstanding. At least cris, h8300, m68knommu, s390, and xtensa all presently lack a definition for it. The nommu cases are easily handled, but the rest still require some attention from their architecture maintainers before we can really start treating this as a generic interface.

Until then, you will have to do what every other user of pgprot_noncached() code does in generic code:

	#ifdef pgprot_noncached
		vma->vm_page_prot = pgprot_noncached(...);
	#endif

OTOH, I guess we could just add something like:

	#define pgprot_noncached(x)	(x) 

which works fine for the nommu case, and which functionally is no different from what happens right now anyways for the users that don't wire it up sanely.

Arnd, what do you think about throwing this at asm-generic?

---

 include/asm-generic/pgtable.h |    4 ++++
 1 file changed, 4 insertions(+)
include/asm-generic/pgtable.h +4 -0
index e410f60..e2bd73e 100644
@@ -129,6 +129,10 @@ static inline void ptep_set_wrprotect(struct mm_struct *mm, unsigned long addres
 #define move_pte(pte, prot, old_addr, new_addr)	(pte)
 #endif
 
+#ifndef pgprot_noncached
+#define pgprot_noncached(prot)	(prot)
+#endif
+
 #ifndef pgprot_writecombine
 #define pgprot_writecombine pgprot_noncached
 #endif

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

maintainer
#2 lore raw
Quoted 47 lines · On Mon, 15 Jun 2009 12:32:40 +0900 Paul Mundt <[email protected]> wrote:
Quoted 15 lines · Magnus Damm
mm/mmap.c +7 -1
@@ -1209,8 +1209,20 @@ munmap_back:
 	pgoff = vma->vm_pgoff;
 	vm_flags = vma->vm_flags;
 
-	if (vma_wants_writenotify(vma))
+	if (vma_wants_writenotify(vma)) {
+		pgprot_t pprot = vma->vm_page_prot;
+
+		/* Can vma->vm_page_prot have changed??
+		 *
+		 * Answer: Yes, drivers may have changed it in their
+		 *         f_op->mmap method.

mm/mmap.c munmap_back: · line 1228

Quoted 10 lines
mm/mmap.c +6 -0
+		 *
+		 * Ensures that vmas marked as uncached stay that way.
+		 */
 		vma->vm_page_prot = vm_get_page_prot(vm_flags & ~VM_SHARED);
+		if (pgprot_val(pprot) == pgprot_val(pgprot_noncached(pprot)))
+			vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
+	}
 
 	vma_link(mm, vma, prev, rb_link, rb_parent);
 	file = vma->vm_file;

I guess the only real issue here is that we presently have no generic interface in the kernel for setting a VMA uncached. pgprot_noncached() is the closest approximation we have, but there are still architectures that do not implement it.

Given that this comes up at least once a month, perhaps it makes sense to see which platforms are still outstanding. At least cris, h8300, m68knommu, s390, and xtensa all presently lack a definition for it. The nommu cases are easily handled, but the rest still require some attention from their architecture maintainers before we can really start treating this as a generic interface.

Until then, you will have to do what every other user of pgprot_noncached() code does in generic code:

	#ifdef pgprot_noncached
		vma->vm_page_prot = pgprot_noncached(...);
	#endif

OTOH, I guess we could just add something like:

Quoted 8 lines
	#define pgprot_noncached(x)	(x) 

which works fine for the nommu case, and which functionally is no different from what happens right now anyways for the users that don't wire it up sanely.

Arnd, what do you think about throwing this at asm-generic?

I think Arnd fell asleep ;)

Quoted 9 lines
 include/asm-generic/pgtable.h |    4 ++++
 1 file changed, 4 insertions(+)
include/asm-generic/pgtable.h +0 -0
index e410f60..e2bd73e 100644

include/asm-generic/pgtable.h static inline void ptep_set_wrprotect(struct mm_struct *mm, unsigned long addres · line 138

Quoted 10 lines
include/asm-generic/pgtable.h +4 -0
@@ -129,6 +129,10 @@ static inline void ptep_set_wrprotect(struct mm_struct *mm, unsigned long addres
 #define move_pte(pte, prot, old_addr, new_addr)	(pte)
 #endif
 
+#ifndef pgprot_noncached
+#define pgprot_noncached(prot)	(prot)
+#endif
+
 #ifndef pgprot_writecombine
 #define pgprot_writecombine pgprot_noncached
 #endif

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

maintainer
#3 lore raw
Quoted 11 lines · you
Quoted 11 lines

I guess the only real issue here is that we presently have no generic interface in the kernel for setting a VMA uncached. pgprot_noncached() is the closest approximation we have, but there are still architectures that do not implement it.

Given that this comes up at least once a month, perhaps it makes sense to see which platforms are still outstanding. At least cris, h8300, m68knommu, s390, and xtensa all presently lack a definition for it. The nommu cases are easily handled, but the rest still require some attention from their architecture maintainers before we can really start treating this as a generic interface.

For m68knommu, h8300 and s390, doing nothing is correct because they either don't have page tables or don't control caching through them. Xtensa could easily add it, they have the respective caching strategy in their page flags. On cris, caching is controlled through the high bit of the address, but I guess that means we could just add '#define _PAGE_UNCACHED (1 << 31)' there and set that for uncached.

Quoted 8 lines
Quoted 6 lines

which works fine for the nommu case, and which functionally is no different from what happens right now anyways for the users that don't wire it up sanely.

Arnd, what do you think about throwing this at asm-generic?

I think Arnd fell asleep ;)

For some reason I did not get the original mail. I've added the patch to my asm-generic queue and will send a pull request together with other patches I got.

	Arnd <><

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

maintainer
#4 lore raw

PATCH 1/1 asm-generic: add dummy pgprot_noncached()

In reply to Andrew Morton

From: Paul Mundt <[email protected]>

Most architectures now provide a pgprot_noncached(), the remaining ones can simply use an dummy default implementation, except for cris and xtensa, which should override the default appropriately.

Cc Jesper Nilsson <[email protected]>
Cc Chris Zankel <[email protected]>
Cc Magnus Damm <[email protected]>
---
 include/asm-generic/pgtable.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
include/asm-generic/pgtable.h +4 -0
index e410f60..e2bd73e 100644
@@ -129,6 +129,10 @@ static inline void ptep_set_wrprotect(struct mm_struct *mm, unsigned long addres
 #define move_pte(pte, prot, old_addr, new_addr)	(pte)
 #endif
 
+#ifndef pgprot_noncached
+#define pgprot_noncached(prot)	(prot)
+#endif
+
 #ifndef pgprot_writecombine
 #define pgprot_writecombine pgprot_noncached
 #endif
Signature · 7 lines
1.6.3.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#5 lore raw

Re: [PATCH] asm-generic: add dummy pgprot_noncached()

Quoted 11 lines · Arnd Bergmann

From: Paul Mundt <[email protected]>

Most architectures now provide a pgprot_noncached(), the remaining ones can simply use an dummy default implementation, except for cris and xtensa, which should override the default appropriately.

Cc Jesper Nilsson <[email protected]>
Cc Chris Zankel <[email protected]>
Cc Magnus Damm <[email protected]>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

maintainer
#6 lore raw

xtensa: add pgprot_noncached

In reply to Arnd Bergmann

This adds a straightforward pgprot_noncached() macro for xtensa, so we can use it in architecture independent code in the future.

arch/xtensa/include/asm/pgtable.h +3 -0
@@ -251,6 +251,9 @@ static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
 	return __pte((pte_val(pte) & _PAGE_CHG_MASK) | pgprot_val(newprot));
 }
 
+#define pgprot_noncached(prot) \
+	__pgprot((pgprot_val(prot) & ~_PAGE_CA_MASK) | _PAGE_CA_BYPASS)
+
 /*
  * Certain architectures need to do special things when pte's
  * within a page table are directly modified.  Thus, the following

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

maintainer
#7 lore raw

m32r: define pgprot_noncached

In reply to Arnd Bergmann

On m32r, pgprot_noncached is an inline function and not a macro, which means that various bits of code that check its presence with #ifdef never get to call it.
In particular, the asm-generic version of that macro would override it.

This adds a self-referencing macro like other architectures do it to make the checks work correctly.

arch/m32r/include/asm/pgtable.h +1 -0
@@ -281,6 +281,7 @@ static inline pgprot_t pgprot_noncached(pgprot_t _prot)
 	return __pgprot(prot);
 }
 
+#define pgprot_noncached(prot) pgprot_noncached(prot)
 #define pgprot_writecombine(prot) pgprot_noncached(prot)
 
 /*

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

maintainer
#8 lore raw

PATCH 1/1 cris: add pgprot_noncached

In reply to Andrew Morton

On cris, the high address bit controls caching, which means that we can add a pgprot_noncached() macro that sets this bit in the address.

---

Jesper, does this patch make any sense to you? I could well be misunderstanding how cris works.

 arch/cris/include/arch-v10/arch/mmu.h |    9 +++++----
 arch/cris/include/arch-v32/arch/mmu.h |   10 ++++++----
 arch/cris/include/asm/pgtable.h       |    2 ++
 3 files changed, 13 insertions(+), 8 deletions(-)
arch/cris/include/arch-v10/arch/mmu.h +5 -4
index df84f17..e829e5a 100644
@@ -33,10 +33,10 @@ typedef struct
 
 /* CRIS PTE bits (see R_TLB_LO in the register description)
  *
- *   Bit:  31-13 12-------4    3        2       1       0  
- *         ________________________________________________
- *        | pfn | reserved | global | valid | kernel | we  |
- *        |_____|__________|________|_______|________|_____|
+ *   Bit:  31     30-13 12-------4    3        2       1       0
+ *         _______________________________________________________
+ *        | cache |pfn | reserved | global | valid | kernel | we  |
+ *        |_______|____|__________|________|_______|________|_____|
  *
  * (pfn = physical frame number)
  */
@@ -53,6 +53,7 @@ typedef struct
 #define _PAGE_VALID	   (1<<2) /* page is valid */
 #define _PAGE_SILENT_READ  (1<<2) /* synonym */
 #define _PAGE_GLOBAL       (1<<3) /* global page - context is ignored */
+#define _PAGE_NO_CACHE	   (1<<31) /* part of the uncached memory map */
 
 /* Bits the HW doesn't care about but the kernel uses them in SW */
 
arch/cris/include/arch-v32/arch/mmu.h +6 -4
index 6bcdc3f..a05b033 100644
@@ -28,10 +28,10 @@ typedef struct
 /*
  * CRISv32 PTE bits:
  *
- *  Bit:  31-13  12-5     4        3       2        1        0
- *       +-----+------+--------+-------+--------+-------+---------+
- *       | pfn | zero | global | valid | kernel | write | execute |
- *       +-----+------+--------+-------+--------+-------+---------+
+ *  Bit:   31     30-13  12-5     4        3       2        1        0
+ *       +-------+-----+------+--------+-------+--------+-------+---------+
+ *       | cache | pfn | zero | global | valid | kernel | write | execute |
+ *       +-------+-----+------+--------+-------+--------+-------+---------+
  */
 
 /*
@@ -45,6 +45,8 @@ typedef struct
 #define _PAGE_VALID         (1 << 3)	/* Page is valid. */
 #define _PAGE_SILENT_READ   (1 << 3)	/* Same as above. */
 #define _PAGE_GLOBAL        (1 << 4)	/* Global page. */
+#define _PAGE_NO_CACHE	    (1 <<31)	/* part of the uncached memory map */
+
 
 /*
  * The hardware doesn't care about these bits, but the kernel uses them in
arch/cris/include/asm/pgtable.h +2 -0
index 50aa974..1fcce00 100644
@@ -197,6 +197,8 @@ static inline pte_t __mk_pte(void * page, pgprot_t pgprot)
 static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
 { pte_val(pte) = (pte_val(pte) & _PAGE_CHG_MASK) | pgprot_val(newprot); return pte; }
 
+#define pgprot_noncached(prot) __pgprot((pgprot_val(prot) | _PAGE_NO_CACHE))
+
 
 /* pte_val refers to a page in the 0x4xxxxxxx physical DRAM interval
  * __pte_page(pte_val) refers to the "virtual" DRAM interval

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Acked-by on v1 1/1
#9 lore raw

Re: [PATCH] cris: add pgprot_noncached

Quoted 5 lines · Arnd Bergmann

On cris, the high address bit controls caching, which means that we can add a pgprot_noncached() macro that sets this bit in the address.

Quoted 4 lines

---

Jesper, does this patch make any sense to you? I could well be misunderstanding how cris works.

No, this looks good to me.
Do you want me to grab it for the CRIS tree or do you want to keep it as a series?

Quoted 65 lines
 arch/cris/include/arch-v10/arch/mmu.h |    9 +++++----
 arch/cris/include/arch-v32/arch/mmu.h |   10 ++++++----
 arch/cris/include/asm/pgtable.h       |    2 ++
 3 files changed, 13 insertions(+), 8 deletions(-)
arch/cris/include/arch-v10/arch/mmu.h +5 -4
index df84f17..e829e5a 100644
@@ -33,10 +33,10 @@ typedef struct
 
 /* CRIS PTE bits (see R_TLB_LO in the register description)
  *
- *   Bit:  31-13 12-------4    3        2       1       0  
- *         ________________________________________________
- *        | pfn | reserved | global | valid | kernel | we  |
- *        |_____|__________|________|_______|________|_____|
+ *   Bit:  31     30-13 12-------4    3        2       1       0
+ *         _______________________________________________________
+ *        | cache |pfn | reserved | global | valid | kernel | we  |
+ *        |_______|____|__________|________|_______|________|_____|
  *
  * (pfn = physical frame number)
  */
@@ -53,6 +53,7 @@ typedef struct
 #define _PAGE_VALID	   (1<<2) /* page is valid */
 #define _PAGE_SILENT_READ  (1<<2) /* synonym */
 #define _PAGE_GLOBAL       (1<<3) /* global page - context is ignored */
+#define _PAGE_NO_CACHE	   (1<<31) /* part of the uncached memory map */
 
 /* Bits the HW doesn't care about but the kernel uses them in SW */
 
arch/cris/include/arch-v32/arch/mmu.h +6 -4
index 6bcdc3f..a05b033 100644
@@ -28,10 +28,10 @@ typedef struct
 /*
  * CRISv32 PTE bits:
  *
- *  Bit:  31-13  12-5     4        3       2        1        0
- *       +-----+------+--------+-------+--------+-------+---------+
- *       | pfn | zero | global | valid | kernel | write | execute |
- *       +-----+------+--------+-------+--------+-------+---------+
+ *  Bit:   31     30-13  12-5     4        3       2        1        0
+ *       +-------+-----+------+--------+-------+--------+-------+---------+
+ *       | cache | pfn | zero | global | valid | kernel | write | execute |
+ *       +-------+-----+------+--------+-------+--------+-------+---------+
  */
 
 /*
@@ -45,6 +45,8 @@ typedef struct
 #define _PAGE_VALID         (1 << 3)	/* Page is valid. */
 #define _PAGE_SILENT_READ   (1 << 3)	/* Same as above. */
 #define _PAGE_GLOBAL        (1 << 4)	/* Global page. */
+#define _PAGE_NO_CACHE	    (1 <<31)	/* part of the uncached memory map */
+
 
 /*
  * The hardware doesn't care about these bits, but the kernel uses them in
arch/cris/include/asm/pgtable.h +0 -0
index 50aa974..1fcce00 100644

arch/cris/include/asm/pgtable.h static inline pte_t __mk_pte(void * page, pgprot_t pgprot) · line 204

Quoted 8 lines
arch/cris/include/asm/pgtable.h +2 -0
@@ -197,6 +197,8 @@ static inline pte_t __mk_pte(void * page, pgprot_t pgprot)
 static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
 { pte_val(pte) = (pte_val(pte) & _PAGE_CHG_MASK) | pgprot_val(newprot); return pte; }
 
+#define pgprot_noncached(prot) __pgprot((pgprot_val(prot) | _PAGE_NO_CACHE))
+
 
 /* pte_val refers to a page in the 0x4xxxxxxx physical DRAM interval
  * __pte_page(pte_val) refers to the "virtual" DRAM interval

/^JN - Jesper Nilsson

Signature · 7 lines
               Jesper Nilsson -- [email protected]

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
maintainer
#10 lore raw

Re: [PATCH] cris: add pgprot_noncached

Quoted 3 lines · Jesper Nilsson

No, this looks good to me.
Do you want me to grab it for the CRIS tree or do you want to keep it as a series?

I'd prefer you to take it. The order of the four patches is entirely arbitrary anyway and there are no other dependencies on it once the asm-generic version is merged.

	Arnd <><

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#11 lore raw

Re: [PATCH] cris: add pgprot_noncached

Quoted 8 lines · Arnd Bergmann
Quoted 3 lines · Jesper Nilsson

No, this looks good to me.
Do you want me to grab it for the CRIS tree or do you want to keep it as a series?

I'd prefer you to take it. The order of the four patches is entirely arbitrary anyway and there are no other dependencies on it once the asm-generic version is merged.

Right, I'll add it the CRIS tree. Thanks!

Quoted 1 line
	Arnd <><

/^JN - Jesper Nilsson

Signature · 7 lines
               Jesper Nilsson -- [email protected]

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>