mm: introduce VM_MIXEDMAP

PATCH In review [email protected] 5 participants 25 messages Jan 18, 2008 – Mar 4, 2008 1 message not in the archive

Subsystems (7)

Memory management VFS MM core Generic include/asm header files +3 more subsystems

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

[email protected]

author

2008-01-18 04:56 UTC #1 lore raw

PATCH 1/6 mm: introduce VM_MIXEDMAP

Introduce a new type of mapping, VM_MIXEDMAP. This is unlike VM_PFNMAP in that it can support COW mappings of arbitrary ranges including ranges without struct page (PFNMAP can only support COW in those cases where the un-COW-ed translations are mapped linearly in the virtual address).

VM_MIXEDMAP achieves this by refcounting all pfn_valid pages, and not refcounting !pfn_valid pages (which is not an option for VM_PFNMAP, because it needs to avoid refcounting pfn_valid pages eg. for /dev/mem mappings).

(Needs Jared's SOB)

Signed-off-by Nick Piggin <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Hugh Dickins <[email protected]>

Cc Jared Hulbert <[email protected]>

Cc Carsten Otte <[email protected]>

Cc Martin Schwidefsky <[email protected]>

Cc Heiko Carstens <[email protected]>

Cc [email protected]

---
 include/linux/mm.h |    1 
 mm/memory.c        |   79 ++++++++++++++++++++++++++++++++++++++---------------
 2 files changed, 59 insertions(+), 21 deletions(-)

include/linux/mm.h +1 -0

Index: linux-2.6/include/linux/mm.h
===================================================================
@@ -106,6 +106,7 @@ extern unsigned int kobjsize(const void 
 #define VM_ALWAYSDUMP	0x04000000	/* Always include in core dumps */
 
 #define VM_CAN_NONLINEAR 0x08000000	/* Has ->fault & does nonlinear pages */
+#define VM_MIXEDMAP	0x10000000	/* Can contain "struct page" and pure PFN pages */
 
 #ifndef VM_STACK_DEFAULT_FLAGS		/* arch can override this */
 #define VM_STACK_DEFAULT_FLAGS VM_DATA_DEFAULT_FLAGS

mm/memory.c +58 ~~-21~~

117 lines not on this page · raw message

Signature · 5 lines

--
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>

[email protected]

author

2008-01-18 04:56 UTC #2 lore raw

PATCH 2/6 mm: introduce pte_special pte bit

In reply to a message not in the archive

s390 for one, cannot implement VM_MIXEDMAP with pfn_valid, due to their memory model (which is more dynamic than most). Instead, they had proposed to implement it with an additional path through vm_normal_page(), using a bit in the pte to determine whether or not the page should be refcounted:

vm_normal_page()

{
	...
        if (unlikely(vma->vm_flags & (VM_PFNMAP|VM_MIXEDMAP))) {
                if (vma->vm_flags & VM_MIXEDMAP) {
#ifdef s390
			if (!mixedmap_refcount_pte(pte))
				return NULL;
#else
                        if (!pfn_valid(pfn))
                                return NULL;
#endif
                        goto out;
                }
	...
}

This is fine, however if we are allowed to use a bit in the pte to determine refcountedness, we can use that to _completely_ replace all the vma based schemes. So instead of adding more cases to the already complex vma-based scheme, we can have a clearly seperate and simple pte-based scheme (and get slightly better code generation in the process):

vm_normal_page()
{

#ifdef s390
	if (!mixedmap_refcount_pte(pte))
		return NULL;
	return pte_page(pte);
#else
	...

#endif
}

And finally, we may rather make this concept usable by any architecture rather than making it s390 only, so implement a new type of pte state for this. Unfortunately the old vma based code must stay, because some architectures may not be able to spare pte bits. This makes vm_normal_page a little bit more ugly than we would like, but the 2 cases are clearly seperate.

So introduce a pte_special pte state, and use it in mm/memory.c. It is currently a noop for all architectures, so this doesn't actually result in any compiled code changes to mm/memory.o.

Signed-off-by Nick Piggin <[email protected]>

Cc Linus Torvalds <torvalds-de/[email protected]>

Cc Hugh Dickins <[email protected]>

Cc Jared Hulbert <[email protected]>

Cc Carsten Otte <[email protected]>

Cc Martin Schwidefsky <[email protected]>

Cc Heiko Carstens <[email protected]>

Cc [email protected]

---
 include/asm-alpha/pgtable.h         |    2 
 include/asm-avr32/pgtable.h         |    8 ++
 include/asm-cris/pgtable.h          |    2 
 include/asm-frv/pgtable.h           |    2 
 include/asm-ia64/pgtable.h          |    3 +
 include/asm-m32r/pgtable.h          |   10 +++
 include/asm-m68k/motorola_pgtable.h |    2 
 include/asm-m68k/sun3_pgtable.h     |    2 
 include/asm-mips/pgtable.h          |    2 
 include/asm-parisc/pgtable.h        |    2 
 include/asm-powerpc/pgtable-ppc32.h |    3 +
 include/asm-powerpc/pgtable-ppc64.h |    3 +
 include/asm-ppc/pgtable.h           |    3 +
 include/asm-s390/pgtable.h          |   10 +++
 include/asm-sh64/pgtable.h          |    2 
 include/asm-sparc/pgtable.h         |    7 ++
 include/asm-sparc64/pgtable.h       |   10 +++
 include/asm-um/pgtable.h            |   10 +++
 include/asm-x86/pgtable_32.h        |    2 
 include/asm-x86/pgtable_64.h        |    2 
 include/asm-xtensa/pgtable.h        |    4 +
 include/linux/mm.h                  |    3 -
 mm/memory.c                         |   98 +++++++++++++++++++-----------------
 23 files changed, 147 insertions(+), 45 deletions(-)

include/asm-um/pgtable.h +10 -0

Index: linux-2.6/include/asm-um/pgtable.h
===================================================================
@@ -220,6 +220,11 @@ static inline int pte_newprot(pte_t pte)
 	return(pte_present(pte) && (pte_get_bits(pte, _PAGE_NEWPROT)));
 }
 
+static inline int pte_special(pte_t pte)
+{
+	return 0;
+}
+
 /*
  * =================================
  * Flags setting section.

@@ -288,6 +293,11 @@ static inline pte_t pte_mknewpage(pte_t 
 	return(pte);
 }
 
+static inline pte_t pte_mkspecial(pte_t pte)
+{
+	return(pte);
+}
+
 static inline void set_pte(pte_t *pteptr, pte_t pteval)
 {
 	pte_copy(*pteptr, pteval);

include/asm-x86/pgtable_32.h +2 -0

Index: linux-2.6/include/asm-x86/pgtable_32.h
===================================================================
@@ -219,6 +219,7 @@ static inline int pte_dirty(pte_t pte)		
 static inline int pte_young(pte_t pte)		{ return (pte).pte_low & _PAGE_ACCESSED; }
 static inline int pte_write(pte_t pte)		{ return (pte).pte_low & _PAGE_RW; }
 static inline int pte_huge(pte_t pte)		{ return (pte).pte_low & _PAGE_PSE; }
+static inline int pte_special(pte_t pte)	{ return 0; }
 
 /*
  * The following only works if pte_present() is not true.

@@ -232,6 +233,7 @@ static inline pte_t pte_mkdirty(pte_t pt
 static inline pte_t pte_mkyoung(pte_t pte)	{ (pte).pte_low |= _PAGE_ACCESSED; return pte; }
 static inline pte_t pte_mkwrite(pte_t pte)	{ (pte).pte_low |= _PAGE_RW; return pte; }
 static inline pte_t pte_mkhuge(pte_t pte)	{ (pte).pte_low |= _PAGE_PSE; return pte; }
+static inline pte_t pte_mkspecial(pte_t pte)	{ return pte; }
 
 #ifdef CONFIG_X86_PAE
 # include <asm/pgtable-3level.h>

include/asm-x86/pgtable_64.h +2 -0

Index: linux-2.6/include/asm-x86/pgtable_64.h
===================================================================
@@ -272,6 +272,7 @@ static inline int pte_young(pte_t pte)		
 static inline int pte_write(pte_t pte)		{ return pte_val(pte) & _PAGE_RW; }
 static inline int pte_file(pte_t pte)		{ return pte_val(pte) & _PAGE_FILE; }
 static inline int pte_huge(pte_t pte)		{ return pte_val(pte) & _PAGE_PSE; }
+static inline int pte_special(pte_t pte)	{ return 0; }
 
 static inline pte_t pte_mkclean(pte_t pte)	{ set_pte(&pte, __pte(pte_val(pte) & ~_PAGE_DIRTY)); return pte; }
 static inline pte_t pte_mkold(pte_t pte)	{ set_pte(&pte, __pte(pte_val(pte) & ~_PAGE_ACCESSED)); return pte; }

@@ -282,6 +283,7 @@ static inline pte_t pte_mkyoung(pte_t pt
 static inline pte_t pte_mkwrite(pte_t pte)	{ set_pte(&pte, __pte(pte_val(pte) | _PAGE_RW)); return pte; }
 static inline pte_t pte_mkhuge(pte_t pte)	{ set_pte(&pte, __pte(pte_val(pte) | _PAGE_PSE)); return pte; }
 static inline pte_t pte_clrhuge(pte_t pte)	{ set_pte(&pte, __pte(pte_val(pte) & ~_PAGE_PSE)); return pte; }
+static inline pte_t pte_mkspecial(pte_t pte)	{ return pte; }
 
 struct vm_area_struct;

include/linux/mm.h +2 -1

Index: linux-2.6/include/linux/mm.h
===================================================================
@@ -699,7 +699,8 @@ struct zap_details {
 	unsigned long truncate_count;		/* Compare vm_truncate_count */
 };
 
-struct page *vm_normal_page(struct vm_area_struct *, unsigned long, pte_t);
+struct page *vm_normal_page(struct vm_area_struct *vma, unsigned long addr, pte_t pte);
+
 unsigned long zap_page_range(struct vm_area_struct *vma, unsigned long address,
 		unsigned long size, struct zap_details *);
 unsigned long unmap_vmas(struct mmu_gather **tlb,

mm/memory.c +54 ~~-44~~

156 lines not on this page · raw message

include/asm-alpha/pgtable.h +2 -0

Index: linux-2.6/include/asm-alpha/pgtable.h
===================================================================
@@ -268,6 +268,7 @@ extern inline int pte_write(pte_t pte)		
 extern inline int pte_dirty(pte_t pte)		{ return pte_val(pte) & _PAGE_DIRTY; }
 extern inline int pte_young(pte_t pte)		{ return pte_val(pte) & _PAGE_ACCESSED; }
 extern inline int pte_file(pte_t pte)		{ return pte_val(pte) & _PAGE_FILE; }
+extern inline int pte_special(pte_t pte)	{ return 0; }
 
 extern inline pte_t pte_wrprotect(pte_t pte)	{ pte_val(pte) |= _PAGE_FOW; return pte; }
 extern inline pte_t pte_mkclean(pte_t pte)	{ pte_val(pte) &= ~(__DIRTY_BITS); return pte; }

@@ -275,6 +276,7 @@ extern inline pte_t pte_mkold(pte_t pte)
 extern inline pte_t pte_mkwrite(pte_t pte)	{ pte_val(pte) &= ~_PAGE_FOW; return pte; }
 extern inline pte_t pte_mkdirty(pte_t pte)	{ pte_val(pte) |= __DIRTY_BITS; return pte; }
 extern inline pte_t pte_mkyoung(pte_t pte)	{ pte_val(pte) |= __ACCESS_BITS; return pte; }
+extern inline pte_t pte_mkspecial(pte_t pte)	{ return pte; }
 
 #define PAGE_DIR_OFFSET(tsk,address) pgd_offset((tsk),(address))

include/asm-avr32/pgtable.h +8 -0

Index: linux-2.6/include/asm-avr32/pgtable.h
===================================================================
@@ -211,6 +211,10 @@ static inline int pte_young(pte_t pte)
 {
 	return pte_val(pte) & _PAGE_ACCESSED;
 }
+static inline int pte_special(pte_t pte)
+{
+	return 0;
+}
 
 /*
  * The following only work if pte_present() is not true.

@@ -251,6 +255,10 @@ static inline pte_t pte_mkyoung(pte_t pt
 	set_pte(&pte, __pte(pte_val(pte) | _PAGE_ACCESSED));
 	return pte;
 }
+static inline pte_t pte_mkspecial(pte_t pte)
+{
+	return pte;
+}
 
 #define pmd_none(x)	(!pmd_val(x))
 #define pmd_present(x)	(pmd_val(x) & _PAGE_PRESENT)

include/asm-cris/pgtable.h +2 -0

Index: linux-2.6/include/asm-cris/pgtable.h
===================================================================
@@ -115,6 +115,7 @@ static inline int pte_write(pte_t pte)  
 static inline int pte_dirty(pte_t pte)          { return pte_val(pte) & _PAGE_MODIFIED; }
 static inline int pte_young(pte_t pte)          { return pte_val(pte) & _PAGE_ACCESSED; }
 static inline int pte_file(pte_t pte)           { return pte_val(pte) & _PAGE_FILE; }
+static inline int pte_special(pte_t pte)	{ return 0; }
 
 static inline pte_t pte_wrprotect(pte_t pte)
 {

@@ -162,6 +163,7 @@ static inline pte_t pte_mkyoung(pte_t pt
         }
         return pte;
 }
+static inline pte_t pte_mkspecial(pte_t pte)	{ return pte; }
 
 /*
  * Conversion functions: convert a page and protection to a page entry,

include/asm-frv/pgtable.h +2 -0

Index: linux-2.6/include/asm-frv/pgtable.h
===================================================================
@@ -380,6 +380,7 @@ static inline pmd_t *pmd_offset(pud_t *d
 static inline int pte_dirty(pte_t pte)		{ return (pte).pte & _PAGE_DIRTY; }
 static inline int pte_young(pte_t pte)		{ return (pte).pte & _PAGE_ACCESSED; }
 static inline int pte_write(pte_t pte)		{ return !((pte).pte & _PAGE_WP); }
+static inline int pte_special(pte_t pte)	{ return 0; }
 
 static inline pte_t pte_mkclean(pte_t pte)	{ (pte).pte &= ~_PAGE_DIRTY; return pte; }
 static inline pte_t pte_mkold(pte_t pte)	{ (pte).pte &= ~_PAGE_ACCESSED; return pte; }

@@ -387,6 +388,7 @@ static inline pte_t pte_wrprotect(pte_t 
 static inline pte_t pte_mkdirty(pte_t pte)	{ (pte).pte |= _PAGE_DIRTY; return pte; }
 static inline pte_t pte_mkyoung(pte_t pte)	{ (pte).pte |= _PAGE_ACCESSED; return pte; }
 static inline pte_t pte_mkwrite(pte_t pte)	{ (pte).pte &= ~_PAGE_WP; return pte; }
+static inline pte_t pte_mkspecial(pte_t pte)	{ return pte; }
 
 static inline int ptep_test_and_clear_young(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep)
 {

include/asm-ia64/pgtable.h +3 -0

Index: linux-2.6/include/asm-ia64/pgtable.h
===================================================================
@@ -302,6 +302,8 @@ ia64_phys_addr_valid (unsigned long addr
 #define pte_dirty(pte)		((pte_val(pte) & _PAGE_D) != 0)
 #define pte_young(pte)		((pte_val(pte) & _PAGE_A) != 0)
 #define pte_file(pte)		((pte_val(pte) & _PAGE_FILE) != 0)
+#define pte_special(pte)	0
+
 /*
  * Note: we convert AR_RWX to AR_RX and AR_RW to AR_R by clearing the 2nd bit in the
  * access rights:

@@ -313,6 +315,7 @@ ia64_phys_addr_valid (unsigned long addr
 #define pte_mkclean(pte)	(__pte(pte_val(pte) & ~_PAGE_D))
 #define pte_mkdirty(pte)	(__pte(pte_val(pte) | _PAGE_D))
 #define pte_mkhuge(pte)		(__pte(pte_val(pte)))
+#define pte_mkspecial(pte)	(pte)
 
 /*
  * Because ia64's Icache and Dcache is not coherent (on a cpu), we need to

include/asm-m32r/pgtable.h +10 -0

Index: linux-2.6/include/asm-m32r/pgtable.h
===================================================================
@@ -214,6 +214,11 @@ static inline int pte_file(pte_t pte)
 	return pte_val(pte) & _PAGE_FILE;
 }
 
+static inline int pte_special(pte_t pte)
+{
+	return 0;
+}
+
 static inline pte_t pte_mkclean(pte_t pte)
 {
 	pte_val(pte) &= ~_PAGE_DIRTY;

@@ -250,6 +255,11 @@ static inline pte_t pte_mkwrite(pte_t pt
 	return pte;
 }
 
+static inline pte_t pte_mkspecial(pte_t pte)
+{
+	return pte;
+}
+
 static inline  int ptep_test_and_clear_young(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep)
 {
 	return test_and_clear_bit(_PAGE_BIT_ACCESSED, ptep);

include/asm-m68k/motorola_pgtable.h +2 -0

Index: linux-2.6/include/asm-m68k/motorola_pgtable.h
===================================================================
@@ -168,6 +168,7 @@ static inline int pte_write(pte_t pte)		
 static inline int pte_dirty(pte_t pte)		{ return pte_val(pte) & _PAGE_DIRTY; }
 static inline int pte_young(pte_t pte)		{ return pte_val(pte) & _PAGE_ACCESSED; }
 static inline int pte_file(pte_t pte)		{ return pte_val(pte) & _PAGE_FILE; }
+static inline int pte_special(pte_t pte)	{ return 0; }
 
 static inline pte_t pte_wrprotect(pte_t pte)	{ pte_val(pte) |= _PAGE_RONLY; return pte; }
 static inline pte_t pte_mkclean(pte_t pte)	{ pte_val(pte) &= ~_PAGE_DIRTY; return pte; }

@@ -185,6 +186,7 @@ static inline pte_t pte_mkcache(pte_t pt
 	pte_val(pte) = (pte_val(pte) & _CACHEMASK040) | m68k_supervisor_cachemode;
 	return pte;
 }
+static inline pte_t pte_mkspecial(pte_t pte)	{ return pte; }
 
 #define PAGE_DIR_OFFSET(tsk,address) pgd_offset((tsk),(address))

include/asm-m68k/sun3_pgtable.h +2 -0

Index: linux-2.6/include/asm-m68k/sun3_pgtable.h
===================================================================
@@ -169,6 +169,7 @@ static inline int pte_write(pte_t pte)		
 static inline int pte_dirty(pte_t pte)		{ return pte_val(pte) & SUN3_PAGE_MODIFIED; }
 static inline int pte_young(pte_t pte)		{ return pte_val(pte) & SUN3_PAGE_ACCESSED; }
 static inline int pte_file(pte_t pte)		{ return pte_val(pte) & SUN3_PAGE_ACCESSED; }
+static inline int pte_special(pte_t pte)	{ return 0; }
 
 static inline pte_t pte_wrprotect(pte_t pte)	{ pte_val(pte) &= ~SUN3_PAGE_WRITEABLE; return pte; }
 static inline pte_t pte_mkclean(pte_t pte)	{ pte_val(pte) &= ~SUN3_PAGE_MODIFIED; return pte; }

@@ -181,6 +182,7 @@ static inline pte_t pte_mknocache(pte_t 
 //static inline pte_t pte_mkcache(pte_t pte)	{ pte_val(pte) &= SUN3_PAGE_NOCACHE; return pte; }
 // until then, use:
 static inline pte_t pte_mkcache(pte_t pte)	{ return pte; }
+static inline pte_t pte_mkspecial(pte_t pte)	{ return pte; }
 
 extern pgd_t swapper_pg_dir[PTRS_PER_PGD];
 extern pgd_t kernel_pg_dir[PTRS_PER_PGD];

include/asm-mips/pgtable.h +2 -0

Index: linux-2.6/include/asm-mips/pgtable.h
===================================================================
@@ -285,6 +285,8 @@ static inline pte_t pte_mkyoung(pte_t pt
 	return pte;
 }
 #endif
+static inline int pte_special(pte_t pte)	{ return 0; }
+static inline pte_t pte_mkspecial(pte_t pte)	{ return pte; }
 
 /*
  * Macro to make mark a page protection value as "uncacheable".  Note

include/asm-parisc/pgtable.h +2 -0

Index: linux-2.6/include/asm-parisc/pgtable.h
===================================================================
@@ -331,6 +331,7 @@ static inline int pte_dirty(pte_t pte)		
 static inline int pte_young(pte_t pte)		{ return pte_val(pte) & _PAGE_ACCESSED; }
 static inline int pte_write(pte_t pte)		{ return pte_val(pte) & _PAGE_WRITE; }
 static inline int pte_file(pte_t pte)		{ return pte_val(pte) & _PAGE_FILE; }
+static inline int pte_special(pte_t pte)	{ return 0; }
 
 static inline pte_t pte_mkclean(pte_t pte)	{ pte_val(pte) &= ~_PAGE_DIRTY; return pte; }
 static inline pte_t pte_mkold(pte_t pte)	{ pte_val(pte) &= ~_PAGE_ACCESSED; return pte; }

@@ -338,6 +339,7 @@ static inline pte_t pte_wrprotect(pte_t 
 static inline pte_t pte_mkdirty(pte_t pte)	{ pte_val(pte) |= _PAGE_DIRTY; return pte; }
 static inline pte_t pte_mkyoung(pte_t pte)	{ pte_val(pte) |= _PAGE_ACCESSED; return pte; }
 static inline pte_t pte_mkwrite(pte_t pte)	{ pte_val(pte) |= _PAGE_WRITE; return pte; }
+static inline pte_t pte_mkspecial(pte_t pte)	{ return pte; }
 
 /*
  * Conversion functions: convert a page and protection to a page entry,

include/asm-powerpc/pgtable-ppc32.h +3 -0

Index: linux-2.6/include/asm-powerpc/pgtable-ppc32.h
===================================================================
@@ -514,6 +514,7 @@ static inline int pte_write(pte_t pte)		
 static inline int pte_dirty(pte_t pte)		{ return pte_val(pte) & _PAGE_DIRTY; }
 static inline int pte_young(pte_t pte)		{ return pte_val(pte) & _PAGE_ACCESSED; }
 static inline int pte_file(pte_t pte)		{ return pte_val(pte) & _PAGE_FILE; }
+static inline int pte_special(pte_t pte)	{ return 0; }
 
 static inline void pte_uncache(pte_t pte)       { pte_val(pte) |= _PAGE_NO_CACHE; }
 static inline void pte_cache(pte_t pte)         { pte_val(pte) &= ~_PAGE_NO_CACHE; }

@@ -531,6 +532,8 @@ static inline pte_t pte_mkdirty(pte_t pt
 	pte_val(pte) |= _PAGE_DIRTY; return pte; }
 static inline pte_t pte_mkyoung(pte_t pte) {
 	pte_val(pte) |= _PAGE_ACCESSED; return pte; }
+static inline pte_t pte_mkspecial(pte_t pte) {
+	return pte; }
 
 static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
 {

include/asm-powerpc/pgtable-ppc64.h +3 -0

Index: linux-2.6/include/asm-powerpc/pgtable-ppc64.h
===================================================================
@@ -239,6 +239,7 @@ static inline int pte_write(pte_t pte) {
 static inline int pte_dirty(pte_t pte) { return pte_val(pte) & _PAGE_DIRTY;}
 static inline int pte_young(pte_t pte) { return pte_val(pte) & _PAGE_ACCESSED;}
 static inline int pte_file(pte_t pte) { return pte_val(pte) & _PAGE_FILE;}
+static inline int pte_special(pte_t pte) { return 0; }
 
 static inline void pte_uncache(pte_t pte) { pte_val(pte) |= _PAGE_NO_CACHE; }
 static inline void pte_cache(pte_t pte)   { pte_val(pte) &= ~_PAGE_NO_CACHE; }

@@ -257,6 +258,8 @@ static inline pte_t pte_mkyoung(pte_t pt
 	pte_val(pte) |= _PAGE_ACCESSED; return pte; }
 static inline pte_t pte_mkhuge(pte_t pte) {
 	return pte; }
+static inline pte_t pte_mkspecial(pte_t pte) {
+	return pte; }
 
 /* Atomic PTE updates */
 static inline unsigned long pte_update(struct mm_struct *mm,

include/asm-ppc/pgtable.h +3 -0

Index: linux-2.6/include/asm-ppc/pgtable.h
===================================================================
@@ -537,6 +537,7 @@ static inline int pte_write(pte_t pte)		
 static inline int pte_dirty(pte_t pte)		{ return pte_val(pte) & _PAGE_DIRTY; }
 static inline int pte_young(pte_t pte)		{ return pte_val(pte) & _PAGE_ACCESSED; }
 static inline int pte_file(pte_t pte)		{ return pte_val(pte) & _PAGE_FILE; }
+static inline int pte_special(pte_t pte)	{ return 0; }
 
 static inline void pte_uncache(pte_t pte)       { pte_val(pte) |= _PAGE_NO_CACHE; }
 static inline void pte_cache(pte_t pte)         { pte_val(pte) &= ~_PAGE_NO_CACHE; }

@@ -554,6 +555,8 @@ static inline pte_t pte_mkdirty(pte_t pt
 	pte_val(pte) |= _PAGE_DIRTY; return pte; }
 static inline pte_t pte_mkyoung(pte_t pte) {
 	pte_val(pte) |= _PAGE_ACCESSED; return pte; }
+static inline pte_t pte_mkspecial(pte_t pte) {
+	return pte; }
 
 static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
 {

include/asm-s390/pgtable.h +10 -0

Index: linux-2.6/include/asm-s390/pgtable.h
===================================================================
@@ -504,6 +504,11 @@ static inline int pte_file(pte_t pte)
 	return (pte_val(pte) & mask) == _PAGE_TYPE_FILE;
 }
 
+static inline int pte_special(pte_t pte)
+{
+	return 0;
+}
+
 #define __HAVE_ARCH_PTE_SAME
 #define pte_same(a,b)  (pte_val(a) == pte_val(b))

@@ -654,6 +659,11 @@ static inline pte_t pte_mkyoung(pte_t pt
 	return pte;
 }
 
+static inline pte_t pte_mkspecial(pte_t pte)
+{
+	return pte;
+}
+
 #define __HAVE_ARCH_PTEP_TEST_AND_CLEAR_YOUNG
 static inline int ptep_test_and_clear_young(struct vm_area_struct *vma,
 					    unsigned long addr, pte_t *ptep)

include/asm-sh64/pgtable.h +2 -0

Index: linux-2.6/include/asm-sh64/pgtable.h
===================================================================
@@ -419,6 +419,7 @@ static inline int pte_dirty(pte_t pte){ 
 static inline int pte_young(pte_t pte){ return pte_val(pte) & _PAGE_ACCESSED; }
 static inline int pte_file(pte_t pte) { return pte_val(pte) & _PAGE_FILE; }
 static inline int pte_write(pte_t pte){ return pte_val(pte) & _PAGE_WRITE; }
+static inline int pte_special(pte_t pte)	{ return 0; }
 
 static inline pte_t pte_wrprotect(pte_t pte)	{ set_pte(&pte, __pte(pte_val(pte) & ~_PAGE_WRITE)); return pte; }
 static inline pte_t pte_mkclean(pte_t pte)	{ set_pte(&pte, __pte(pte_val(pte) & ~_PAGE_DIRTY)); return pte; }

@@ -427,6 +428,7 @@ static inline pte_t pte_mkwrite(pte_t pt
 static inline pte_t pte_mkdirty(pte_t pte)	{ set_pte(&pte, __pte(pte_val(pte) | _PAGE_DIRTY)); return pte; }
 static inline pte_t pte_mkyoung(pte_t pte)	{ set_pte(&pte, __pte(pte_val(pte) | _PAGE_ACCESSED)); return pte; }
 static inline pte_t pte_mkhuge(pte_t pte)	{ set_pte(&pte, __pte(pte_val(pte) | _PAGE_SZHUGE)); return pte; }
+static inline pte_t pte_mkspecial(pte_t pte)	{ return pte; }
 
 
 /*

include/asm-sparc/pgtable.h +7 -0

Index: linux-2.6/include/asm-sparc/pgtable.h
===================================================================
@@ -219,6 +219,11 @@ static inline int pte_file(pte_t pte)
 	return pte_val(pte) & BTFIXUP_HALF(pte_filei);
 }
 
+static inline int pte_special(pte_t pte)
+{
+	return 0;
+}
+
 /*
  */
 BTFIXUPDEF_HALF(pte_wrprotecti)

@@ -251,6 +256,8 @@ BTFIXUPDEF_CALL_CONST(pte_t, pte_mkyoung
 #define pte_mkdirty(pte) BTFIXUP_CALL(pte_mkdirty)(pte)
 #define pte_mkyoung(pte) BTFIXUP_CALL(pte_mkyoung)(pte)
 
+#define pte_mkspecial(pte_t pte)    (pte)
+
 #define pfn_pte(pfn, prot)		mk_pte(pfn_to_page(pfn), prot)
 
 BTFIXUPDEF_CALL(unsigned long,	 pte_pfn, pte_t)

include/asm-sparc64/pgtable.h +10 -0

Index: linux-2.6/include/asm-sparc64/pgtable.h
===================================================================
@@ -506,6 +506,11 @@ static inline pte_t pte_mkyoung(pte_t pt
 	return __pte(pte_val(pte) | mask);
 }
 
+static inline pte_t pte_mkspecial(pte_t pte)
+{
+	return pte;
+}
+
 static inline unsigned long pte_young(pte_t pte)
 {
 	unsigned long mask;

@@ -608,6 +613,11 @@ static inline unsigned long pte_present(
 	return val;
 }
 
+static inline int pte_special(pte_t pte)
+{
+	return 0;
+}
+
 #define pmd_set(pmdp, ptep)	\
 	(pmd_val(*(pmdp)) = (__pa((unsigned long) (ptep)) >> 11UL))
 #define pud_set(pudp, pmdp)	\

include/asm-xtensa/pgtable.h +4 -0

Index: linux-2.6/include/asm-xtensa/pgtable.h
===================================================================
@@ -212,6 +212,8 @@ static inline int pte_write(pte_t pte) {
 static inline int pte_dirty(pte_t pte) { return pte_val(pte) & _PAGE_DIRTY; }
 static inline int pte_young(pte_t pte) { return pte_val(pte) & _PAGE_ACCESSED; }
 static inline int pte_file(pte_t pte)  { return pte_val(pte) & _PAGE_FILE; }
+static inline int pte_special(pte_t pte) { return 0; }
+
 static inline pte_t pte_wrprotect(pte_t pte)	
 	{ pte_val(pte) &= ~(_PAGE_WRITABLE | _PAGE_HW_WRITE); return pte; }
 static inline pte_t pte_mkclean(pte_t pte)

@@ -224,6 +226,8 @@ static inline pte_t pte_mkyoung(pte_t pt
 	{ pte_val(pte) |= _PAGE_ACCESSED; return pte; }
 static inline pte_t pte_mkwrite(pte_t pte)
 	{ pte_val(pte) |= _PAGE_WRITABLE; return pte; }
+static inline pte_t pte_mkspecial(pte_t pte)
+	{ return pte; }
 
 /*
  * Conversion functions: convert a page and protection to a page entry,

Signature · 4 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-arch" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Linus Torvalds

2008-01-18 16:41 UTC #3 lore raw

Re: [patch 2/6] mm: introduce pte_special pte bit

mm/memory.c static inline int is_cow_mapping(unsigne · line 414

Quoted 12 lines · On Fri, 18 Jan 2008, [email protected] wrote:

mm/memory.c +8 -1

+#ifdef __HAVE_ARCH_PTE_SPECIAL
+# define HAVE_PTE_SPECIAL 1
+#else
+# define HAVE_PTE_SPECIAL 0
+#endif
 struct page *vm_normal_page(struct vm_area_struct *vma, unsigned long addr, pte_t pte)
 {
-	unsigned long pfn = pte_pfn(pte);
+	unsigned long pfn;
+
+	if (HAVE_PTE_SPECIAL) {

I really don't think this is *any* different from "#ifdefs in code".

#ifdef's in code is not about syntax, it's about abstraction. This is still the exact same thing as having an #ifdef around it, and in many ways it is *worse*, because now it's just made to look somewhat different with a particularly ugly #ifdef.

IOW, this didn't abstract the issue away, it just massaged it to look different.

I suspect that the nicest abstraction would be to simply make the whole function be a per-architecture thing. Not exposing a "pte_special()" bit at all, but instead having the interface simply be:

- create special entries:
pte_t pte_mkspecial(pte_t pte)

- check if an entry is special:
struct page *vm_normal_page(vma, addr, pte)

and now it's not while the naming is a bit odd (for historical reasons), at least it is properly *abstracted* and you don't have any #ifdef's in code (and we'd probably need to extend that abstraction then for the "locklessly look up page" case eventually).

[ To make it slightly more regular, we could make "pte_mkspecial()" take
the vma/addr thing too, even though it would never really use it except to perhaps have a VM_BUG_ON() that it only happens within XIP/PFNMAP vma's.

The "pte_mkspecial()" definitely has more to to with "vm_normal_page()" than with the other "pte_mkxyzzy()" functions, so it really might make sense to instead make the thing

void set_special_page(vma, addr, pte_t *, pfn, pgprot)

because it is never acceptable to do "pte_mkspecial()" on any existent PTE *anyway*, so we might as well make the interface reflect that: it's not that you make a pte "special", it's that you insert a special page into the VM.

So the operation really conceptually has more to do with "set_pte()" than with "pte_mkxxx()", no? ]

Then, just have a library version of the long form, and make architectures that don't support it just use that (just so that you don't have to duplicate that silly thing). So an architecture that support special page flags would do somethiing like

	#define set_special_page(vma,addr,ptep,pfn,prot) \
		set_pte_at(vma, addr, ptep, mk_special_pte(pfn,prot))
	#define vm_normal_page(vma,addr,pte)
		(pte_special(pte) ? NULL : pte_page(pte))

and other architectures would just do

	#define set_special_page(vma,addr,ptep,pfn,prot) \
		set_pte_at(vma, addr, ptep, mk_pte(pfn,prot))
	#define vm_normal_page(vma,addr,pte) \
		generic_vm_normal_page(vma,addr,pte)

or something.

THAT is what I mean by "no #ifdef's in code" - that the selection is done at a higher level, the same way we have good interfaces with clear *conceptual* meaning for all the other PTE accessing stuff, rather than have conditionals in the architecture-independent code.

It's not about syntax - it's about having good conceptual abstractions.

Linus

--
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>

Sam Ravnborg

2008-01-18 18:04 UTC #4 lore raw

Re: [patch 2/6] mm: introduce pte_special pte bit

Quoted 17 lines · Linus Torvalds

mm/memory.c static inline int is_cow_mapping(unsigne · line 414
Quoted 12 lines · On Fri, 18 Jan 2008, [email protected] wrote:
*/
mm/memory.c +8 -1
+#ifdef __HAVE_ARCH_PTE_SPECIAL
+# define HAVE_PTE_SPECIAL 1
+#else
+# define HAVE_PTE_SPECIAL 0
+#endif
 struct page *vm_normal_page(struct vm_area_struct *vma, unsigned long addr, pte_t pte)
 {
-	unsigned long pfn = pte_pfn(pte);
+	unsigned long pfn;
+
+	if (HAVE_PTE_SPECIAL) {
I really don't think this is *any* different from "#ifdefs in code".

One fundamental difference is that with the above syntax we always compile both versions of the code - so we do not end up with one version that builds and another version that dont.

This has always striked me as a good reason to do the above and I think it is busybox that does so with success.

Sam

Linus Torvalds

2008-01-18 18:28 UTC #5 lore raw

Re: [patch 2/6] mm: introduce pte_special pte bit

Quoted 4 lines · Sam Ravnborg

One fundamental difference is that with the above syntax we always compile both versions of the code - so we do not end up with one version that builds and another version that dont.

Yes, in that sense it tends to be better to use C language constructs over preprocessor constructs, since error diagnostics and syntax checking is improved.

So yeah, I'll give you that it can be an improvement. It's just not what I was really hoping for.

Linus

Sam Ravnborg

2008-01-18 18:53 UTC #6 lore raw

Re: [patch 2/6] mm: introduce pte_special pte bit

Quoted 8 lines · Linus Torvalds

Quoted 4 lines · Sam Ravnborg
One fundamental difference is that with the above syntax we always compile both versions of the code - so we do not end up with one version that builds and another version that dont.

Quoted 6 lines

Yes, in that sense it tends to be better to use C language constructs over preprocessor constructs, since error diagnostics and syntax checking is improved.
So yeah, I'll give you that it can be an improvement. It's just not what I was really hoping for.

Just to clarify - my comment was solely related to the usage
of if (HAVE_*) versus #ifdef.
I had nothing to do with the actual discussion which I do not try to follw .

Sam

Nick Piggin

author

2008-01-18 22:46 UTC #7 lore raw

Re: [patch 2/6] mm: introduce pte_special pte bit

In reply to Linus Torvalds

Quoted 18 lines · Linus Torvalds

mm/memory.c static inline int is_cow_mapping(unsigne · line 414
Quoted 12 lines · On Fri, 18 Jan 2008, [email protected] wrote:
*/
mm/memory.c +8 -1
+#ifdef __HAVE_ARCH_PTE_SPECIAL
+# define HAVE_PTE_SPECIAL 1
+#else
+# define HAVE_PTE_SPECIAL 0
+#endif
 struct page *vm_normal_page(struct vm_area_struct *vma, unsigned long addr, pte_t pte)
 {
-	unsigned long pfn = pte_pfn(pte);
+	unsigned long pfn;
+
+	if (HAVE_PTE_SPECIAL) {
I really don't think this is *any* different from "#ifdefs in code".

Quoted 7 lines

#ifdef's in code is not about syntax, it's about abstraction. This is still the exact same thing as having an #ifdef around it, and in many ways it is *worse*, because now it's just made to look somewhat different with a particularly ugly #ifdef.
IOW, this didn't abstract the issue away, it just massaged it to look different.

Yes, the if () is just to please Andrew, not you ;)

I thought in your last mail on the subject, that you had conceded the vma-based scheme should stay, so I might have misunderstood that to mean you would, reluctantly, go with the scheme. I guess I need to try a bit harder ;)

Quoted 7 lines

I suspect that the nicest abstraction would be to simply make the whole function be a per-architecture thing. Not exposing a "pte_special()" bit at all, but instead having the interface simply be:
- create special entries:
pte_t pte_mkspecial(pte_t pte)

Quoted 7 lines

- check if an entry is special:
struct page *vm_normal_page(vma, addr, pte)
and now it's not while the naming is a bit odd (for historical reasons), at least it is properly *abstracted* and you don't have any #ifdef's in code (and we'd probably need to extend that abstraction then for the "locklessly look up page" case eventually).

Now I would have done this in a flash, except the existing vm_normal_page code is quite a lot, and complex, to duplicate in every architecture.

Quoted 11 lines

[ To make it slightly more regular, we could make "pte_mkspecial()" take
the vma/addr thing too, even though it would never really use it except to perhaps have a VM_BUG_ON() that it only happens within XIP/PFNMAP vma's.
The "pte_mkspecial()" definitely has more to to with "vm_normal_page()" than with the other "pte_mkxyzzy()" functions, so it really might make sense to instead make the thing
void set_special_page(vma, addr, pte_t *, pfn, pgprot)

Quoted 7 lines

because it is never acceptable to do "pte_mkspecial()" on any existent PTE *anyway*, so we might as well make the interface reflect that: it's not that you make a pte "special", it's that you insert a special page into the VM.
So the operation really conceptually has more to do with "set_pte()" than with "pte_mkxxx()", no? ]

Possibly, although I think going that far is hiding things from mm/ a bit much. If you have a look at the places that call pte_mkspecial, it isn't too much I think...

Quoted 17 lines

Then, just have a library version of the long form, and make architectures that don't support it just use that (just so that you don't have to duplicate that silly thing). So an architecture that support special page flags would do somethiing like
	#define set_special_page(vma,addr,ptep,pfn,prot) \
		set_pte_at(vma, addr, ptep, mk_special_pte(pfn,prot))
	#define vm_normal_page(vma,addr,pte)
		(pte_special(pte) ? NULL : pte_page(pte))
and other architectures would just do
	#define set_special_page(vma,addr,ptep,pfn,prot) \
		set_pte_at(vma, addr, ptep, mk_pte(pfn,prot))
	#define vm_normal_page(vma,addr,pte) \
		generic_vm_normal_page(vma,addr,pte)

Quoted 6 lines

or something.
THAT is what I mean by "no #ifdef's in code" - that the selection is done at a higher level, the same way we have good interfaces with clear *conceptual* meaning for all the other PTE accessing stuff, rather than have conditionals in the architecture-independent code.

OK, that gets around the "duplicate vm_normal_page everywhere" issue I had. I'm still not quite happy with it ;)

How about taking a different approach. How about also having a pte_normal() function. Each architecture that has a pte special bit would make this !pte_special, and those that don't would return 0. They return 0 from both pte_special and pte_normal because they don't know whether the pte is special or normal.

Then vm_normal_page would become:

    if (pte_special(pte))
        return NULL;
    else if (pte_normal(pte))
        return pte_page(pte);

    ... /* vma based scheme */

Linus Torvalds

2008-01-18 23:03 UTC #8 lore raw

Re: [patch 2/6] mm: introduce pte_special pte bit

Quoted 5 lines · Nick Piggin

I thought in your last mail on the subject, that you had conceded the vma-based scheme should stay, so I might have misunderstood that to mean you would, reluctantly, go with the scheme. I guess I need to try a bit harder ;)

Yes, I did concede that apparently we cannot just mandate "let's just use a bit in the pte".

So I do agree that we seem to be forced to have two different
implementations: one for architectures where we can make use of a marker on the PTE itself (or perhaps some *other* way to distinguish things automatically), and one for the ones where we need to just be able to distinguish purely based on our own data structures.

I just then didn't like the lack of abstraction.

Quoted 2 lines

How about taking a different approach. How about also having a pte_normal() function.

Well, one reason I'd prefer not to, is that I can well imagine an architecture that doesn't actually put the "normal" bit in the PTE itself, but in a separate data structure.

In particular, let's say that you decide that

- the architecture really doesn't have any space in the hw page tables
- but for various reasons you *really* don't want to use the tricky "page->offset" logic etc
- ..and you realize that PFNMAP and FIXMAP are actually very rare

so..

- you just associate each PFNMAP/FIXMAP vma with a simple bitmap that contains the "special" bit.

It's actually not that hard to do. If you have an architecture-specific interface like

	struct page *vm_normal_page(struct vm_area_struct *vma, unsigned long addr, pte_t pte);

then it wouldn't be too hard at all to create a hash lookup on the VMA (or perhaps on a "vma, 256-page-aligned(addr)" tuple) to look up a bitmap, and then use the address to see if it was marked special or not.

But yes, then you'd also need to have that extended

	set_special_pte_at(vma, addr, pfn, prot);

interface to set that bit in that bitmap.

See?

Is it better than what we already have for the generic case? Possibly not. But I like abstractions that aren't tied to *one* particular implementation.

Linus

Nick Piggin

author

2008-01-19 05:07 UTC #9 lore raw

Re: [patch 2/6] mm: introduce pte_special pte bit

Quoted 9 lines · Linus Torvalds

Quoted 5 lines · Nick Piggin
I thought in your last mail on the subject, that you had conceded the vma-based scheme should stay, so I might have misunderstood that to mean you would, reluctantly, go with the scheme. I guess I need to try a bit harder ;)

Quoted 8 lines

Yes, I did concede that apparently we cannot just mandate "let's just use a bit in the pte".
So I do agree that we seem to be forced to have two different
implementations: one for architectures where we can make use of a marker on the PTE itself (or perhaps some *other* way to distinguish things automatically), and one for the ones where we need to just be able to distinguish purely based on our own data structures.

Yep, thanks for the clarification.

Quoted 34 lines

I just then didn't like the lack of abstraction.
Quoted 2 lines
How about taking a different approach. How about also having a pte_normal() function.
Well, one reason I'd prefer not to, is that I can well imagine an architecture that doesn't actually put the "normal" bit in the PTE itself, but in a separate data structure.
In particular, let's say that you decide that
- the architecture really doesn't have any space in the hw page tables
- but for various reasons you *really* don't want to use the tricky "page->offset" logic etc
- ..and you realize that PFNMAP and FIXMAP are actually very rare
so..
- you just associate each PFNMAP/FIXMAP vma with a simple bitmap that contains the "special" bit.
It's actually not that hard to do. If you have an architecture-specific interface like
	struct page *vm_normal_page(struct vm_area_struct *vma, unsigned long addr, pte_t pte);
then it wouldn't be too hard at all to create a hash lookup on the VMA (or perhaps on a "vma, 256-page-aligned(addr)" tuple) to look up a bitmap, and then use the address to see if it was marked special or not.
But yes, then you'd also need to have that extended
	set_special_pte_at(vma, addr, pfn, prot);

Quoted 7 lines

interface to set that bit in that bitmap.
See?
Is it better than what we already have for the generic case? Possibly not. But I like abstractions that aren't tied to *one* particular implementation.

Well that's all true, but I would be a bit worried about architectures inventing their own ways of doin gthings. I mean, _every_ implementation needs to be understood by core mm/ developers; and conversely, none of the architecture maintainers need to care a single bit about any of the implementations if they provide some basic low level things to us.

So I'd argue that if someone really needed to invent another scheme, then that should also be somehow folded into mm/ code if possible rather than let the arch do it...

Nick Piggin

author

2008-01-21 09:43 UTC #10 lore raw

Re: [patch 2/6] mm: introduce pte_special pte bit

In reply to Nick Piggin

Quoted 35 lines · Nick Piggin

Quoted 17 lines · Linus Torvalds
Then, just have a library version of the long form, and make architectures that don't support it just use that (just so that you don't have to duplicate that silly thing). So an architecture that support special page flags would do somethiing like
	#define set_special_page(vma,addr,ptep,pfn,prot) \
		set_pte_at(vma, addr, ptep, mk_special_pte(pfn,prot))
	#define vm_normal_page(vma,addr,pte)
		(pte_special(pte) ? NULL : pte_page(pte))
and other architectures would just do
	#define set_special_page(vma,addr,ptep,pfn,prot) \
		set_pte_at(vma, addr, ptep, mk_pte(pfn,prot))
	#define vm_normal_page(vma,addr,pte) \
		generic_vm_normal_page(vma,addr,pte)
Quoted 6 lines
or something.
THAT is what I mean by "no #ifdef's in code" - that the selection is done at a higher level, the same way we have good interfaces with clear *conceptual* meaning for all the other PTE accessing stuff, rather than have conditionals in the architecture-independent code.
OK, that gets around the "duplicate vm_normal_page everywhere" issue I had. I'm still not quite happy with it ;)
How about taking a different approach. How about also having a pte_normal() function. Each architecture that has a pte special bit would make this !pte_special, and those that don't would return 0. They return 0 from both pte_special and pte_normal because they don't know whether the pte is special or normal.

Quoted 8 lines

Then vm_normal_page would become:

    if (pte_special(pte))
        return NULL;
    else if (pte_normal(pte))
        return pte_page(pte);

    ... /* vma based scheme */

Hmm, it's not *quite* trivial as that for one important case:
vm_insert_mixed. Because we don't actually have a pte yet, so we can't easily reuse insert_page / insert_pfn, rather we have to build the pte first and then check it (patch attached, but I think it is a step backwards)...

Really, I don't think either of my two approaches or your approach is really a fundamentally different _abstraction_. It basically just has to accommodate 2 different code paths no matter how you look at it. I don't know how this is different to, say, conditionally compiling eg. the FLATMEM/SPARSEMEM memory model code, or rwsem code, depending on whether an architecture has defined some symbol. It happens all over the kernel.

Actually, I'd argue it is _better_ than that, because the logic stays in one place (one screenful, even), and away from abuse or divergence by arch code.

If one actually came up with a new API that handles both cases better, I'd say that is a different abstraction. Or if you could come up with some different arch functions which would allow vm_normal_page to be streamlined to read more like a regular C function, that should be a different abstraction...

I'm still keen on my first patch. I know it isn't beautiful, but I think it is better than the alternatives.

---

mm: add vm_insert_mixed

vm_insert_mixed will insert either a raw pfn or a refcounted struct page into the page tables, depending on whether vm_normal_page() will return the page or not. With the introduction of the new pte bit, this is now a bit too tricky for drivers to be doing themselves.

filemap_xip uses this in a subsequent patch.

---

include/linux/mm.h +2 -0

Index: linux-2.6/include/linux/mm.h
===================================================================
@@ -1097,6 +1097,8 @@ int remap_pfn_range(struct vm_area_struc
 int vm_insert_page(struct vm_area_struct *, unsigned long addr, struct page *);
 int vm_insert_pfn(struct vm_area_struct *vma, unsigned long addr,
 			unsigned long pfn);
+int vm_insert_mixed(struct vm_area_struct *vma, unsigned long addr,
+			unsigned long pfn);
 
 struct page *follow_page(struct vm_area_struct *, unsigned long address,
 			unsigned int foll_flags);

mm/memory.c +47 -0

53 lines not on this page · raw message

[email protected]

author

2008-01-18 04:56 UTC #11 lore raw

PATCH 3/6 mm: add vm_insert_mixed

In reply to a message not in the archive

filemap_xip uses this in a subsequent patch.

Signed-off-by Nick Piggin <[email protected]>

Cc Jared Hulbert <[email protected]>

Cc Carsten Otte <[email protected]>

Cc Martin Schwidefsky <[email protected]>

Cc Heiko Carstens <[email protected]>

Cc [email protected]

---
 include/linux/mm.h |    2 +
 mm/memory.c        |   79 ++++++++++++++++++++++++++++++++++++-----------------
 2 files changed, 57 insertions(+), 24 deletions(-)

include/linux/mm.h +2 -0

Index: linux-2.6/include/linux/mm.h
===================================================================
@@ -1097,6 +1097,8 @@ int remap_pfn_range(struct vm_area_struc
 int vm_insert_page(struct vm_area_struct *, unsigned long addr, struct page *);
 int vm_insert_pfn(struct vm_area_struct *vma, unsigned long addr,
 			unsigned long pfn);
+int vm_insert_mixed(struct vm_area_struct *vma, unsigned long addr,
+			unsigned long pfn);
 
 struct page *follow_page(struct vm_area_struct *, unsigned long address,
 			unsigned int foll_flags);

mm/memory.c +55 ~~-24~~

111 lines not on this page · raw message

Signature · 5 lines

--
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>

[email protected]

author

2008-01-18 04:56 UTC #12 lore raw

PATCH 4/6 xip: support non-struct page backed memory

In reply to a message not in the archive

Convert XIP to support non-struct page backed memory, using VM_MIXEDMAP for the user mappings.

This requires the get_xip_page API to be changed to an address based one. Improve the API layering a little bit too, while we're here.

(The kaddr->pfn conversion may not be quite right for all architectures or XIP memory mappings, and the cacheflushing may need to be added for some archs).

This scheme has been tested and works for Jared's work-in-progress filesystem, with s390's xip, and with the new brd driver. It is required to have XIP filesystems on memory that isn't backed with struct page.

Signed-off-by Nick Piggin <[email protected]>

Cc Jared Hulbert <[email protected]>

Cc Carsten Otte <[email protected]>

Cc Martin Schwidefsky <[email protected]>

Cc Heiko Carstens <[email protected]>

Cc [email protected]

---
 fs/ext2/inode.c    |    2 
 fs/ext2/xip.c      |   36 ++++-----
 fs/ext2/xip.h      |    8 +-
 fs/open.c          |    2 
 include/linux/fs.h |    3 
 mm/fadvise.c       |    2 
 mm/filemap_xip.c   |  191 ++++++++++++++++++++++++++---------------------------
 mm/madvise.c       |    2 
 8 files changed, 122 insertions(+), 124 deletions(-)

fs/ext2/inode.c +1 -1

Index: linux-2.6/fs/ext2/inode.c
===================================================================
@@ -800,7 +800,7 @@ const struct address_space_operations ex
 
 const struct address_space_operations ext2_aops_xip = {
 	.bmap			= ext2_bmap,
-	.get_xip_page		= ext2_get_xip_page,
+	.get_xip_address	= ext2_get_xip_address,
 };
 
 const struct address_space_operations ext2_nobh_aops = {

fs/ext2/xip.c +18 ~~-18~~

Index: linux-2.6/fs/ext2/xip.c
===================================================================
@@ -15,24 +15,25 @@
 #include "xip.h"
 
 static inline int
-__inode_direct_access(struct inode *inode, sector_t sector,
-		      unsigned long *data)
+__inode_direct_access(struct inode *inode, sector_t block, unsigned long *data)
 {
+	sector_t sector;
 	BUG_ON(!inode->i_sb->s_bdev->bd_disk->fops->direct_access);
+
+	sector = block * (PAGE_SIZE / 512); /* ext2 block to bdev sector */
 	return inode->i_sb->s_bdev->bd_disk->fops
-		->direct_access(inode->i_sb->s_bdev,sector,data);
+		->direct_access(inode->i_sb->s_bdev, sector, data);
 }
 
 static inline int
-__ext2_get_sector(struct inode *inode, sector_t offset, int create,
+__ext2_get_block(struct inode *inode, pgoff_t pgoff, int create,
 		   sector_t *result)
 {
 	struct buffer_head tmp;
 	int rc;
 
 	memset(&tmp, 0, sizeof(struct buffer_head));
-	rc = ext2_get_block(inode, offset/ (PAGE_SIZE/512), &tmp,
-			    create);
+	rc = ext2_get_block(inode, pgoff, &tmp, create);
 	*result = tmp.b_blocknr;
 
 	/* did we get a sparse block (hole in the file)? */

@@ -45,13 +46,12 @@ __ext2_get_sector(struct inode *inode, s
 }
 
 int
-ext2_clear_xip_target(struct inode *inode, int block)
+ext2_clear_xip_target(struct inode *inode, sector_t block)
 {
-	sector_t sector = block * (PAGE_SIZE/512);
 	unsigned long data;
 	int rc;
 
-	rc = __inode_direct_access(inode, sector, &data);
+	rc = __inode_direct_access(inode, block, &data);
 	if (!rc)
 		clear_page((void*)data);
 	return rc;

@@ -69,24 +69,24 @@ void ext2_xip_verify_sb(struct super_blo
 	}
 }
 
-struct page *
-ext2_get_xip_page(struct address_space *mapping, sector_t offset,
-		   int create)
+void *
+ext2_get_xip_address(struct address_space *mapping, pgoff_t pgoff, int create)
 {
 	int rc;
 	unsigned long data;
-	sector_t sector;
+	sector_t block;
 
 	/* first, retrieve the sector number */
-	rc = __ext2_get_sector(mapping->host, offset, create, &sector);
+	rc = __ext2_get_block(mapping->host, pgoff, create, &block);
 	if (rc)
 		goto error;
 
 	/* retrieve address of the target data */
-	rc = __inode_direct_access
-		(mapping->host, sector * (PAGE_SIZE/512), &data);
-	if (!rc)
-		return virt_to_page(data);
+	rc = __inode_direct_access(mapping->host, block, &data);
+	if (rc)
+		goto error;
+
+	return (void *)data;
 
  error:
 	return ERR_PTR(rc);

fs/ext2/xip.h +4 -4

Index: linux-2.6/fs/ext2/xip.h
===================================================================
@@ -7,19 +7,19 @@
 
 #ifdef CONFIG_EXT2_FS_XIP
 extern void ext2_xip_verify_sb (struct super_block *);
-extern int ext2_clear_xip_target (struct inode *, int);
+extern int ext2_clear_xip_target (struct inode *, sector_t);
 
 static inline int ext2_use_xip (struct super_block *sb)
 {
 	struct ext2_sb_info *sbi = EXT2_SB(sb);
 	return (sbi->s_mount_opt & EXT2_MOUNT_XIP);
 }
-struct page* ext2_get_xip_page (struct address_space *, sector_t, int);
-#define mapping_is_xip(map) unlikely(map->a_ops->get_xip_page)
+void *ext2_get_xip_address(struct address_space *, sector_t, int);
+#define mapping_is_xip(map) unlikely(map->a_ops->get_xip_address)
 #else
 #define mapping_is_xip(map)			0
 #define ext2_xip_verify_sb(sb)			do { } while (0)
 #define ext2_use_xip(sb)			0
 #define ext2_clear_xip_target(inode, chain)	0
-#define ext2_get_xip_page			NULL
+#define ext2_get_xip_address			NULL
 #endif

fs/open.c +1 -1

Index: linux-2.6/fs/open.c
===================================================================
@@ -778,7 +778,7 @@ static struct file *__dentry_open(struct
 	if (f->f_flags & O_DIRECT) {
 		if (!f->f_mapping->a_ops ||
 		    ((!f->f_mapping->a_ops->direct_IO) &&
-		    (!f->f_mapping->a_ops->get_xip_page))) {
+		    (!f->f_mapping->a_ops->get_xip_address))) {
 			fput(f);
 			f = ERR_PTR(-EINVAL);
 		}

include/linux/fs.h +1 -2

Index: linux-2.6/include/linux/fs.h
===================================================================
@@ -473,8 +473,7 @@ struct address_space_operations {
 	int (*releasepage) (struct page *, gfp_t);
 	ssize_t (*direct_IO)(int, struct kiocb *, const struct iovec *iov,
 			loff_t offset, unsigned long nr_segs);
-	struct page* (*get_xip_page)(struct address_space *, sector_t,
-			int);
+	void * (*get_xip_address)(struct address_space *, pgoff_t, int);
 	/* migrate the contents of a page to the specified target */
 	int (*migratepage) (struct address_space *,
 			struct page *, struct page *);

mm/fadvise.c +1 -1

Index: linux-2.6/mm/fadvise.c
===================================================================
@@ -49,7 +49,7 @@ asmlinkage long sys_fadvise64_64(int fd,
 		goto out;
 	}
 
-	if (mapping->a_ops->get_xip_page)
+	if (mapping->a_ops->get_xip_address)
 		/* no bad return value, but ignore advice */
 		goto out;

mm/filemap_xip.c +95 ~~-96~~

349 lines not on this page · raw message

mm/madvise.c +1 -1

Index: linux-2.6/mm/madvise.c
===================================================================
@@ -112,7 +112,7 @@ static long madvise_willneed(struct vm_a
 	if (!file)
 		return -EBADF;
 
-	if (file->f_mapping->a_ops->get_xip_page) {
+	if (file->f_mapping->a_ops->get_xip_address) {
 		/* no bad return value, but ignore advice */
 		return 0;
 	}

Signature · 5 lines

--
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>

Jared Hulbert

2008-03-01 08:14 UTC #13 lore raw

Re: [patch 4/6] xip: support non-struct page backed memory

Quoted 4 lines

(The kaddr->pfn conversion may not be quite right for all architectures or XIP memory mappings, and the cacheflushing may need to be added for some archs).
This scheme has been tested and works for Jared's work-in-progress filesystem,

Opps. I screwed up testing this. It doesn't work with MTD devices and ARM....

The problem is that virt_to_phys() gives bogus answer for a mtd->point()'ed address. It's a ioremap()'ed address which doesn't work with the ARM virt_to_phys(). I can get a physical address from mtd->point() with a patch I dropped a little while back.

So I was thinking how about instead of:

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

void * get_xip_address(struct address_space *mapping, pgoff_t pgoff, int create);

xip_mem = mapping->a_ops->get_xip_address(mapping, vmf->pgoff, 0); pfn = virt_to_phys((void *)xip_mem) >> PAGE_SHIFT;
err = vm_insert_mixed(vma, (unsigned long)vmf->virtual_address, pfn); <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<

Could we do?

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

int get_xip_address(struct address_space *mapping, pgoff_t pgoff, int create, unsigned long *address);

if(mapping->a_ops->get_xip_address(mapping, vmf->pgoff, 0, &xip_mem)){
     /* virtual address */
     pfn = virt_to_phys((void *)xip_mem) >> PAGE_SHIFT;
} else {
     /* physical address */
     pfn = xip_mem >> PAGE_SHIFT;
}

err = vm_insert_mixed(vma, (unsigned long)vmf->virtual_address, pfn); <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<

Or maybe like...

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

unsigned long get_xip_address(struct address_space *mapping, pgoff_t pgoff, int create, int *switch);

xip_mem = mapping->a_ops->get_xip_address(mapping, vmf->pgoff, 0, &switch);

if(switch){
     /* virtual address */
     pfn = virt_to_phys((void *)xip_mem) >> PAGE_SHIFT;
} else {
     /* physical address */
     pfn = xip_mem >> PAGE_SHIFT;
}

err = vm_insert_mixed(vma, (unsigned long)vmf->virtual_address, pfn); <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<

Or...

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

void get_xip_address(struct address_space *mapping, pgoff_t pgoff, int create, unsigned long *phys, void **virt);

mapping->a_ops->get_xip_address(mapping, vmf->pgoff, 0, &phys, &virt);

if(phys){
     /* physical address */
     pfn = phys >> PAGE_SHIFT;
} else {
     /* physical address */
     pfn = virt_to_phys(virt) >> PAGE_SHIFT;
}

err = vm_insert_mixed(vma, (unsigned long)vmf->virtual_address, pfn); <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<

Nick Piggin

author

2008-03-03 05:29 UTC #14 lore raw

Re: [patch 4/6] xip: support non-struct page backed memory

Quoted 11 lines · Jared Hulbert

Quoted 4 lines
(The kaddr->pfn conversion may not be quite right for all architectures or XIP memory mappings, and the cacheflushing may need to be added for some archs).
This scheme has been tested and works for Jared's work-in-progress filesystem,
Opps. I screwed up testing this. It doesn't work with MTD devices and ARM....
The problem is that virt_to_phys() gives bogus answer for a mtd->point()'ed address. It's a ioremap()'ed address which doesn't work with the ARM virt_to_phys(). I can get a physical address from mtd->point() with a patch I dropped a little while back.

Yeah, I thought that virt_to_phys was going to be problematic...

Quoted 57 lines

So I was thinking how about instead of:
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
void * get_xip_address(struct address_space *mapping, pgoff_t pgoff, int create);
xip_mem = mapping->a_ops->get_xip_address(mapping, vmf->pgoff, 0); pfn = virt_to_phys((void *)xip_mem) >> PAGE_SHIFT;
err = vm_insert_mixed(vma, (unsigned long)vmf->virtual_address, pfn); <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
Could we do?
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
int get_xip_address(struct address_space *mapping, pgoff_t pgoff, int create, unsigned long *address);
if(mapping->a_ops->get_xip_address(mapping, vmf->pgoff, 0, &xip_mem)){
     /* virtual address */
     pfn = virt_to_phys((void *)xip_mem) >> PAGE_SHIFT;
} else {
     /* physical address */
     pfn = xip_mem >> PAGE_SHIFT;
}
err = vm_insert_mixed(vma, (unsigned long)vmf->virtual_address, pfn); <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
Or maybe like...
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
unsigned long get_xip_address(struct address_space *mapping, pgoff_t pgoff, int create, int *switch);
xip_mem = mapping->a_ops->get_xip_address(mapping, vmf->pgoff, 0, &switch);
if(switch){
     /* virtual address */
     pfn = virt_to_phys((void *)xip_mem) >> PAGE_SHIFT;
} else {
     /* physical address */
     pfn = xip_mem >> PAGE_SHIFT;
}
err = vm_insert_mixed(vma, (unsigned long)vmf->virtual_address, pfn); <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
Or...
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
void get_xip_address(struct address_space *mapping, pgoff_t pgoff, int create, unsigned long *phys, void **virt);
mapping->a_ops->get_xip_address(mapping, vmf->pgoff, 0, &phys, &virt);
if(phys){
     /* physical address */
     pfn = phys >> PAGE_SHIFT;
} else {
     /* physical address */
     pfn = virt_to_phys(virt) >> PAGE_SHIFT;
}

Quoted 2 lines

err = vm_insert_mixed(vma, (unsigned long)vmf->virtual_address, pfn); <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<

OK right... one problem is that we need an address for the kernel to manipulate the memory with, but we also need a pfn to insert into user page tables. So I like your last suggestion, but I think we always need both address and pfn.

What about
int get_xip_mem(mapping, pgoff, create, void **kaddr, unsigned long *pfn)

get_xip_mem(mapping, pgoff, create, &addr, &pfn);
if (pagefault)

    vm_insert_mixed(vma, vaddr, pfn);
else if (read/write) {
    memcpy(kaddr, blah, sizeof);

My simple brd driver can easily do
*kaddr = page_address(page);
*pfn = page_to_pfn(page);

This should work for you too?

Thanks,
Nick

Carsten Otte

2008-03-03 08:30 UTC #15 lore raw

Re: [patch 4/6] xip: support non-struct page backed memory

Quoted 8 lines · Nick Piggin

What about int get_xip_mem(mapping, pgoff, create, void **kaddr, unsigned long *pfn)
get_xip_mem(mapping, pgoff, create, &addr, &pfn);
if (pagefault)
    vm_insert_mixed(vma, vaddr, pfn);
else if (read/write) {
    memcpy(kaddr, blah, sizeof);

Quoted 5 lines

My simple brd driver can easily do
*kaddr = page_address(page);
*pfn = page_to_pfn(page);
This should work for you too?

Looks good to me. Otoh, if there is an easy way to fix virt_to_phys() I would like that better.

Jared Hulbert

2008-03-03 15:59 UTC #16 lore raw

Re: [patch 4/6] xip: support non-struct page backed memory

In reply to Nick Piggin

Quoted 4 lines

OK right... one problem is that we need an address for the kernel to manipulate the memory with, but we also need a pfn to insert into user page tables. So I like your last suggestion, but I think we always need both address and pfn.

Right I forgot about the xip_file_read() path.

I like to use UML kernels with the file-to-iomem interface for testing. I forget how UML kernels deal with physical address like this. I remember there are some caveats. I'll look into it. But I think I can do for UML:

*kaddr = (void *)(start_virt_addr + offset);
*pfn = virt_to_phys(*kaddr) >> PAGE_SHIFT;

And the ARM + MTD I can do:

*kaddr = (void *)(start_virt_addr + offset);
*pfn = (start_phys_addr + offset) >> PAGE_SHIFT;

Quoted 1 line

This should work for you too?

I think so. But like Carsten I'd prefer a virtual only solution.

Carsten Otte

2008-03-03 08:18 UTC #17 lore raw

Re: [patch 4/6] xip: support non-struct page backed memory

In reply to Jared Hulbert

Quoted 4 lines · Jared Hulbert

The problem is that virt_to_phys() gives bogus answer for a mtd->point()'ed address. It's a ioremap()'ed address which doesn't work with the ARM virt_to_phys(). I can get a physical address from mtd->point() with a patch I dropped a little while back.

Is there a chance virt_to_phys() can be fixed on arm? It looks like a simple page table walk to me. If not, I would prefer to have get_xip_address return a physical address over having to split the code path here. S390 has a 1:1 mapping for xip mappings, thus it would'nt be a big change for us.

Jared Hulbert

2008-03-03 15:44 UTC #18 lore raw

Re: [patch 4/6] xip: support non-struct page backed memory

Quoted 2 lines

Is there a chance virt_to_phys() can be fixed on arm? It looks like a simple page table walk to me.

Are there functions already available for doing a page table walk? If so it could be done. I'd like that. If somebody could point me in the right direction I'd appreciate it.

It might be a problem because today the simple case for virt_to_phys() just subtracts 0x20000000 to go from 0xCXXXXXXX to 0xAXXXXXXX. So it could have a negative performance if we complicate it.

Is it possible that it might be easier to fix this if we changed ioremap()? I got the impression that ioremap() on ARM ends up placing ioremap()'ed memory in the middle of the 0xCXXXXXXX range that is valid for RAM.

Linus Torvalds

2008-03-03 18:40 UTC #19 lore raw

Re: [patch 4/6] xip: support non-struct page backed memory

In reply to Carsten Otte

Quoted 8 lines · Carsten Otte

Quoted 4 lines · Jared Hulbert
The problem is that virt_to_phys() gives bogus answer for a mtd->point()'ed address. It's a ioremap()'ed address which doesn't work with the ARM virt_to_phys(). I can get a physical address from mtd->point() with a patch I dropped a little while back.
Is there a chance virt_to_phys() can be fixed on arm?

NO!

"virt_to_phys()" is about kernel 1:1-mapped virtual addresses, and "fixing" it would be totally wrong. We don't do crap like following page tables, and we shouldn't encourage anybody to even think that we do.

If somebody needs to follow page table pointers, they had better do it themselves and open-code the fact that they are doing something stupid and expensive, not make it easy for everybody else to do that mistake without even realising.

A lot of the kernel architecture is all about making it really hard to do stupid things by mistake.

Linus

Jared Hulbert

2008-03-03 19:38 UTC #20 lore raw

Re: [patch 4/6] xip: support non-struct page backed memory

Quoted 4 lines

NO!
"virt_to_phys()" is about kernel 1:1-mapped virtual addresses, and "fixing" it would be totally wrong.

By 1:1 you mean virtual + offset == physical + offset right?

Linus Torvalds

2008-03-03 20:04 UTC #21 lore raw

Re: [patch 4/6] xip: support non-struct page backed memory

Quoted 2 lines · Jared Hulbert

By 1:1 you mean virtual + offset == physical + offset right?

Right. It's a special case, and it's an important special case because it's the only one that is fast to do.

It's not very common, but it's common enough that it's worth doing.

That said, xip should probably never have used virt_to_phys() in the first place. It should be limited to purely architecture-specific memory management routines.

[ There's a number of drivers that need "physical" addresses for DMA, and
that use virt_to_phys, but they should use the DMA interfaces that do this right, and even for legacy things that don't use the proper DMA allocator things virt_to_phys is wrong, because it's about _bus_ addresses, not CPU physical addresses. Only architecture code can know when the two actually mean the same thing ]

Quite frankly, I think it's totally wrong to use kernel-virtual addresses in those interfaces in first place. Either you use "struct page *" or you use a pfn number. Nothing else is simply valid.

Linus

Nick Piggin

author

2008-03-03 20:32 UTC #22 lore raw

Re: [patch 4/6] xip: support non-struct page backed memory

Quoted 6 lines · Linus Torvalds

Quoted 2 lines · Jared Hulbert
By 1:1 you mean virtual + offset == physical + offset right?

Quoted 8 lines

Right. It's a special case, and it's an important special case because it's the only one that is fast to do.
It's not very common, but it's common enough that it's worth doing.
That said, xip should probably never have used virt_to_phys() in the first place. It should be limited to purely architecture-specific memory management routines.

Actually, xip in your kernel doesn't, it was just a patch I proposed. Basically I wanted to get a pfn from a kva, however that kva might be ioremapped which I didn't actually worry about because only testing a plain RAM backed system.

Quoted 10 lines

[ There's a number of drivers that need "physical" addresses for DMA, and
that use virt_to_phys, but they should use the DMA interfaces that do this right, and even for legacy things that don't use the proper DMA allocator things virt_to_phys is wrong, because it's about _bus_ addresses, not CPU physical addresses. Only architecture code can know when the two actually mean the same thing ]
Quite frankly, I think it's totally wrong to use kernel-virtual addresses in those interfaces in first place. Either you use "struct page *" or you use a pfn number. Nothing else is simply valid.

Although they were already using kernel-virtual addresses before I got there, we want to remove the requirement to have a struct page, and there are no good accessors to kmap a pfn (AFAIK) otherwise we could indeed just use a pfn.

We'll scrap the virt_to_phys idea and make the interface return both the kaddr and the pfn, I think.

Linus Torvalds

2008-03-03 22:21 UTC #23 lore raw

Re: [patch 4/6] xip: support non-struct page backed memory

Quoted 5 lines · Nick Piggin

Although they were already using kernel-virtual addresses before I got there, we want to remove the requirement to have a struct page, and there are no good accessors to kmap a pfn (AFAIK) otherwise we could indeed just use a pfn.

Implementing a kmap_pfn() sounds like a perfectly sane idea. But why does it need to even be mapped into kernel space? Is it for the ELF header reading or something (not having looked at the patch, just reacting to the wrongness of using virt_to_phys())?

Linus

Jared Hulbert

2008-03-03 23:25 UTC #24 lore raw

Re: [patch 4/6] xip: support non-struct page backed memory

Quoted 4 lines

Implementing a kmap_pfn() sounds like a perfectly sane idea. But why does it need to even be mapped into kernel space? Is it for the ELF header reading or something (not having looked at the patch, just reacting to the wrongness of using virt_to_phys())?

Right.

My AXFS prefers the filesystem image to be in memory like Flash. So it also uses the kaddr to read it's data structures and to fetch data for the readpage(). In fact, the MTD doesn't provide access to the physical address of a given partition without a patch.

Carsten Otte

2008-03-04 09:06 UTC #25 lore raw

Re: [patch 4/6] xip: support non-struct page backed memory

In reply to Linus Torvalds

Quoted 1 line · Linus Torvalds

Implementing a kmap_pfn() sounds like a perfectly sane idea. But why does it need to even be mapped into kernel space? Is it for the ELF header reading or something (not having looked at the patch, just reacting to the wrongness of using virt_to_phys())?

It needs to be mapped into kernel space to do regular file operations other than mmap. In mm/filemap_xip.c we do access the xip memory from kernel to fulfill sys_read/sys_write and friends [copy_from/to_user to user's buffer].

so long,
Carsten