Hi Ingo,
This series does more unification of pgalloc, and creates a unified

mm/pgtable.c for common pagetable functions.  Ends up removing

pgalloc_32/64.h in favour of pgalloc.h.
[ I thought I'd mailed this earlier, but I don't see it on lkml.

Maybe I created the mbox without sending it.  Anyway, this should go

before the set I just mailed. ]
Thanks,

	J
--

Common definitions for 3-level pagetable functions.
Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>

---

 arch/x86/mm/pgtable.c        |    8 ++++++++

 arch/x86/mm/pgtable_32.c     |   10 ----------

 include/asm-x86/pgalloc.h    |   31 +++++++++++++++++++++++++++++++

 include/asm-x86/pgalloc_32.h |   31 -------------------------------

 include/asm-x86/pgalloc_64.h |   26 --------------------------

 5 files changed, 39 insertions(+), 67 deletions(-)
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c

--- a/arch/x86/mm/init_64.c

+++ b/arch/x86/mm/init_64.c

@@ -843,11 +843,6 @@

 }

 #endif
-void __pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmd)

-{

-	tlb_remove_page(tlb, virt_to_page(pmd));

-}

-

 void __pud_free_tlb(struct mmu_gather *tlb, pud_t *pud)

 {

 	tlb_remove_page(tlb, virt_to_page(pud));

diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c

--- a/arch/x86/mm/pgtable.c

+++ b/arch/x86/mm/pgtable.c

@@ -23,6 +23,14 @@

 	paravirt_release_pt(page_to_pfn(pte));

 	tlb_remove_page(tlb, pte);

 }

+

+#if PAGETABLE_LEVELS > 2

+void __pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmd)

+{

+	paravirt_release_pd(__pa(pmd) >> PAGE_SHIFT);

+	tlb_remove_page(tlb, virt_to_page(pmd));

+}

+#endif	/* PAGETABLE_LEVELS > 2 */
 #ifdef CONFIG_X86_64

 static inline void pgd_list_add(pgd_t *pgd)

diff --git a/arch/x86/mm/pgtable_32.c b/arch/x86/mm/pgtable_32.c

--- a/arch/x86/mm/pgtable_32.c

+++ b/arch/x86/mm/pgtable_32.c

@@ -177,13 +177,3 @@

 	__FIXADDR_TOP = -reserve - PAGE_SIZE;

 	__VMALLOC_RESERVE += reserve;

 }

-

-#ifdef CONFIG_X86_PAE

-

-void __pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmd)

-{

-	paravirt_release_pd(__pa(pmd) >> PAGE_SHIFT);

-	tlb_remove_page(tlb, virt_to_page(pmd));

-}

-

-#endif

diff --git a/include/asm-x86/pgalloc.h b/include/asm-x86/pgalloc.h

--- a/include/asm-x86/pgalloc.h

+++ b/include/asm-x86/pgalloc.h

@@ -40,6 +40,37 @@
 extern void __pte_free_tlb(struct mmu_gather *tlb, struct page *pte);
...
[ message continues ]

Common definitions for 4-level pagetable functions.
Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>

---

 arch/x86/mm/pgtable.c        |    7 ++++++

 include/asm-x86/pgalloc.h    |   46 ++++++++++++++++++++++++++++++++++++------

 include/asm-x86/pgalloc_32.h |   24 ---------------------

 include/asm-x86/pgalloc_64.h |   32 -----------------------------

 4 files changed, 47 insertions(+), 62 deletions(-)
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c

--- a/arch/x86/mm/init_64.c

+++ b/arch/x86/mm/init_64.c

@@ -842,8 +842,3 @@

 	return 0;

 }

 #endif

-

-void __pud_free_tlb(struct mmu_gather *tlb, pud_t *pud)

-{

-	tlb_remove_page(tlb, virt_to_page(pud));

-}

diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c

--- a/arch/x86/mm/pgtable.c

+++ b/arch/x86/mm/pgtable.c

@@ -30,6 +30,13 @@

 	paravirt_release_pd(__pa(pmd) >> PAGE_SHIFT);

 	tlb_remove_page(tlb, virt_to_page(pmd));

 }

+

+#if PAGETABLE_LEVELS > 3

+void __pud_free_tlb(struct mmu_gather *tlb, pud_t *pud)

+{

+	tlb_remove_page(tlb, virt_to_page(pud));

+}

+#endif	/* PAGETABLE_LEVELS > 3 */

 #endif	/* PAGETABLE_LEVELS > 2 */
 #ifdef CONFIG_X86_64

