All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Kirill A. Shutemov" <kirill@shutemov.name>
To: Mikulas Patocka <mpatocka@redhat.com>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	linux-mm@kvack.org, linux-parisc@vger.kernel.org,
	jejb@parisc-linux.org, dave.anglin@bell.net
Subject: Re: [PATCH v2] mm: don't count preallocated pmds
Date: Wed, 18 Mar 2015 20:54:48 +0200	[thread overview]
Message-ID: <20150318185448.GA6493@node.dhcp.inet.fi> (raw)
In-Reply-To: <alpine.LRH.2.02.1503181328450.17058@file01.intranet.prod.int.rdu2.redhat.com>

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>

      reply	other threads:[~2015-03-18 18:54 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-18 15:16 [PATCH] mm: don't count preallocated pmds Mikulas Patocka
2015-03-18 15:16 ` 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 17:42         ` Mikulas Patocka
2015-03-18 18:54         ` Kirill A. Shutemov [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150318185448.GA6493@node.dhcp.inet.fi \
    --to=kirill@shutemov.name \
    --cc=dave.anglin@bell.net \
    --cc=jejb@parisc-linux.org \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-mm@kvack.org \
    --cc=linux-parisc@vger.kernel.org \
    --cc=mpatocka@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.