linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mm: don't count preallocated pmds
@ 2015-03-18 15:16 Mikulas Patocka
  2015-03-18 16:12 ` Kirill A. Shutemov
  0 siblings, 1 reply; 6+ messages in thread
From: Mikulas Patocka @ 2015-03-18 15:16 UTC (permalink / raw)
  To: Kirill A. Shutemov, linux-mm, linux-parisc; +Cc: jejb, dave.anglin

Hi

Here I'm sending a patch that fixes numerous "BUG: non-zero nr_pmds on 
freeing mm: -1" errors on 64-bit PA-RISC kernel.

I think the patch posted here 
http://www.spinics.net/lists/linux-parisc/msg05981.html is incorrect, it 
wouldn't work if the affected address range is freed and allocated 
multiple times.
	- 1. alloc pgd with built-in pmd, the count of pmds is 1
	- 2. free the range covered by the built-in pmd, the count of pmds 
		is 0, but the built-in pmd is still present
	- 3. alloc some memory in the range affected by the built-in pmd, 
		the count of pmds is still 0
	- 4. free the range covered by the built-in pmd, the counter 
		underflows to -1

Mikulas


From: Mikulas Patocka <mpatocka@redhat.com>

The patch dc6c9a35b66b520cf67e05d8ca60ebecad3b0479 that counts pmds 
allocated for a process introduced a bug on 64-bit PA-RISC kernels. There 
are many "BUG: non-zero nr_pmds on freeing mm: -1" messages.

The PA-RISC architecture preallocates one pmd with each pgd. This
preallocated pmd can never be freed - pmd_free does nothing when it is
called with this pmd. When the kernel attempts to free this preallocated
pmd, it decreases the count of allocated pmds. The result is that the
counter underflows and this error is reported.

This patch fixes the bug by introducing a macro pmd_preallocated and
making sure that the counter is not decremented when this preallocated pmd
is freed.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 arch/parisc/include/asm/pgalloc.h |    2 ++
 mm/memory.c                       |    5 ++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

Index: linux-4.0-rc4/arch/parisc/include/asm/pgalloc.h
===================================================================
--- linux-4.0-rc4.orig/arch/parisc/include/asm/pgalloc.h	2015-03-18 15:31:10.000000000 +0100
+++ linux-4.0-rc4/arch/parisc/include/asm/pgalloc.h	2015-03-18 15:33:20.000000000 +0100
@@ -81,6 +81,8 @@ static inline void pmd_free(struct mm_st
 	free_pages((unsigned long)pmd, PMD_ORDER);
 }
 
+#define pmd_preallocated(pmd)	(pmd_flag(*(pmd)) & PxD_FLAG_ATTACHED)
+
 #else
 
 /* Two Level Page Table Support for pmd's */
Index: linux-4.0-rc4/mm/memory.c
===================================================================
--- linux-4.0-rc4.orig/mm/memory.c	2015-03-18 15:30:42.000000000 +0100
+++ linux-4.0-rc4/mm/memory.c	2015-03-18 15:32:33.000000000 +0100
@@ -427,8 +427,11 @@ static inline void free_pmd_range(struct
 
 	pmd = pmd_offset(pud, start);
 	pud_clear(pud);
+#ifdef pmd_preallocated
+	if (!pmd_preallocated(pmd))
+#endif
+		mm_dec_nr_pmds(tlb->mm);
 	pmd_free_tlb(tlb, pmd, start);
-	mm_dec_nr_pmds(tlb->mm);
 }
 
 static inline void free_pud_range(struct mmu_gather *tlb, pgd_t *pgd,

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] mm: don't count preallocated pmds
  2015-03-18 15:16 [PATCH] mm: don't count preallocated pmds Mikulas Patocka
@ 2015-03-18 16:12 ` Kirill A. Shutemov
  2015-03-18 16:25   ` Mikulas Patocka
  0 siblings, 1 reply; 6+ messages in thread
From: Kirill A. Shutemov @ 2015-03-18 16:12 UTC (permalink / raw)
  To: Mikulas Patocka
  Cc: Kirill A. Shutemov, linux-mm, linux-parisc, jejb, dave.anglin

On Wed, Mar 18, 2015 at 11:16:42AM -0400, Mikulas Patocka wrote:
> Hi
> 
> Here I'm sending a patch that fixes numerous "BUG: non-zero nr_pmds on 
> freeing mm: -1" errors on 64-bit PA-RISC kernel.
> 
> I think the patch posted here 
> http://www.spinics.net/lists/linux-parisc/msg05981.html is incorrect, it 
> wouldn't work if the affected address range is freed and allocated 
> multiple times.
> 	- 1. alloc pgd with built-in pmd, the count of pmds is 1
> 	- 2. free the range covered by the built-in pmd, the count of pmds 
> 		is 0, but the built-in pmd is still present

Hm. Okay. I didn't realize you have special case in pmd_clear() for these
pmds.

What about adding mm_inc_nr_pmds() in pmd_clear() for PxD_FLAG_ATTACHED
to compensate mm_dec_nr_pmds() in free_pmd_range()?

I don't like pmd_preallocated() in generic code. It's too specific to
parisc.

> 	- 3. alloc some memory in the range affected by the built-in pmd, 
> 		the count of pmds is still 0
> 	- 4. free the range covered by the built-in pmd, the counter 
> 		underflows to -1
> 
> Mikulas
> 
> 
> From: Mikulas Patocka <mpatocka@redhat.com>
> 
> The patch dc6c9a35b66b520cf67e05d8ca60ebecad3b0479 that counts pmds 
> allocated for a process introduced a bug on 64-bit PA-RISC kernels. There 
> are many "BUG: non-zero nr_pmds on freeing mm: -1" messages.
> 
> The PA-RISC architecture preallocates one pmd with each pgd. This
> preallocated pmd can never be freed - pmd_free does nothing when it is
> called with this pmd. When the kernel attempts to free this preallocated
> pmd, it decreases the count of allocated pmds. The result is that the
> counter underflows and this error is reported.
> 
> This patch fixes the bug by introducing a macro pmd_preallocated and
> making sure that the counter is not decremented when this preallocated pmd
> is freed.
> 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> 
> ---
>  arch/parisc/include/asm/pgalloc.h |    2 ++
>  mm/memory.c                       |    5 ++++-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> Index: linux-4.0-rc4/arch/parisc/include/asm/pgalloc.h
> ===================================================================
> --- linux-4.0-rc4.orig/arch/parisc/include/asm/pgalloc.h	2015-03-18 15:31:10.000000000 +0100
> +++ linux-4.0-rc4/arch/parisc/include/asm/pgalloc.h	2015-03-18 15:33:20.000000000 +0100
> @@ -81,6 +81,8 @@ static inline void pmd_free(struct mm_st
>  	free_pages((unsigned long)pmd, PMD_ORDER);
>  }
>  
> +#define pmd_preallocated(pmd)	(pmd_flag(*(pmd)) & PxD_FLAG_ATTACHED)
> +
>  #else
>  
>  /* Two Level Page Table Support for pmd's */
> Index: linux-4.0-rc4/mm/memory.c
> ===================================================================
> --- linux-4.0-rc4.orig/mm/memory.c	2015-03-18 15:30:42.000000000 +0100
> +++ linux-4.0-rc4/mm/memory.c	2015-03-18 15:32:33.000000000 +0100
> @@ -427,8 +427,11 @@ static inline void free_pmd_range(struct
>  
>  	pmd = pmd_offset(pud, start);
>  	pud_clear(pud);
> +#ifdef pmd_preallocated
> +	if (!pmd_preallocated(pmd))
> +#endif
> +		mm_dec_nr_pmds(tlb->mm);
>  	pmd_free_tlb(tlb, pmd, start);
> -	mm_dec_nr_pmds(tlb->mm);
>  }
>  
>  static inline void free_pud_range(struct mmu_gather *tlb, pgd_t *pgd,
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

-- 
 Kirill A. Shutemov

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] mm: don't count preallocated pmds
  2015-03-18 16:12 ` Kirill A. Shutemov