diff --git a/include/asm-x86/pgalloc.h b/include/asm-x86/pgalloc.h

--- a/include/asm-x86/pgalloc.h

+++ b/include/asm-x86/pgalloc.h

@@ -69,12 +69,46 @@

 }
 extern void __pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmd);

+

+static inline void pud_populate(struct mm_struct *mm, pud_t *pudp, pmd_t *pmd)

+{

+	paravirt_alloc_pd(mm, __pa(pmd) >> PAGE_SHIFT);

+

+	/* Note: almost everything apart from _PAGE_PRESENT is

+	   reserved at the pmd (PDPT) level. */

+	set_pud(pudp, __pud(__pa(pmd) | _PAGE_PRESENT));

+

+#ifdef CONFIG_X86_PAE

+	/*

+	 * According to Intel App note "TLBs, Paging-Structure Caches,

+	 * and Their Invalidation", April 2007, document 317080-001,

+	 * section 8.1: in PAE mode we explicitly have to flush the

+	 * TLB via cr3 if the top-level pgd is changed...

+	 */

+	if (mm == current->active_mm...
[ message continues ]

random-qa found an early bootup hang on 32-bit (config attached).
i bisected it down to this patch of yours. It's a bit large so it's not

obvious what is happening. Could you please keep patches that do

functional changes smaller? It's not a problem if the non-functional or

trivial restructuring patches are larger, but it's a killer for these

functional ones. I've dropped this one for now, and all the dependent

patches as well:
Subject: x86: move pud/pgd functions into common asm/pgalloc.h

Subject: x86: move all the pgd_list handling to one place

Subject: x86: rename paravirt_alloc_pt etc after the pagetable structure

Subject: x86: add pud_alloc for 4-level pagetables

Subject: x86/pgtable.h: demacro ptep_set_access_flags

Subject: x86/pgtable.h: demacro ptep_test_and_clear_young

Subject: x86/pgtable.h: demacro ptep_clear_flush_young
	Ingo

--

s/32-bit/64-bit
	Ingo

--

Will do, though this one is more or less pure code motion.  But I can

make it actual pure code motion with a separate merge patch.
    J

--

yes but the early hang is very real so either my hardware is stubbornly

ignoring that your patch is pure code movement (in which case i'll have

to have a word or two with my hardware), or your patch is perhaps wrong

somewhere ;-)
generally you can protect yourself against full reverts by separating

the NOP changes from the non-NOP changes. If a change is small enough i

might spot the bug immediately and fix it - otherwise i have to undo

your whole series to keep the x86.git ball rolling. I thought we went

through this excercise a few times already :-/ ...
	Ingo

--

I see what it is.  set_pud on 32-bit needs only _PAGE_PRESENT, but for 
This one was supposed to be pure motion, but my eyeball diff failed me.
    J

--

config attached now.
	Ingo

Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>

---

 arch/x86/mm/pgtable.c |   24 +++++-------------------

 1 file changed, 5 insertions(+), 19 deletions(-)
diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c

--- a/arch/x86/mm/pgtable.c

+++ b/arch/x86/mm/pgtable.c

@@ -39,32 +39,30 @@

 #endif	/* PAGETABLE_LEVELS > 3 */

 #endif	/* PAGETABLE_LEVELS > 2 */
-#ifdef CONFIG_X86_64

 static inline void pgd_list_add(pgd_t *pgd)

 {

 	struct page *page = virt_to_page(pgd);
-	spin_lock(&pgd_lock);

 	list_add(&page->lru, &pgd_list);

-	spin_unlock(&pgd_lock);

 }
 static inline void pgd_list_del(pgd_t *pgd)

 {

 	struct page *page = virt_to_page(pgd);
-	spin_lock(&pgd_lock);

 	list_del(&page->lru);

-	spin_unlock(&pgd_lock);

 }
+#ifdef CONFIG_X86_64

 pgd_t *pgd_alloc(struct mm_struct *mm)

 {

 	unsigned boundary;

 	pgd_t *pgd = (pgd_t *)__get_free_page(GFP_KERNEL|__GFP_REPEAT);

 	if (!pgd)

 		return NULL;

+	spin_lock(&pgd_lock);

 	pgd_list_add(pgd);

+	spin_unlock(&pgd_lock);

 	/*

 	 * Copy kernel pointers in from init.

 	 * Could keep a freelist or slab cache of those because the kernel

@@ -81,7 +79,9 @@

 void pgd_free(pgd_t *pgd)

 {

 	BUG_ON((unsigned long)pgd & (PAGE_SIZE-1));

+	spin_lock(&pgd_lock);

 	pgd_list_del(pgd);

+	spin_unlock(&pgd_lock);

 	free_page((unsigned long)pgd);

 }
@@ -96,20 +96,6 @@

  * vmalloc faults work because attached pagetables are never freed.

  * -- wli

  */

-static inline void pgd_list_add(pgd_t *pgd)

-{

-	struct page *page = virt_to_page(pgd);

-

-	list_add(&page->lru, &pgd_list);

-}

-

-static inline void pgd_list_del(pgd_t *pgd)

-{

-	struct page *page = virt_to_page(pgd);

-

-	list_del(&page->lru);

-}

-

 #define UNSHARED_PTRS_PER_PGD				\

 	(SHARED_KERNEL_PMD ? USER_PTRS_PER_PGD : PTRS_PER_PGD)
--

Add a common arch/x86/mm/pgtable.c file for common pagetable functions.
Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>

---

 arch/x86/mm/Makefile_32      |    2

 arch/x86/mm/Makefile_64      |    2

 arch/x86/mm/pgtable.c        |  234 ++++++++++++++++++++++++++++++++++++++++++

 arch/x86/mm/pgtable_32.c     |  185 ---------------------------------

 include/asm-x86/pgalloc.h    |   19 +++

 include/asm-x86/pgalloc_32.h |   11 -

 include/asm-x86/pgalloc_64.h |   61 ----------

 7 files changed, 255 insertions(+), 259 deletions(-)
diff --git a/arch/x86/mm/Makefile_32 b/arch/x86/mm/Makefile_32

--- a/arch/x86/mm/Makefile_32

+++ b/arch/x86/mm/Makefile_32

@@ -2,7 +2,7 @@

 # Makefile for the linux i386-specific parts of the memory manager.

 #
-obj-y	:= init_32.o pgtable_32.o fault.o ioremap.o extable.o pageattr.o mmap.o

+obj-y	:= init_32.o pgtable.o pgtable_32.o fault.o ioremap.o extable.o pageattr.o mmap.o
 obj-$(CONFIG_NUMA) += discontig_32.o

 obj-$(CONFIG_HUGETLB_PAGE) += hugetlbpage.o

diff --git a/arch/x86/mm/Makefile_64 b/arch/x86/mm/Makefile_64

--- a/arch/x86/mm/Makefile_64

+++ b/arch/x86/mm/Makefile_64

@@ -2,7 +2,7 @@

 # Makefile for the linux x86_64-specific parts of the memory manager.

 #
-obj-y	 := init_64.o fault.o ioremap.o extable.o pageattr.o mmap.o

+obj-y	 := init_64.o fault.o ioremap.o extable.o pageattr.o pgtable.o mmap.o

 obj-$(CONFIG_HUGETLB_PAGE) += hugetlbpage.o

 obj-$(CONFIG_NUMA) += numa_64.o

 obj-$(CONFIG_K8_NUMA) += k8topology_64.o

diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c

new file mode 100644

--- /dev/null

+++ b/arch/x86/mm/pgtable.c

@@ -0,0 +1,235 @@

+#include <asm/pgalloc.h>

+#include <asm/tlb.h>

+

+pte_t *pte_alloc_one_kernel(struct mm_struct *mm, unsigned long address)

+{

+	return (pte_t *)__get_free_page(GFP_KERNEL|__GFP_REPEAT|__GFP_ZERO);

+}

+

+struct page *pte_alloc_one(struct mm_struct *mm, unsigned long address)

+{

+	struct page *pte;

+

+#ifdef CONFIG_HIGHPTE

+	pte = alloc_pages(GFP...
[ message continues ]

randconfig testing found a build breakage on 32-bit, and that got

bisected down to this patch of yours. Config attached. I've dropped both

of your series now, please resend it once you've fixed the problems. And

do more testing please ...
	Ingo

Couldn't reproduce.  What was the failure?
    J

--

oops, i thought i pasted that. It was this:
arch/x86/mm/pgtable.c: In function 'pgd_alloc':

arch/x86/mm/pgtable.c:213: error: implicit declaration of function 'quicklist_alloc'

arch/x86/mm/pgtable.c:213: warning: initialization makes pointer from integer without a cast

arch/x86/mm/pgtable.c:218: error: implicit declaration of function 'quicklist_free'

arch/x86/mm/pgtable.c: In function 'check_pgt_cache':

arch/x86/mm/pgtable.c:233: error: implicit declaration of function 'quicklist_trim'
also, config re-attached. (maybe i messed up the previous one)
	Ingo

Still can't reproduce, but it's a simple case of missing headers.
    J

--

ok, i'll figure it out if/when it happens with your resent queue.
	Ingo

--

I added an explict <linux/quicklist.h> to pgtable.c, so there's no

excuse to still complain.  Will resend the combined series shortly.
    J

--

Common definitions for 2-level pagetable functions.
Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>

---

 arch/x86/mm/pgtable.c        |    6 ++++++

 arch/x86/mm/pgtable_32.c     |    6 ------

 include/asm-x86/pgalloc.h    |   16 ++++++++++++++++

 include/asm-x86/pgalloc_32.h |   17 -----------------

 include/asm-x86/pgalloc_64.h |   19 -------------------

 5 files changed, 22 insertions(+), 42 deletions(-)
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c

--- a/arch/x86/mm/init_64.c

+++ b/arch/x86/mm/init_64.c

@@ -843,11 +843,6 @@

 }

 #endif
-void __pte_free_tlb(struct mmu_gather *tlb, struct page *pte)

-{

-	tlb_remove_page(tlb, pte);

-}

-

 void __pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmd)

 {

 	tlb_remove_page(tlb, virt_to_page(pmd));

diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c

--- a/arch/x86/mm/pgtable.c

+++ b/arch/x86/mm/pgtable.c

@@ -16,6 +16,12 @@

 	pte = alloc_pages(GFP_KERNEL|__GFP_REPEAT|__GFP_ZERO, 0);

 #endif

 	return pte;

+}

+

+void __pte_free_tlb(struct mmu_gather *tlb, struct page *pte)

+{

+	paravirt_release_pt(page_to_pfn(pte));

+	tlb_remove_page(tlb, pte);

 }
 #ifdef CONFIG_X86_64

diff --git a/arch/x86/mm/pgtable_32.c b/arch/x86/mm/pgtable_32.c

--- a/arch/x86/mm/pgtable_32.c

+++ b/arch/x86/mm/pgtable_32.c

@@ -178,12 +178,6 @@

 	__VMALLOC_RESERVE += reserve;

 }
-void __pte_free_tlb(struct mmu_gather *tlb, struct page *pte)

-{

-	paravirt_release_pt(page_to_pfn(pte));

-	tlb_remove_page(tlb, pte);

-}

-

 #ifdef CONFIG_X86_PAE
 void __pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmd)

