regression: 2f569af (CONFIG_HIGHPTE vs. sub-page page tables.)

regression: 2f569af (CONFIG_HIGHPTE vs. sub-page page tables.)

[Uwe Kleine-König]

Hello,

I see the following:

	/ # for i in 1 2 3; do grep PageTables /proc/meminfo; done
	PageTables:          4 kB
	PageTables:   4294967292 kB
	PageTables:   4294967284 kB

and I bisected it down to 2f569af (CONFIG_HIGHPTE vs. sub-page page
tables.)  This still happens in 2.6.25-rc3.

I have not investigated further, so I cannot tell if it's only the
output in meminfo that is broken.

This is on ARCH=arm, on a Digi cc9p9360 with ns9xxx_defconfig.

Best regards
Uwe

-- 
Uwe Kleine-König, Software Engineer
Digi International GmbH Branch Breisach, Küferstrasse 8, 79206 Breisach, Germany
Tax: 315/5781/0242 / VAT: DE153662976 / Reg. Amtsgericht Dortmund HRB 13962

Re: regression: 2f569af (CONFIG_HIGHPTE vs. sub-page page tables.)

[Russell King]

On Mon, Feb 25, 2008 at 02:26:48PM +0100, Uwe Kleine-König wrote:
> Hello,
> 
> I see the following:
> 
> 	/ # for i in 1 2 3; do grep PageTables /proc/meminfo; done
> 	PageTables:          4 kB
> 	PageTables:   4294967292 kB
> 	PageTables:   4294967284 kB
> 
> and I bisected it down to 2f569af (CONFIG_HIGHPTE vs. sub-page page
> tables.)  This still happens in 2.6.25-rc3.
> 
> I have not investigated further, so I cannot tell if it's only the
> output in meminfo that is broken.
> 
> This is on ARCH=arm, on a Digi cc9p9360 with ns9xxx_defconfig.

Note that there's been other reports of this on other ARM platforms as
well.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: regression: 2f569af (CONFIG_HIGHPTE vs. sub-page page tables.)

[Uwe Kleine-König]

Hello,

Russell King wrote:
> On Mon, Feb 25, 2008 at 02:26:48PM +0100, Uwe Kleine-König wrote:
> > Hello,
> > 
> > I see the following:
> > 
> > 	/ # for i in 1 2 3; do grep PageTables /proc/meminfo; done
> > 	PageTables:          4 kB
> > 	PageTables:   4294967292 kB
> > 	PageTables:   4294967284 kB
> > 
> > and I bisected it down to 2f569af (CONFIG_HIGHPTE vs. sub-page page
> > tables.)  This still happens in 2.6.25-rc3.
> > 
> > I have not investigated further, so I cannot tell if it's only the
> > output in meminfo that is broken.
> > 
> > This is on ARCH=arm, on a Digi cc9p9360 with ns9xxx_defconfig.
> 
> Note that there's been other reports of this on other ARM platforms as
> well.
I didn't see these.  @Russell: can you point me to these reports?