@ 2015-03-18 16:25   ` Mikulas Patocka
  2015-03-18 16:53     ` Kirill A. Shutemov
  0 siblings, 1 reply; 6+ messages in thread
From: Mikulas Patocka @ 2015-03-18 16:25 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: Kirill A. Shutemov, linux-mm, linux-parisc, jejb, dave.anglin



On Wed, 18 Mar 2015, Kirill A. Shutemov wrote:

> On Wed, Mar 18, 2015 at 11:16:42AM -0400, Mikulas Patocka wrote:
> > Hi
> > 
> > Here I'm sending a patch that fixes numerous "BUG: non-zero nr_pmds on 
> > freeing mm: -1" errors on 64-bit PA-RISC kernel.
> > 
> > I think the patch posted here 
> > http://www.spinics.net/lists/linux-parisc/msg05981.html is incorrect, it 
> > wouldn't work if the affected address range is freed and allocated 
> > multiple times.
> > 	- 1. alloc pgd with built-in pmd, the count of pmds is 1
> > 	- 2. free the range covered by the built-in pmd, the count of pmds 
> > 		is 0, but the built-in pmd is still present
> 
> Hm. Okay. I didn't realize you have special case in pmd_clear() for these
> pmds.
> 
> What about adding mm_inc_nr_pmds() in pmd_clear() for PxD_FLAG_ATTACHED
> to compensate mm_dec_nr_pmds() in free_pmd_range()?