diff --git a/include/asm-x86/pgalloc.h b/include/asm-x86/pgalloc.h

--- a/include/asm-x86/pgalloc.h

+++ b/include/asm-x86/pgalloc.h

@@ -24,6 +24,22 @@

 pte_t *pte_alloc_one_kernel(struct mm_struct *mm, unsigned long address);

 struct page *pte_alloc_one(struct mm_struct *mm, unsigned long address);
+/* Should really implement gc for free page table pages. This could be

+   done with a reference count in st...
[ message continues ]

i've applied your series.
i've changed that to:
 /*

  * Should really implement gc for free page table pages. This could be

  * done with a reference count in struct page.

  */
	Ingo

--

Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>

---

 arch/x86/mm/pageattr.c       |    2 --

 include/asm-x86/pgalloc.h    |   11 +++++++++++

 include/asm-x86/pgalloc_32.h |   10 ----------

 3 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c

--- a/arch/x86/mm/pageattr.c

+++ b/arch/x86/mm/pageattr.c

@@ -249,9 +249,7 @@

 	address = __pa(address);

 	addr = address & LARGE_PAGE_MASK;

 	pbase = (pte_t *)page_address(base);

-#ifdef CONFIG_X86_32

 	paravirt_alloc_pt(&init_mm, page_to_pfn(base));