With the patch below, I get two times printascii('-'), zero times
printascii('+') and the following two stacktraces:

	[<c0120330>] (dump_stack+0x0/0x14) from [<c01232ec>] (free_pgd_slow+0xa4/0x10c)
	[<c0123248>] (free_pgd_slow+0x0/0x10c) from [<c01318d8>] (__mmdrop+0x24/0x54)
	 r7:c1c88000 r6:c14d8cc0 r5:c1c4cc40 r4:c14d8cc0
	[<c01318b4>] (__mmdrop+0x0/0x54) from [<c01319b4>] (mmput+0xac/0xc4)
	 r4:c14d8cc0
	[<c0131908>] (mmput+0x0/0xc4) from [<c018318c>] (flush_old_exec+0x320/0x680)
	 r4:c14d8b60
	[<c0182e6c>] (flush_old_exec+0x0/0x680) from [<c01b0460>] (load_elf_binary+0x3d4/0x15d0)
	[<c01b008c>] (load_elf_binary+0x0/0x15d0) from [<c0183af4>] (search_binary_handler+0xa4/0x1fc)
	[<c0183a50>] (search_binary_handler+0x0/0x1fc) from [<c0183db0>] (do_execve+0x164/0x188)
	[<c0183c4c>] (do_execve+0x0/0x188) from [<c011fb94>] (sys_execve+0x3c/0x5c)
	 r8:c011cb44 r7:c1c24000 r6:c1c89fb0 r5:0008cba8 r4:c1c24000
	[<c011fb58>] (sys_execve+0x0/0x5c) from [<c011c9a0>] (ret_fast_syscall+0x0/0x2c)
	 r7:0000000b r6:00000001 r5:0008cb88 r4:0008cba0

	[<c0120330>] (dump_stack+0x0/0x14) from [<c01232ec>] (free_pgd_slow+0xa4/0x10c)
	[<c0123248>] (free_pgd_slow+0x0/0x10c) from [<c01318d8>] (__mmdrop+0x24/0x54)
	 r7:c1c4cc40 r6:c14d8e20 r5:00000040 r4:c14d8b60
	[<c01318b4>] (__mmdrop+0x0/0x54) from [<c012d204>] (finish_task_switch+0x8c/0x90)
	 r4:c1c4cc40
	[<c012d178>] (finish_task_switch+0x0/0x90) from [<c0215614>] (schedule+0x1c0/0x2b0)
	 r5:c1c13bc0 r4:c14d8b60
	[<c0215454>] (schedule+0x0/0x2b0) from [<c01363d4>] (do_wait+0x278/0xbb0)
	[<c013615c>] (do_wait+0x0/0xbb0) from [<c0136da4>] (sys_wait4+0x98/0xd4)
	[<c0136d0c>] (sys_wait4+0x0/0xd4) from [<c011c9a0>] (ret_fast_syscall+0x0/0x2c)
	 r8:c011cb44 r7:00000072 r6:00075989 r5:00000000 r4:00090288

when executing /bin/true.  I assume these are the two entries that go
missing?  Maybe this helps someone with a deeper understanding?

IMHO the last hunk is worth to make it into mainstream.

Best regards
Uwe

diff --git a/include/asm-arm/pgalloc.h b/include/asm-arm/pgalloc.h
index 163b030..0c85d94 100644
--- a/include/asm-arm/pgalloc.h
+++ b/include/asm-arm/pgalloc.h
@@ -75,7 +75,7 @@ pte_alloc_one(struct mm_struct *mm, unsigned long addr)
 	if (pte) {
 		void *page = page_address(pte);
 		clean_dcache_area(page, sizeof(pte_t) * PTRS_PER_PTE);
-		pgtable_page_ctor(pte);
+		//pgtable_page_ctor(pte);
 	}
 
 	return pte;
@@ -94,7 +94,7 @@ static inline void pte_free_kernel(struct mm_struct *mm, pte_t *pte)
 
 static inline void pte_free(struct mm_struct *mm, pgtable_t pte)
 {
-	pgtable_page_dtor(pte);
+	//pgtable_page_dtor(pte);
 	__free_page(pte);
 }
 
diff --git a/include/linux/vmstat.h b/include/linux/vmstat.h
index 75370ec..5553d3f 100644
--- a/include/linux/vmstat.h
+++ b/include/linux/vmstat.h
@@ -225,8 +225,13 @@ static inline void __mod_zone_page_state(struct zone *zone,
 	zone_page_state_add(delta, zone, item);
 }
 
+void printch(char);
 static inline void __inc_zone_state(struct zone *zone, enum zone_stat_item item)
 {
+	if (item == NR_PAGETABLE) {
+		printch('+');
+	}
+
 	atomic_long_inc(&zone->vm_stat[item]);
 	atomic_long_inc(&vm_stat[item]);
 }