pmd_clear clears one entry in the pmd, it wouldn't work. You need to add 
it to pgd_clear. That clears the pointer to the pmd (and does nothing if 
it is asked to clear the pointer to the preallocated pmd). But pgd_clear 
doesn't receive the pointer to mm.

> I don't like pmd_preallocated() in generic code. It's too specific to
> parisc.

The question is if it is better to use pmd_preallocated, or pass the 
pointer to the mm to pgd_clear (that would affect all architectures).

Mikulas

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] mm: don't count preallocated pmds
  2015-03-18 16:25   ` Mikulas Patocka
@ 2015-03-18 16:53     ` Kirill A. Shutemov
  2015-03-18 17:42       ` [PATCH v2] " Mikulas Patocka
  0 siblings, 1 reply; 6+ messages in thread
From: Kirill A. Shutemov @ 2015-03-18 16:53 UTC (permalink / raw)
  To: Mikulas Patocka
  Cc: Kirill A. Shutemov, linux-mm, linux-parisc, jejb, dave.anglin

On Wed, Mar 18, 2015 at 12:25:11PM -0400, Mikulas Patocka wrote:
> 
> 
> On Wed, 18 Mar 2015, Kirill A. Shutemov wrote:
> 
> > On Wed, Mar 18, 2015 at 11:16:42AM -0400, Mikulas Patocka wrote:
> > > Hi
> > > 
> > > Here I'm sending a patch that fixes numerous "BUG: non-zero nr_pmds on 
> > > freeing mm: -1" errors on 64-bit PA-RISC kernel.
> > > 
> > > I think the patch posted here 
> > > http://www.spinics.net/lists/linux-parisc/msg05981.html is incorrect, it 
> > > wouldn't work if the affected address range is freed and allocated 
> > > multiple times.
> > > 	- 1. alloc pgd with built-in pmd, the count of pmds is 1
> > > 	- 2. free the range covered by the built-in pmd, the count of pmds 
> > > 		is 0, but the built-in pmd is still present
> > 
> > Hm. Okay. I didn't realize you have special case in pmd_clear() for these
> > pmds.
> > 
> > What about adding mm_inc_nr_pmds() in pmd_clear() for PxD_FLAG_ATTACHED
> > to compensate mm_dec_nr_pmds() in free_pmd_range()?
> 
> pmd_clear clears one entry in the pmd, it wouldn't work. You need to add 
> it to pgd_clear. That clears the pointer to the pmd (and does nothing if 
> it is asked to clear the pointer to the preallocated pmd). But pgd_clear 
> doesn't receive the pointer to mm.

I meant pmd_free(), not pmd_clear(). This should work fine.

> 
> > I don't like pmd_preallocated() in generic code. It's too specific to
> > parisc.
> 
> The question is if it is better to use pmd_preallocated, or pass the 
> pointer to the mm to pgd_clear (that would affect all architectures).
> 
> Mikulas

-- 
 Kirill A. Shutemov

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v2] mm: don't count preallocated pmds
  2015-03-18 16:53     ` Kirill A. Shutemov
@ 2015-03-18 17:42       ` Mikulas Patocka
  2015-03-18 18:54         ` Kirill A. Shutemov
  0 siblings, 1 reply; 6+ messages in thread
