Skip to contentSkip to conversation

pass mm into pagewalkers

PATCH In review Dave Hansen 5 participants 15 messages Jun 11, 2008 – Jun 12, 2008

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

PATCH v4 1/2 pass mm into pagewalkers

We need this at least for huge page detection for now.

It might also come in handy for some of the other
users.

---

 linux-2.6.git-dave/fs/proc/task_mmu.c |   44 +++++++++++++++++++---------------
 linux-2.6.git-dave/include/linux/mm.h |   17 ++++++-------
 linux-2.6.git-dave/mm/pagewalk.c      |   42 +++++++++++++++++---------------
 3 files changed, 56 insertions(+), 47 deletions(-)
mm/pagewalk.c renamed from mm/pagewalk.c~pass-mm-into-pagewalkers +22 -20
diff -puN mm/pagewalk.c~pass-mm-into-pagewalkers mm/pagewalk.c
@@ -3,14 +3,14 @@
 #include <linux/sched.h>
 
 static int walk_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
-			  const struct mm_walk *walk, void *private)
+			  struct mm_walk *walk)
 {
 	pte_t *pte;
 	int err = 0;
 
 	pte = pte_offset_map(pmd, addr);
 	for (;;) {
-		err = walk->pte_entry(pte, addr, addr + PAGE_SIZE, private);
+		err = walk->pte_entry(pte, addr, addr + PAGE_SIZE, walk);
 		if (err)
 		       break;
 		addr += PAGE_SIZE;
@@ -24,7 +24,7 @@ static int walk_pte_range(pmd_t *pmd, un
 }
 
 static int walk_pmd_range(pud_t *pud, unsigned long addr, unsigned long end,
-			  const struct mm_walk *walk, void *private)
+			  struct mm_walk *walk)
 {
 	pmd_t *pmd;
 	unsigned long next;
@@ -35,15 +35,15 @@ static int walk_pmd_range(pud_t *pud, un
 		next = pmd_addr_end(addr, end);
 		if (pmd_none_or_clear_bad(pmd)) {
 			if (walk->pte_hole)
-				err = walk->pte_hole(addr, next, private);
+				err = walk->pte_hole(addr, next, walk);
 			if (err)
 				break;
 			continue;
 		}
 		if (walk->pmd_entry)
-			err = walk->pmd_entry(pmd, addr, next, private);
+			err = walk->pmd_entry(pmd, addr, next, walk);
 		if (!err && walk->pte_entry)
-			err = walk_pte_range(pmd, addr, next, walk, private);
+			err = walk_pte_range(pmd, addr, next, walk);
 		if (err)
 			break;
 	} while (pmd++, addr = next, addr != end);
@@ -52,7 +52,7 @@ static int walk_pmd_range(pud_t *pud, un
 }
 
 static int walk_pud_range(pgd_t *pgd, unsigned long addr, unsigned long end,
-			  const struct mm_walk *walk, void *private)
+			  struct mm_walk *walk)
 {
 	pud_t *pud;
 	unsigned long next;
@@ -63,15 +63,15 @@ static int walk_pud_range(pgd_t *pgd, un
 		next = pud_addr_end(addr, end);
 		if (pud_none_or_clear_bad(pud)) {
 			if (walk->pte_hole)
-				err = walk->pte_hole(addr, next, private);
+				err = walk->pte_hole(addr, next, walk);
 			if (err)
 				break;
 			continue;
 		}
 		if (walk->pud_entry)
-			err = walk->pud_entry(pud, addr, next, private);
+			err = walk->pud_entry(pud, addr, next, walk);
 		if (!err && (walk->pmd_entry || walk->pte_entry))
-			err = walk_pmd_range(pud, addr, next, walk, private);
+			err = walk_pmd_range(pud, addr, next, walk);
 		if (err)
 			break;
 	} while (pud++, addr = next, addr != end);
@@ -85,15 +85,15 @@ static int walk_pud_range(pgd_t *pgd, un
  * @addr: starting address
  * @end: ending address
  * @walk: set of callbacks to invoke for each level of the tree
- * @private: private data passed to the callback function
  *
  * Recursively walk the page table for the memory area in a VMA,
  * calling supplied callbacks. Callbacks are called in-order (first
  * PGD, first PUD, first PMD, first PTE, second PTE... second PMD,
  * etc.). If lower-level callbacks are omitted, walking depth is reduced.
  *
- * Each callback receives an entry pointer, the start and end of the
- * associated range, and a caller-supplied private data pointer.
+ * Each callback receives an entry pointer and the start and end of the
+ * associated range, and a copy of the original mm_walk for access to
+ * the ->private or ->mm fields.
  *
  * No locks are taken, but the bottom level iterator will map PTE
  * directories from highmem if necessary.
@@ -101,9 +101,8 @@ static int walk_pud_range(pgd_t *pgd, un
  * If any callback returns a non-zero value, the walk is aborted and
  * the return value is propagated back to the caller. Otherwise 0 is returned.
  */
-int walk_page_range(const struct mm_struct *mm,
-		    unsigned long addr, unsigned long end,
-		    const struct mm_walk *walk, void *private)
+int walk_page_range(unsigned long addr, unsigned long end,
+		    struct mm_walk *walk)
 {
 	pgd_t *pgd;
 	unsigned long next;
@@ -112,21 +111,24 @@ int walk_page_range(const struct mm_stru
 	if (addr >= end)
 		return err;
 
-	pgd = pgd_offset(mm, addr);
+	if (!walk->mm)
+		return -EINVAL;
+
+	pgd = pgd_offset(walk->mm, addr);
 	do {
 		next = pgd_addr_end(addr, end);
 		if (pgd_none_or_clear_bad(pgd)) {
 			if (walk->pte_hole)
-				err = walk->pte_hole(addr, next, private);
+				err = walk->pte_hole(addr, next, walk);
 			if (err)
 				break;
 			continue;
 		}
 		if (walk->pgd_entry)
-			err = walk->pgd_entry(pgd, addr, next, private);
+			err = walk->pgd_entry(pgd, addr, next, walk);
 		if (!err &&
 		    (walk->pud_entry || walk->pmd_entry || walk->pte_entry))
-			err = walk_pud_range(pgd, addr, next, walk, private);
+			err = walk_pud_range(pgd, addr, next, walk);
 		if (err)
 			break;
 	} while (pgd++, addr = next, addr != end);
include/linux/mm.h renamed from include/linux/mm.h~pass-mm-into-pagewalkers +9 -8
diff -puN include/linux/mm.h~pass-mm-into-pagewalkers include/linux/mm.h
@@ -760,16 +760,17 @@ unsigned long unmap_vmas(struct mmu_gath
  * (see walk_page_range for more details)
  */
 struct mm_walk {
-	int (*pgd_entry)(pgd_t *, unsigned long, unsigned long, void *);
-	int (*pud_entry)(pud_t *, unsigned long, unsigned long, void *);
-	int (*pmd_entry)(pmd_t *, unsigned long, unsigned long, void *);
-	int (*pte_entry)(pte_t *, unsigned long, unsigned long, void *);
-	int (*pte_hole)(unsigned long, unsigned long, void *);
+	int (*pgd_entry)(pgd_t *, unsigned long, unsigned long, struct mm_walk *);
+	int (*pud_entry)(pud_t *, unsigned long, unsigned long, struct mm_walk *);
+	int (*pmd_entry)(pmd_t *, unsigned long, unsigned long, struct mm_walk *);
+	int (*pte_entry)(pte_t *, unsigned long, unsigned long, struct mm_walk *);
+	int (*pte_hole)(unsigned long, unsigned long, struct mm_walk *);
+	struct mm_struct *mm;
+	void *private;
 };
 
-int walk_page_range(const struct mm_struct *, unsigned long addr,
-		    unsigned long end, const struct mm_walk *walk,
-		    void *private);
+int walk_page_range(unsigned long addr, unsigned long end,
+		struct mm_walk *walk);
 void free_pgd_range(struct mmu_gather **tlb, unsigned long addr,
 		unsigned long end, unsigned long floor, unsigned long ceiling);
 void free_pgtables(struct mmu_gather **tlb, struct vm_area_struct *start_vma,
fs/proc/task_mmu.c renamed from fs/proc/task_mmu.c~pass-mm-into-pagewalkers +25 -19

107 lines not on this page · raw message

_

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

PATCH v4 2/2 fix large pages in pagemap

We were walking right into huge page areas in the pagemap walker, and calling the pmds pmd_bad() and clearing them.

That leaked huge pages. Bad.

This patch at least works around that for now. It ignores huge pages in the pagemap walker for the time being, and won't leak those pages.

---

 linux-2.6.git-dave/fs/proc/task_mmu.c |   39 ++++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 9 deletions(-)
fs/proc/task_mmu.c renamed from fs/proc/task_mmu.c~fix-large-pages-in-pagemap +30 -9
diff -puN fs/proc/task_mmu.c~fix-large-pages-in-pagemap fs/proc/task_mmu.c
@@ -563,24 +563,45 @@ static u64 swap_pte_to_pagemap_entry(pte
 	return swp_type(e) | (swp_offset(e) << MAX_SWAPFILES_SHIFT);
 }
 
+static unsigned long pte_to_pagemap_entry(pte_t pte)
+{
+	unsigned long pme = 0;
+	if (is_swap_pte(pte))
+		pme = PM_PFRAME(swap_pte_to_pagemap_entry(pte))
+			| PM_PSHIFT(PAGE_SHIFT) | PM_SWAP;
+	else if (pte_present(pte))
+		pme = PM_PFRAME(pte_pfn(pte))
+			| PM_PSHIFT(PAGE_SHIFT) | PM_PRESENT;
+	return pme;
+}
+
 static int pagemap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
 			     struct mm_walk *walk)
 {
+	struct vm_area_struct *vma;
 	struct pagemapread *pm = walk->private;
 	pte_t *pte;
 	int err = 0;
 
+	/* find the first VMA at or above 'addr' */
+	vma = find_vma(walk->mm, addr);
 	for (; addr != end; addr += PAGE_SIZE) {
 		u64 pfn = PM_NOT_PRESENT;
-		pte = pte_offset_map(pmd, addr);
-		if (is_swap_pte(*pte))
-			pfn = PM_PFRAME(swap_pte_to_pagemap_entry(*pte))
-				| PM_PSHIFT(PAGE_SHIFT) | PM_SWAP;
-		else if (pte_present(*pte))
-			pfn = PM_PFRAME(pte_pfn(*pte))
-				| PM_PSHIFT(PAGE_SHIFT) | PM_PRESENT;
-		/* unmap so we're not in atomic when we copy to userspace */
-		pte_unmap(pte);
+
+		/* check to see if we've left 'vma' behind
+		 * and need a new, higher one */
+		if (vma && (addr >= vma->vm_end))
+			vma = find_vma(walk->mm, addr);
+
+		/* check that 'vma' actually covers this address,
+		 * and that it isn't a huge page vma */
+		if (vma && (vma->vm_start <= addr) &&
+		    !is_vm_hugetlb_page(vma)) {
+			pte = pte_offset_map(pmd, addr);
+			pfn = pte_to_pagemap_entry(*pte);
+			/* unmap before userspace copy */
+			pte_unmap(pte);
+		}
 		err = add_to_pagemap(addr, pfn, pm);
 		if (err)
 			return err;

diff -puN mm/pagewalk.c~fix-large-pages-in-pagemap mm/pagewalk.c _

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

Re: [v4][PATCH 2/2] fix large pages in pagemap

Quoted 10 lines · On Wed, 11 Jun 2008 11:02:31 -0700 Dave Hansen <[email protected]> wrote:

We were walking right into huge page areas in the pagemap walker, and calling the pmds pmd_bad() and clearing them.

That leaked huge pages. Bad.

This patch at least works around that for now. It ignores huge pages in the pagemap walker for the time being, and won't leak those pages.

I don't get it. Why can't we just stick a

	if (pmd_huge(pmd))
		continue;

into pagemap_pte_range()? Or something like that.

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

#3 lore raw

Re: [v4][PATCH 2/2] fix large pages in pagemap

[adding Hugh to the cc:]

Quoted 14 lines · Andrew Morton
Quoted 10 lines · On Wed, 11 Jun 2008 11:02:31 -0700 Dave Hansen <[email protected]> wrote:

We were walking right into huge page areas in the pagemap walker, and calling the pmds pmd_bad() and clearing them.

That leaked huge pages. Bad.

This patch at least works around that for now. It ignores huge pages in the pagemap walker for the time being, and won't leak those pages.

Quoted 6 lines

I don't get it. Why can't we just stick a

	if (pmd_huge(pmd))
		continue;

into pagemap_pte_range()? Or something like that.

That's certainly what you'd hope to be able to do, yes.

If I recall the earlier discussion, some arches with huge pages can only identify them via a VMA. Obviously, any arch with hardware that walks our pagetables directly must be able to identify huge pages directly from those tables, but I think PPC and a couple others that don't have hardware TLB fill fail to store such a bit in the tables at all.

Signature · 7 lines
Mathematics is the supreme nostalgia of our time.

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

Re: [v4][PATCH 2/2] fix large pages in pagemap

Quoted 24 lines · On Wed, 11 Jun 2008 14:53:00 -0500 Matt Mackall <[email protected]> wrote:

[adding Hugh to the cc:]

Quoted 14 lines · Andrew Morton
Quoted 10 lines · On Wed, 11 Jun 2008 11:02:31 -0700 Dave Hansen <[email protected]> wrote:

We were walking right into huge page areas in the pagemap walker, and calling the pmds pmd_bad() and clearing them.

That leaked huge pages. Bad.

This patch at least works around that for now. It ignores huge pages in the pagemap walker for the time being, and won't leak those pages.

Quoted 6 lines

I don't get it. Why can't we just stick a

	if (pmd_huge(pmd))
		continue;

into pagemap_pte_range()? Or something like that.

Quoted 7 lines

That's certainly what you'd hope to be able to do, yes.

If I recall the earlier discussion, some arches with huge pages can only identify them via a VMA. Obviously, any arch with hardware that walks our pagetables directly must be able to identify huge pages directly from those tables, but I think PPC and a couple others that don't have hardware TLB fill fail to store such a bit in the tables at all.

Really? There already a couple of pmd_huge() tests in mm/memory.c and Rik's access_process_vm-device-memory-infrastructure.patch adds another one.

--
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: [v4][PATCH 2/2] fix large pages in pagemap

Quoted 35 lines · Andrew Morton
Quoted 24 lines · On Wed, 11 Jun 2008 14:53:00 -0500 Matt Mackall <[email protected]> wrote:

[adding Hugh to the cc:]

Quoted 14 lines · Andrew Morton
Quoted 10 lines · On Wed, 11 Jun 2008 11:02:31 -0700 Dave Hansen <[email protected]> wrote:

We were walking right into huge page areas in the pagemap walker, and calling the pmds pmd_bad() and clearing them.

That leaked huge pages. Bad.

This patch at least works around that for now. It ignores huge pages in the pagemap walker for the time being, and won't leak those pages.

Quoted 6 lines

I don't get it. Why can't we just stick a

	if (pmd_huge(pmd))
		continue;

into pagemap_pte_range()? Or something like that.

Quoted 7 lines

That's certainly what you'd hope to be able to do, yes.

If I recall the earlier discussion, some arches with huge pages can only identify them via a VMA. Obviously, any arch with hardware that walks our pagetables directly must be able to identify huge pages directly from those tables, but I think PPC and a couple others that don't have hardware TLB fill fail to store such a bit in the tables at all.

Quoted 3 lines

Really? There already a couple of pmd_huge() tests in mm/memory.c and Rik's access_process_vm-device-memory-infrastructure.patch adds another one.

Quoting Hugh:

i>>?A pmd_huge(*pmd) test is tempting, but it only ever says "yes" on x86:
we've carefully left it undefined what happens to the pgd/pud/pmd/pte hierarchy in the general arch case, once you're amongst hugepages.

Signature · 7 lines
Mathematics is the supreme nostalgia of our time.

--
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>
#6 lore raw

Re: [v4][PATCH 2/2] fix large pages in pagemap

In reply to Andrew Morton
Quoted 3 lines · Andrew Morton

Really? There already a couple of pmd_huge() tests in mm/memory.c and Rik's access_process_vm-device-memory-infrastructure.patch adds another one.

We're not supposed to ever hit the one in follow_page() because there are:

                if (is_vm_hugetlb_page(vma)) {
                        i = follow_hugetlb_page(mm, vma, pages, vmas,
                                                &start, &len, i, write);
                        continue;
                }

checks before them like in get_user_pages();

The other mm/memory.c call is under alloc_vm_area(), and that's supposedly only used on kernel addresses. I don't think we even have Linux pagetables for kernel addresses on ppc.

-- Dave

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

Re: [v4][PATCH 2/2] fix large pages in pagemap

Quoted 14 lines · On Wed, 11 Jun 2008 13:34:22 -0700 Dave Hansen <[email protected]> wrote:
Quoted 3 lines · Andrew Morton

Really? There already a couple of pmd_huge() tests in mm/memory.c and Rik's access_process_vm-device-memory-infrastructure.patch adds another one.

We're not supposed to ever hit the one in follow_page() because there are:

                if (is_vm_hugetlb_page(vma)) {
                        i = follow_hugetlb_page(mm, vma, pages, vmas,
                                                &start, &len, i, write);
                        continue;
                }
Quoted 6 lines

checks before them like in get_user_pages();

The other mm/memory.c call is under alloc_vm_area(), and that's supposedly only used on kernel addresses. I don't think we even have Linux pagetables for kernel addresses on ppc.

access_process_vm-device-memory-infrastructure.patch is a powerpc feature, and it uses pmd_huge().

Am I missing something, or is pmd_huge() a whopping big grenade for x86 developers to toss at non-x86 architectures? It seems quite dangerous.

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

#8 lore raw

Re: [v4][PATCH 2/2] fix large pages in pagemap

Quoted 5 lines · On Wed, 11 Jun 2008 13:52:07 -0700 Andrew Morton <[email protected]>…

access_process_vm-device-memory-infrastructure.patch is a powerpc feature, and it uses pmd_huge().

Am I missing something, or is pmd_huge() a whopping big grenade for x86 developers to toss at non-x86 architectures? It seems quite dangerous.

That function is used on x86 too, to access device memory that's been mapped through /dev/memory or PCI thingies under /sys.

The X.org people need that patch on x86 to figure out what's in the GPU's queue before it threw a fit; in short, to better debug the X server.

Signature · 7 lines
All rights reversed.

--
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>
#9 lore raw

Re: [v4][PATCH 2/2] fix large pages in pagemap

In reply to Andrew Morton
Quoted 2 lines · Andrew Morton

access_process_vm-device-memory-infrastructure.patch is a powerpc feature, and it uses pmd_huge().

I think that's bogus. It probably needs to check the VMA in generic_access_phys() if it wants to be safe. I don't see any way that pmd_huge() can give anything back other than 0 on ppc:

arch/powerpc/mm/hugetlbpage.c:

	int pmd_huge(pmd_t pmd)
	{
	        return 0;
	}

or in include/linux/hugetlb.h:

	#define pmd_huge(x)     0
Quoted 2 lines

Am I missing something, or is pmd_huge() a whopping big grenade for x86 developers to toss at non-x86 architectures? It seems quite dangerous.

Yeah, it isn't really usable outside of arch code, although it kinda looks like it.

-- Dave

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

#10 lore raw

Re: [v4][PATCH 2/2] fix large pages in pagemap

Quoted 17 lines · Dave Hansen
Quoted 2 lines · Andrew Morton

access_process_vm-device-memory-infrastructure.patch is a powerpc feature, and it uses pmd_huge().

I think that's bogus. It probably needs to check the VMA in generic_access_phys() if it wants to be safe. I don't see any way that pmd_huge() can give anything back other than 0 on ppc:

arch/powerpc/mm/hugetlbpage.c:

	int pmd_huge(pmd_t pmd)
	{
	        return 0;
	}

or in include/linux/hugetlb.h:

Quoted 7 lines
	#define pmd_huge(x)     0
Quoted 2 lines

Am I missing something, or is pmd_huge() a whopping big grenade for x86 developers to toss at non-x86 architectures? It seems quite dangerous.

Yeah, it isn't really usable outside of arch code, although it kinda looks like it.

That begs the question: if we can't use it reliably outside of arch code, why do other arches even bother defining it?

And the answer seems to be because of the two uses in mm/memory.c. The first seems like it could be avoided with an implementation of follow_huge_addr on x86. The second is either bogus (only works on x86) or superfluous (not needed at all), no?

Signature · 7 lines
Mathematics is the supreme nostalgia of our time.

--
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: [v4][PATCH 2/2] fix large pages in pagemap

Quoted 11 lines · Matt Mackall
Quoted 7 lines · Dave Hansen
Quoted 2 lines · Andrew Morton

Am I missing something, or is pmd_huge() a whopping big grenade for x86 developers to toss at non-x86 architectures? It seems quite dangerous.

Yeah, it isn't really usable outside of arch code, although it kinda looks like it.

That begs the question: if we can't use it reliably outside of arch code, why do other arches even bother defining it?

Good question.

Quoted 3 lines

And the answer seems to be because of the two uses in mm/memory.c. The first seems like it could be avoided with an implementation of follow_huge_addr on x86.

No, I don't think we need even that: because get_user_pages avoids follow_page on huge vmas, and little else uses follow_page, I think we could delete follow_huge_addr from all architectures.

It's gives a warm glow to know that follow_page could cope with huge ranges if necessary, but it doesn't feel so good once you've been misled by pmd_huge.

Incidentally, x86 turns out to have a pmd_large() too!

Quoted 2 lines

The second is either bogus (only works on x86)
or superfluous (not needed at all), no?

Not needed (checking for things that should never happen is nice when it's convenient, but not a reason to uglify).

On Dave's patch: yes, I too would have preferred a pmd_huge-style vma-less approach, but fear that doesn't work out at present. But isn't the patch calling find_vma() much more often than necessary? Is there any architecture which can mix huge and normal pages within the lowest pagetable?

Hugh

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

#12 lore raw

Re: [v4][PATCH 2/2] fix large pages in pagemap

Quoted 19 lines · Hugh Dickins
Quoted 11 lines · Matt Mackall
Quoted 7 lines · Dave Hansen
Quoted 2 lines · Andrew Morton

Am I missing something, or is pmd_huge() a whopping big grenade for x86 developers to toss at non-x86 architectures? It seems quite dangerous.

Yeah, it isn't really usable outside of arch code, although it kinda looks like it.

That begs the question: if we can't use it reliably outside of arch code, why do other arches even bother defining it?

Good question.

Quoted 3 lines

And the answer seems to be because of the two uses in mm/memory.c. The first seems like it could be avoided with an implementation of follow_huge_addr on x86.

Quoted 7 lines

No, I don't think we need even that: because get_user_pages avoids follow_page on huge vmas, and little else uses follow_page, I think we could delete follow_huge_addr from all architectures.

It's gives a warm glow to know that follow_page could cope with huge ranges if necessary, but it doesn't feel so good once you've been misled by pmd_huge.

Since follow_page gets a VMA, it could check.

Quoted 1 line

Incidentally, x86 turns out to have a pmd_large() too!

As does mn10300 - surely that's a mistake?

Quoted 9 lines
Quoted 2 lines

The second is either bogus (only works on x86)
or superfluous (not needed at all), no?

Not needed (checking for things that should never happen is nice when it's convenient, but not a reason to uglify).

On Dave's patch: yes, I too would have preferred a pmd_huge-style vma-less approach, but fear that doesn't work out at present. But isn't the patch calling find_vma() much more often than necessary?

Well not at the level of the function in question. But it could probably cache a current VMA value across the whole walk in the walk structure.

Quoted 2 lines

Is there any architecture which can mix huge and normal pages within the lowest pagetable?

Not sure how that would work?

Signature · 7 lines
Mathematics is the supreme nostalgia of our time.

--
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>
#13 lore raw

Re: [v4][PATCH 2/2] fix large pages in pagemap

In reply to Matt Mackall
Quoted 8 lines · Matt Mackall
Quoted 7 lines

I don't get it. Why can't we just stick a

      if (pmd_huge(pmd))
              continue;

into pagemap_pte_range()? Or something like that.

Quoted 8 lines

That's certainly what you'd hope to be able to do, yes.

If I recall the earlier discussion, some arches with huge pages can only
identify them via a VMA. Obviously, any arch with hardware that walks our pagetables directly must be able to identify huge pages directly from those tables, but I think PPC and a couple others that don't have hardware TLB fill fail to store such a bit in the tables at all.

Yeah, the ppc (and more) huge pmd entries are just the address of the huge page. I would love to get all of them converted so we could use pmd_huge() on them, eventually. But, that's a much bigger undertaking.

-- Dave

--
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
#14 lore raw
In reply to Dave Hansen
Quoted 6 lines · On Wed, 11 Jun 2008 11:02:29 -0700 Dave Hansen <[email protected]> wrote:

We need this at least for huge page detection for now.

It might also come in handy for some of the other
users.

Changelog fails to explain why the `walk' argument was deconstified and I couldn't immediately work this out from the diff.

fs/proc/task_mmu.c static int smaps_pte_range(pmd_t *pmd, u · line 377

Quoted 5 lines
fs/proc/task_mmu.c renamed from fs/proc/task_mmu.c~pass-mm-into-pagewalkers +5 -0
+	struct mm_walk smaps_walk = {
+		.pmd_entry = smaps_pte_range,
+		.mm = vma->vm_mm,
+		.private = &mss,
+	};

a)

fs/proc/task_mmu.c static ssize_t clear_refs_write(struct f · line 482

Quoted 5 lines
fs/proc/task_mmu.c renamed from fs/proc/task_mmu.c~pass-mm-into-pagewalkers +4 -0
 	if (mm) {
+		static struct mm_walk clear_refs_walk;
+		memset(&clear_refs_walk, 0, sizeof(clear_refs_walk));
+		clear_refs_walk.pmd_entry = clear_refs_pte_range;
+		clear_refs_walk.mm = mm;

b)

where a) != b). a) looks nicer.

Please do prefer to put a blank line between end-of-locals and start-of-code. It does improve readability.

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