@@ -239,6 +244,11 @@ static inline void __inc_zone_page_state(struct page *page,
 
 static inline void __dec_zone_state(struct zone *zone, enum zone_stat_item item)
 {
+	if (item == NR_PAGETABLE) {
+		printch('-');
+		dump_stack();
+	}
+
 	atomic_long_dec(&zone->vm_stat[item]);
 	atomic_long_dec(&vm_stat[item]);
 }
@@ -246,8 +256,7 @@ static inline void __dec_zone_state(struct zone *zone, enum zone_stat_item item)
 static inline void __dec_zone_page_state(struct page *page,
 			enum zone_stat_item item)
 {
-	atomic_long_dec(&page_zone(page)->vm_stat[item]);
-	atomic_long_dec(&vm_stat[item]);
+	__dec_zone_state(page_zone(page), item);
 }
 
 /*

-- 
Uwe Kleine-König, Software Engineer
Digi International GmbH Branch Breisach, Küferstrasse 8, 79206 Breisach, Germany
Tax: 315/5781/0242 / VAT: DE153662976 / Reg. Amtsgericht Dortmund HRB 13962

let __dec_zone_page_state use __dec_zone_state

[Uwe Kleine-König]

This removes code duplication and makes __dec_zone_page_state look like
__inc_zone_page_state.

Signed-off-by: Uwe Kleine-König <Uwe.Kleine-Koenig@digi.com>
Cc: Christoph Lameter <clameter@sgi.com>
---
> IMHO the last hunk is worth to make it into mainstream.
So here comes the patch ...

Best regards
Uwe

 include/linux/vmstat.h |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)


diff --git a/include/linux/vmstat.h b/include/linux/vmstat.h
index 75370ec..9f1b4b4 100644
--- a/include/linux/vmstat.h
+++ b/include/linux/vmstat.h
@@ -246,8 +246,7 @@ static inline void __dec_zone_state(struct zone *zone, enum zone_stat_item item)
 static inline void __dec_zone_page_state(struct page *page,
 			enum zone_stat_item item)
 {
-	atomic_long_dec(&page_zone(page)->vm_stat[item]);
-	atomic_long_dec(&vm_stat[item]);
+	__dec_zone_state(page_zone(page), item);
 }
 
 /*
-- 
1.5.4.3

-- 
Uwe Kleine-König, Software Engineer
Digi International GmbH Branch Breisach, Küferstrasse 8, 79206 Breisach, Germany
Tax: 315/5781/0242 / VAT: DE153662976 / Reg. Amtsgericht Dortmund HRB 13962

Re: let __dec_zone_page_state use __dec_zone_state

[Christoph Lameter]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 197 bytes --]

On Mon, 25 Feb 2008, Uwe Kleine-König wrote:

> This removes code duplication and makes __dec_zone_page_state look like
> __inc_zone_page_state.

Acked-by: Christoph Lameter <clameter@sgi.com>

Re: let __dec_zone_page_state use __dec_zone_state

[Uwe Kleine-König]

Hello Christoph,

Christoph Lameter wrote:
> On Mon, 25 Feb 2008, Uwe Kleine-König wrote:
> 
> > This removes code duplication and makes __dec_zone_page_state look like
> > __inc_zone_page_state.
> 
> Acked-by: Christoph Lameter <clameter@sgi.com>
Unless Linus or Andrew take that patch I think the best path for that
patch into mainline is going over your tree.

Best regards
Uwe

-- 
Uwe Kleine-König, Software Engineer
Digi International GmbH Branch Breisach, Küferstrasse 8, 79206 Breisach, Germany
Tax: 315/5781/0242 / VAT: DE153662976 / Reg. Amtsgericht Dortmund HRB 13962

Re: let __dec_zone_page_state use __dec_zone_state

[Christoph Lameter]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 512 bytes --]

On Fri, 29 Feb 2008, Uwe Kleine-König wrote:

> Hello Christoph,
> 
> Christoph Lameter wrote:
> > On Mon, 25 Feb 2008, Uwe Kleine-König wrote:
> > 
> > > This removes code duplication and makes __dec_zone_page_state look like
> > > __inc_zone_page_state.
> > 
> > Acked-by: Christoph Lameter <clameter@sgi.com>
> Unless Linus or Andrew take that patch I think the best path for that
> patch into mainline is going over your tree.

I only do slab stuff via my tree. This has to go through Andrew.

Re: regression: 2f569af (CONFIG_HIGHPTE vs. sub-page page tables.)

[Martin Schwidefsky]

On Mon, 2008-02-25 at 14:26 +0100, Uwe Kleine-König wrote:
> I see the following:
> 
> 	/ # for i in 1 2 3; do grep PageTables /proc/meminfo; done
> 	PageTables:          4 kB
> 	PageTables:   4294967292 kB
> 	PageTables:   4294967284 kB
> 
> and I bisected it down to 2f569af (CONFIG_HIGHPTE vs. sub-page page
> tables.)  This still happens in 2.6.25-rc3.
> 
> I have not investigated further, so I cannot tell if it's only the
> output in meminfo that is broken.
> 
> This is on ARCH=arm, on a Digi cc9p9360 with ns9xxx_defconfig.

Hmm, not good. The number obviously went negative. There is an imbalance
in the number of pgtable_page_ctor vs. pgtable_page_dtor. Could you try
this patch and watch for warnings?

-- 
blue skies,
  Martin.

"Reality continues to ruin my life." - Calvin.

---
diff --git a/include/asm-arm/pgalloc.h b/include/asm-arm/pgalloc.h
index 163b030..eeb973a 100644
--- a/include/asm-arm/pgalloc.h
+++ b/include/asm-arm/pgalloc.h
@@ -94,6 +94,7 @@ static inline void pte_free_kernel(struct mm_struct *mm, pte_t *pte)
 
 static inline void pte_free(struct mm_struct *mm, pgtable_t pte)
 {
+	WARN_ON(!pte);
 	pgtable_page_dtor(pte);
 	__free_page(pte);
 }

Re: regression: 2f569af (CONFIG_HIGHPTE vs. sub-page page tables.)

[Uwe Kleine-König]

Hello Martin,

Martin Schwidefsky wrote:
> On Mon, 2008-02-25 at 14:26 +0100, Uwe Kleine-König wrote:
> > I see the following:
> > 
> > 	/ # for i in 1 2 3; do grep PageTables /proc/meminfo; done
> > 	PageTables:          4 kB
> > 	PageTables:   4294967292 kB
> > 	PageTables:   4294967284 kB
> > 
> > and I bisected it down to 2f569af (CONFIG_HIGHPTE vs. sub-page page
> > tables.)  This still happens in 2.6.25-rc3.
> > 
> > I have not investigated further, so I cannot tell if it's only the
> > output in meminfo that is broken.
> > 
> > This is on ARCH=arm, on a Digi cc9p9360 with ns9xxx_defconfig.
> 
> Hmm, not good. The number obviously went negative. There is an imbalance
> in the number of pgtable_page_ctor vs. pgtable_page_dtor. Could you try
> this patch and watch for warnings?
It doesn't trigger.

And your assumption that there is an imbalance between pgtable_page_ctor
and pgtable_page_dtor is wrong, too.  I added some debug output to both
of the two functions and for one grep both are executed seven times.

Moreover a "grep PageTables /proc/meminfo" resulted in 

	9x __dec_zone_state with item == NR_PAGETABLE
	7x __inc_zone_state with item == NR_PAGETABLE

All those 16 calles where done via __(inc|dec)_zone_page_state.

... some time later ...

I got it.  This code is currently in free_pgd_slow():

        pte = pmd_page(*pmd);
        pmd_clear(pmd);
        dec_zone_page_state(virt_to_page((unsigned long *)pgd), NR_PAGETABLE);
        pte_lock_deinit(pte);
        pte_free(mm, pte);
        pmd_free(mm, pmd);

So because (since 2f569af) pte_free does dec_zone_page_state and
pte_lock_deinit, these two happen twice here.

Removing the calls to dec_zone_page_state() and pte_lock_deinit() in
free_pgd_slow() fixed it for me.

For a complete fix we might want to change the type of pte?

Best regards
Uwe

-- 
Uwe Kleine-König, Software Engineer
Digi International GmbH Branch Breisach, Küferstrasse 8, 79206 Breisach, Germany
Tax: 315/5781/0242 / VAT: DE153662976 / Reg. Amtsgericht Dortmund HRB 13962

Re: regression: 2f569af (CONFIG_HIGHPTE vs. sub-page page tables.)

[Martin Schwidefsky]

Hi Uwe,

On Tue, 2008-02-26 at 15:38 +0100, Uwe Kleine-König wrote:
> Moreover a "grep PageTables /proc/meminfo" resulted in 
> 
> 	9x __dec_zone_state with item == NR_PAGETABLE
> 	7x __inc_zone_state with item == NR_PAGETABLE
> 
> All those 16 calles where done via __(inc|dec)_zone_page_state.
> 
> ... some time later ...
> 
> I got it.  This code is currently in free_pgd_slow():
> 
>         pte = pmd_page(*pmd);
>         pmd_clear(pmd);
>         dec_zone_page_state(virt_to_page((unsigned long *)pgd), NR_PAGETABLE);
>         pte_lock_deinit(pte);
>         pte_free(mm, pte);
>         pmd_free(mm, pmd);
>
> So because (since 2f569af) pte_free does dec_zone_page_state and
> pte_lock_deinit, these two happen twice here.

Ah, good. I wonder if there are any other pte_lock_deinit hidden in the
arch code... nope, arm is the only architecture that does it.

> Removing the calls to dec_zone_page_state() and pte_lock_deinit() in
> free_pgd_slow() fixed it for me.

That is imho the best way to fix it.

> For a complete fix we might want to change the type of pte?

You mean instead of a "struct page *" use a pgtable_t ? Yes, that would
be cleaner even if it is the same type.

-- 
blue skies,
  Martin.

"Reality continues to ruin my life." - Calvin.

Re: regression: 2f569af (CONFIG_HIGHPTE vs. sub-page page tables.)

[Russell King]

On Tue, Feb 26, 2008 at 06:57:46PM +0100, Martin Schwidefsky wrote:
> Hi Uwe,
> 
> On Tue, 2008-02-26 at 15:38 +0100, Uwe Kleine-König wrote:
> > Moreover a "grep PageTables /proc/meminfo" resulted in 
> > 
> > 	9x __dec_zone_state with item == NR_PAGETABLE
> > 	7x __inc_zone_state with item == NR_PAGETABLE
> > 
> > All those 16 calles where done via __(inc|dec)_zone_page_state.
> > 
> > ... some time later ...
> > 
> > I got it.  This code is currently in free_pgd_slow():
> > 
> >         pte = pmd_page(*pmd);
> >         pmd_clear(pmd);
> >         dec_zone_page_state(virt_to_page((unsigned long *)pgd), NR_PAGETABLE);
> >         pte_lock_deinit(pte);
> >         pte_free(mm, pte);
> >         pmd_free(mm, pmd);
> >
> > So because (since 2f569af) pte_free does dec_zone_page_state and
> > pte_lock_deinit, these two happen twice here.
> 
> Ah, good. I wonder if there are any other pte_lock_deinit hidden in the
> arch code... nope, arm is the only architecture that does it.

That's because we _may_ have a page table to map the vectors page at
address 0 (which needs to exist all the time that the page tables are
in use by the CPU.)

> > Removing the calls to dec_zone_page_state() and pte_lock_deinit() in
> > free_pgd_slow() fixed it for me.
> 
> That is imho the best way to fix it.

... but at what cost?

> > For a complete fix we might want to change the type of pte?
> 
> You mean instead of a "struct page *" use a pgtable_t ? Yes, that would
> be cleaner even if it is the same type.

Would someone mind investigating how the code ended up in this mess in
the first place, so we can avoid this kind of thing in the future?

It looks like the pte_lock_deinit() appeared in
4c21e2f2441dc5fbb957b030333f5a3f2d02dea7.

The dec_zone_page_state() used to be dec_page_state(nr_page_table_pages);
which then became a dec_zone_page_state() in
df849a1529c106f7460e51479ca78fe07b07dc8c.

Both changes of those changes on their own look correct.

Ah, is it that someone skipped over ARM when they changed the page table
freeing code?  Yes - 2f569afd9ced9ebec9a6eb3dbf6f83429be0a7b4 has:

diff --git a/mm/memory.c b/mm/memory.c
index 153a54b..e5628a5 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -134,11 +134,9 @@ void pmd_clear_bad(pmd_t *pmd)
  */
 static void free_pte_range(struct mmu_gather *tlb, pmd_t *pmd)
 {
-       struct page *page = pmd_page(*pmd);
+       pgtable_t token = pmd_pgtable(*pmd);
        pmd_clear(pmd);
-       pte_lock_deinit(page);
-       pte_free_tlb(tlb, page);
-       dec_zone_page_state(page, NR_PAGETABLE);
+       pte_free_tlb(tlb, token);
        tlb->mm->nr_ptes--;
 }

and no corresponding change to ARM.  So, ARM needs to be fixed with this
change before some other subtle breakage occurs.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: regression: 2f569af (CONFIG_HIGHPTE vs. sub-page page tables.)

[Uwe Kleine-König]

Hello Russell,

(I didn't add you to the recipients of that mail, because your mail had
a Reply-To: header.  So I assume that's what you want.)

> > > Removing the calls to dec_zone_page_state() and pte_lock_deinit() in
> > > free_pgd_slow() fixed it for me.
> > 
> > That is imho the best way to fix it.
> 
> ... but at what cost?
I don't see any costs.  Can you please enlighten me?

> > > For a complete fix we might want to change the type of pte?
> > 
> > You mean instead of a "struct page *" use a pgtable_t ? Yes, that would
> > be cleaner even if it is the same type.
> 
> Would someone mind investigating how the code ended up in this mess in
> the first place, so we can avoid this kind of thing in the future?
> 
> It looks like the pte_lock_deinit() appeared in
> 4c21e2f2441dc5fbb957b030333f5a3f2d02dea7.
> 
> The dec_zone_page_state() used to be dec_page_state(nr_page_table_pages);
> which then became a dec_zone_page_state() in
> df849a1529c106f7460e51479ca78fe07b07dc8c.
> 
> Both changes of those changes on their own look correct.
> 
> Ah, is it that someone skipped over ARM when they changed the page table
> freeing code?  Yes - 2f569afd9ced9ebec9a6eb3dbf6f83429be0a7b4 has:
If you look at the subject line or the earlier mails in that thread,
that is the exact commit I blamed, too.

So below comes a patch.  In  my eyes this has to go in before 2.6.25.

Best regards
Uwe

---->8----
Fix freeing of page tables for ARM in free_pgd_slow

Since 2f569af (CONFIG_HIGHPTE vs. sub-page page tables.) pte_free() calls
pte_lock_deinit() and dec_zone_page_state().  So free_pgd_slow must not call
the latter two when calling the first.

Signed-off-by: Uwe Kleine-König <Uwe.Kleine-Koenig@digi.com>
Cc: Russell King <rmk@arm.linux.org.uk>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
 arch/arm/mm/pgd.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mm/pgd.c b/arch/arm/mm/pgd.c
index 500c961..e0f19ab 100644
--- a/arch/arm/mm/pgd.c
+++ b/arch/arm/mm/pgd.c
@@ -75,7 +75,7 @@ no_pgd:
 void free_pgd_slow(struct mm_struct *mm, pgd_t *pgd)
 {
 	pmd_t *pmd;
-	struct page *pte;
+	pgtable_t pte;
 
 	if (!pgd)
 		return;
@@ -90,10 +90,8 @@ void free_pgd_slow(struct mm_struct *mm, pgd_t *pgd)
 		goto free;
 	}
 
-	pte = pmd_page(*pmd);
+	pte = pmd_pgtable(*pmd);
 	pmd_clear(pmd);
-	dec_zone_page_state(virt_to_page((unsigned long *)pgd), NR_PAGETABLE);
-	pte_lock_deinit(pte);
 	pte_free(mm, pte);
 	pmd_free(mm, pmd);
 free:
-- 
1.5.4.3


-- 
Uwe Kleine-König, Software Engineer
Digi International GmbH Branch Breisach, Küferstrasse 8, 79206 Breisach, Germany
Tax: 315/5781/0242 / VAT: DE153662976 / Reg. Amtsgericht Dortmund HRB 13962

Re: regression: 2f569af (CONFIG_HIGHPTE vs. sub-page page tables.)

[Martin Schwidefsky]

On Tue, 2008-02-26 at 19:49 +0000, Russell King wrote:
> > > For a complete fix we might want to change the type of pte?
> > 
> > You mean instead of a "struct page *" use a pgtable_t ? Yes, that would
> > be cleaner even if it is the same type.
> 
> Would someone mind investigating how the code ended up in this mess in
> the first place, so we can avoid this kind of thing in the future?

One thing I have learned is that if I ever have to do a change over all
architectures again I will split the patch into the common part and the
arch parts and send it to the respective maintainers directly. Adding
linux-arch seems NOT to be enough to get the attention of the arch
maintainers (having the patch in -mm for 3 month is not doing the trick
either).

> It looks like the pte_lock_deinit() appeared in
> 4c21e2f2441dc5fbb957b030333f5a3f2d02dea7.
> 
> The dec_zone_page_state() used to be dec_page_state(nr_page_table_pages);
> which then became a dec_zone_page_state() in
> df849a1529c106f7460e51479ca78fe07b07dc8c.
> 
> Both changes of those changes on their own look correct.
> 
> Ah, is it that someone skipped over ARM when they changed the page table
> freeing code?  Yes - 2f569afd9ced9ebec9a6eb3dbf6f83429be0a7b4 has:

Yes, sorry. I need one of these brown paper bags..

-- 
blue skies,
  Martin.

"Reality continues to ruin my life." - Calvin.