From: Mikulas Patocka @ 2015-03-18 17:42 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: Kirill A. Shutemov, linux-mm, linux-parisc, jejb, dave.anglin



On Wed, 18 Mar 2015, Kirill A. Shutemov wrote:

> On Wed, Mar 18, 2015 at 12:25:11PM -0400, Mikulas Patocka wrote:
> > 
> > 
> > On Wed, 18 Mar 2015, Kirill A. Shutemov wrote:
> > 
> > > On Wed, Mar 18, 2015 at 11:16:42AM -0400, Mikulas Patocka wrote:
> > > > Hi
> > > > 
> > > > Here I'm sending a patch that fixes numerous "BUG: non-zero nr_pmds on 
> > > > freeing mm: -1" errors on 64-bit PA-RISC kernel.
> > > > 
> > > > I think the patch posted here 
> > > > http://www.spinics.net/lists/linux-parisc/msg05981.html is incorrect, it 
> > > > wouldn't work if the affected address range is freed and allocated 
> > > > multiple times.
> > > > 	- 1. alloc pgd with built-in pmd, the count of pmds is 1
> > > > 	- 2. free the range covered by the built-in pmd, the count of pmds 
> > > > 		is 0, but the built-in pmd is still present
> > > 
> > > Hm. Okay. I didn't realize you have special case in pmd_clear() for these
> > > pmds.
> > > 
> > > What about adding mm_inc_nr_pmds() in pmd_clear() for PxD_FLAG_ATTACHED
> > > to compensate mm_dec_nr_pmds() in free_pmd_range()?
> > 
> > pmd_clear clears one entry in the pmd, it wouldn't work. You need to add 
> > it to pgd_clear. That clears the pointer to the pmd (and does nothing if 
> > it is asked to clear the pointer to the preallocated pmd). But pgd_clear 
> > doesn't receive the pointer to mm.
> 
> I meant pmd_free(), not pmd_clear(). This should work fine.

OK, here is the updated patch.


From: Mikulas Patocka <mpatocka@redhat.com>

The patch dc6c9a35b66b520cf67e05d8ca60ebecad3b0479 that counts pmds
allocated for a process introduced a bug on 64-bit PA-RISC kernels.

The PA-RISC architecture preallocates one pmd with each pgd. This
preallocated pmd can never be freed - pmd_free does nothing when it is
called with this pmd. When the kernel attempts to free this preallocated
pmd, it decreases the count of allocated pmds. The result is that the
counter underflows and this error is reported.