-#endif
 	pgprot_val(ref_prot) &= ~_PAGE_NX;

 	for (i = 0; i < PTRS_PER_PTE; i++, addr += PAGE_SIZE)

diff --git a/include/asm-x86/pgalloc.h b/include/asm-x86/pgalloc.h

--- a/include/asm-x86/pgalloc.h

+++ b/include/asm-x86/pgalloc.h

@@ -4,6 +4,16 @@

 #include <linux/threads.h>

 #include <linux/mm.h>		/* for struct page */

 #include <linux/pagemap.h>

+

+#ifdef CONFIG_PARAVIRT

+#include <asm/paravirt.h>

+#else

+#define paravirt_alloc_pt(mm, pfn) do { } while (0)

+#define paravirt_alloc_pd(mm, pfn) do { } while (0)

+#define paravirt_alloc_pd_clone(pfn, clonepfn, start, count) do { } while (0)

+#define paravirt_release_pt(pfn) do { } while (0)

+#define paravirt_release_pd(pfn) do { } while (0)

+#endif
 /*

  * Allocate and free page tables.

diff --git a/include/asm-x86/pgalloc_32.h b/include/asm-x86/pgalloc_32.h

--- a/include/asm-x86/pgalloc_32.h

+++ b/include/asm-x86/pgalloc_32.h

@@ -1,15 +1,5 @@

 #ifndef _I386_PGALLOC_H

 #define _I386_PGALLOC_H

-

-#ifdef CONFIG_PARAVIRT

-#include <asm/paravirt.h>

-#else

-#define paravirt_alloc_pt(mm, pfn) do { } while (0)

-#define paravirt_alloc_pd(mm, pfn) do { } while (0)

-#define paravirt_alloc_pd_clone(pfn, clonepfn, start, count) do { } while (0)

-#define paravirt_release_pt(pfn) do { } while (0)

-#define paravirt_release_pd(pfn) do { } while (0)

-#endif
 static inline void pmd_populate_kernel(struct mm_struct *m...
[ message continues ]

Convert asm-x86/pgalloc_64.h from macros into inline functions.
Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>

---

 include/asm-x86/pgalloc_64.h |   41 ++++++++++++++++++++++++++++++-----------

 1 file changed, 30 insertions(+), 11 deletions(-)
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c

--- a/arch/x86/mm/init_64.c

+++ b/arch/x86/mm/init_64.c

@@ -842,3 +842,18 @@

 	return 0;

 }

 #endif

+

+void __pte_free_tlb(struct mmu_gather *tlb, struct page *pte)

+{

+	tlb_remove_page(tlb, pte);

+}

+

+void __pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmd)

+{

+	tlb_remove_page(tlb, virt_to_page(pmd));

+}

+

+void __pud_free_tlb(struct mmu_gather *tlb, pud_t *pud)

+{

+	tlb_remove_page(tlb, virt_to_page(pud));

+}

diff --git a/include/asm-x86/pgalloc_64.h b/include/asm-x86/pgalloc_64.h

--- a/include/asm-x86/pgalloc_64.h

+++ b/include/asm-x86/pgalloc_64.h

@@ -1,16 +1,24 @@

 #ifndef _X86_64_PGALLOC_H

 #define _X86_64_PGALLOC_H
-#include <asm/pda.h>

 #include <linux/threads.h>

 #include <linux/mm.h>

+#include <asm/pda.h>
-#define pmd_populate_kernel(mm, pmd, pte) \

-		set_pmd(pmd, __pmd(_PAGE_TABLE | __pa(pte)))

-#define pud_populate(mm, pud, pmd) \

-		set_pud(pud, __pud(_PAGE_TABLE | __pa(pmd)))

-#define pgd_populate(mm, pgd, pud) \

-		set_pgd(pgd, __pgd(_PAGE_TABLE | __pa(pud)))

+static inline void pmd_populate_kernel(struct mm_struct *mm, pmd_t *pmd, pte_t *pte)

+{

+	set_pmd(pmd, __pmd(_PAGE_TABLE | __pa(pte)));

+}

+

+static inline void pud_populate(struct mm_struct *mm, pud_t *pud, pmd_t *pmd)

+{

+	set_pud(pud, __pud(_PAGE_TABLE | __pa(pmd)));

+}

+

+static inline void pgd_populate(struct mm_struct *mm, pgd_t *pgd, pud_t *pud)

+{

+	set_pgd(pgd, __pgd(_PAGE_TABLE | __pa(pud)));

+}
 static inline void pmd_populate(struct mm_struct *mm, pmd_t *pmd, struct page *pte)

 {

@@ -109,11 +117,10 @@

 static inline void pte_free(struct page *pte)

 {

 	__free_page(pte);

-}

+}
-#define __pte_free_tlb(tlb,pte) t...
[ message continues ]

Small question: the extern isn't necessary in headers, is it preferred

in linux anyway?
Harvey
--

generally yes, to make it more visually separate from inlines, etc.
	Ingo

--