This patch fixes the bug by artifically incrementing the counter in
pmd_free when the kernel tries to free the preallocated pmd.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 arch/parisc/include/asm/pgalloc.h |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Index: linux-4.0-rc4/arch/parisc/include/asm/pgalloc.h
===================================================================
--- linux-4.0-rc4.orig/arch/parisc/include/asm/pgalloc.h	2015-03-18 18:02:16.000000000 +0100
+++ linux-4.0-rc4/arch/parisc/include/asm/pgalloc.h	2015-03-18 18:03:26.000000000 +0100
@@ -74,8 +74,13 @@ static inline void pmd_free(struct mm_st
 {
 #ifdef CONFIG_64BIT
 	if(pmd_flag(*pmd) & PxD_FLAG_ATTACHED)
-		/* This is the permanent pmd attached to the pgd;
-		 * cannot free it */
+		/*
+		 * This is the permanent pmd attached to the pgd;
+		 * cannot free it.
+		 * Increment the counter to compensate for the decrement
+		 * done by generic mm code.
+		 */
+		mm_inc_nr_pmds(mm);
 		return;
 #endif
 	free_pages((unsigned long)pmd, PMD_ORDER);

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v2] mm: don't count preallocated pmds
  2015-03-18 17:42       ` [PATCH v2] " Mikulas Patocka
@ 2015-03-18 18:54         ` Kirill A. Shutemov
  0 siblings, 0 replies; 6+ messages in thread
From: Kirill A. Shutemov @ 2015-03-18 18:54 UTC (permalink / raw)
  To: Mikulas Patocka
  Cc: Kirill A. Shutemov, linux-mm, linux-parisc, jejb, dave.anglin

On Wed, Mar 18, 2015 at 01:42:38PM -0400, Mikulas Patocka wrote:
> 
> 
> On Wed, 18 Mar 2015, Kirill A. Shutemov wrote:
> 
> > On Wed, Mar 18, 2015 at 12:25:11PM -0400, Mikulas Patocka wrote:
> > > 
> > > 
> > > On Wed, 18 Mar 2015, Kirill A. Shutemov wrote:
> > > 
> > > > On Wed, Mar 18, 2015 at 11:16:42AM -0400, Mikulas Patocka wrote:
> > > > > Hi
> > > > > 
> > > > > Here I'm sending a patch that fixes numerous "BUG: non-zero nr_pmds on 
> > > > > freeing mm: -1" errors on 64-bit PA-RISC kernel.
> > > > > 
> > > > > I think the patch posted here 
> > > > > http://www.spinics.net/lists/linux-parisc/msg05981.html is incorrect, it 
> > > > > wouldn't work if the affected address range is freed and allocated 
> > > > > multiple times.
> > > > > 	- 1. alloc pgd with built-in pmd, the count of pmds is 1
> > > > > 	- 2. free the range covered by the built-in pmd, the count of pmds 
> > > > > 		is 0, but the built-in pmd is still present
> > > > 
> > > > Hm. Okay. I didn't realize you have special case in pmd_clear() for these
> > > > pmds.
> > > > 
> > > > What about adding mm_inc_nr_pmds() in pmd_clear() for PxD_FLAG_ATTACHED
> > > > to compensate mm_dec_nr_pmds() in free_pmd_range()?
> > > 
> > > pmd_clear clears one entry in the pmd, it wouldn't work. You need to add 
> > > it to pgd_clear. That clears the pointer to the pmd (and does nothing if 
> > > it is asked to clear the pointer to the preallocated pmd). But pgd_clear 
> > > doesn't receive the pointer to mm.
> > 
> > I meant pmd_free(), not pmd_clear(). This should work fine.
> 
> OK, here is the updated patch.
> 
> 
> From: Mikulas Patocka <mpatocka@redhat.com>
> 
> The patch dc6c9a35b66b520cf67e05d8ca60ebecad3b0479 that counts pmds
> allocated for a process introduced a bug on 64-bit PA-RISC kernels.
> 
> The PA-RISC architecture preallocates one pmd with each pgd. This
> preallocated pmd can never be freed - pmd_free does nothing when it is
> called with this pmd. When the kernel attempts to free this preallocated
> pmd, it decreases the count of allocated pmds. The result is that the
> counter underflows and this error is reported.
> 
> This patch fixes the bug by artifically incrementing the counter in
> pmd_free when the kernel tries to free the preallocated pmd.
> 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

> ---
>  arch/parisc/include/asm/pgalloc.h |    9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> Index: linux-4.0-rc4/arch/parisc/include/asm/pgalloc.h
> ===================================================================
> --- linux-4.0-rc4.orig/arch/parisc/include/asm/pgalloc.h	2015-03-18 18:02:16.000000000 +0100
> +++ linux-4.0-rc4/arch/parisc/include/asm/pgalloc.h	2015-03-18 18:03:26.000000000 +0100
> @@ -74,8 +74,13 @@ static inline void pmd_free(struct mm_st
>  {
>  #ifdef CONFIG_64BIT
>  	if(pmd_flag(*pmd) & PxD_FLAG_ATTACHED)
> -		/* This is the permanent pmd attached to the pgd;
> -		 * cannot free it */
> +		/*
> +		 * This is the permanent pmd attached to the pgd;
> +		 * cannot free it.
> +		 * Increment the counter to compensate for the decrement
> +		 * done by generic mm code.
> +		 */
> +		mm_inc_nr_pmds(mm);
>  		return;
>  #endif
>  	free_pages((unsigned long)pmd, PMD_ORDER);
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

-- 
 Kirill A. Shutemov

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-03-18 18:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-18 15:16 [PATCH] mm: don't count preallocated pmds Mikulas Patocka
2015-03-18 16:12 ` Kirill A. Shutemov
2015-03-18 16:25   ` Mikulas Patocka
2015-03-18 16:53     ` Kirill A. Shutemov
2015-03-18 17:42       ` [PATCH v2] " Mikulas Patocka
2015-03-18 18:54         ` Kirill A. Shutemov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).