All of lore.kernel.org
 help / color / mirror / Atom feed
From: toshi.kani@hpe.com (Kani, Toshi)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/3] x86/mm: add TLB purge to free pmd/pte page interfaces
Date: Tue, 15 May 2018 16:34:24 +0000	[thread overview]
Message-ID: <1526401993.2693.605.camel@hpe.com> (raw)
In-Reply-To: <20180515140549.GE18595@8bytes.org>

On Tue, 2018-05-15 at 16:05 +0200, Joerg Roedel wrote:
> On Mon, Apr 30, 2018 at 11:59:24AM -0600, Toshi Kani wrote:
> >  int pud_free_pmd_page(pud_t *pud, unsigned long addr)
> >  {
> > -	pmd_t *pmd;
> > +	pmd_t *pmd, *pmd_sv;
> > +	pte_t *pte;
> >  	int i;
> >  
> >  	if (pud_none(*pud))
> >  		return 1;
> >  
> >  	pmd = (pmd_t *)pud_page_vaddr(*pud);
> > +	pmd_sv = (pmd_t *)__get_free_page(GFP_KERNEL);
> 
> So you need to allocate a page to free a page? It is better to put the
> pages into a list with a list_head on the stack.

The code should have checked if pmd_sv is NULL...  I will update the
patch.

For performance, I do not think this page alloc is a problem.  Unlike
pmd_free_pte_page(), pud_free_pmd_page() covers an extremely rare case. 
  Since pud requires 1GB-alignment, pud and pmd/pte mappings do not
share the same ranges within the vmalloc space.  I had to instrument the
kernel to force them share the same ranges in order to test this patch.

> I am still on favour of just reverting the broken commit and do a
> correct and working fix for the/a merge window.

I will reorder the patch series, and change patch 3/3 to 1/3 so that we
can take it first to fix the BUG_ON on PAE.  This revert will disable
2MB ioremap on PAE in some cases, but I do not think it's important on
PAE anyway.

I do not think revert on x86/64 is necessary and I am more worried about
disabling 2MB ioremap in some cases, which can be seen as degradation. 
Patch 2/3 fixes a possible page-directory cache issue that I cannot hit
even though I put ioremap/iounmap with various sizes into a tight loop
for a day.

Thanks,
-Toshi

WARNING: multiple messages have this Message-ID (diff)
From: "Kani, Toshi" <toshi.kani@hpe.com>
To: "joro@8bytes.org" <joro@8bytes.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"tglx@linutronix.de" <tglx@linutronix.de>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>,
	"x86@kernel.org" <x86@kernel.org>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"hpa@zytor.com" <hpa@zytor.com>,
	"mingo@redhat.com" <mingo@redhat.com>,
	"Hocko, Michal" <MHocko@suse.com>,
	"cpandya@codeaurora.org" <cpandya@codeaurora.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 2/3] x86/mm: add TLB purge to free pmd/pte page interfaces
Date: Tue, 15 May 2018 16:34:24 +0000	[thread overview]
Message-ID: <1526401993.2693.605.camel@hpe.com> (raw)
In-Reply-To: <20180515140549.GE18595@8bytes.org>

On Tue, 2018-05-15 at 16:05 +0200, Joerg Roedel wrote:
> On Mon, Apr 30, 2018 at 11:59:24AM -0600, Toshi Kani wrote:
> >  int pud_free_pmd_page(pud_t *pud, unsigned long addr)
> >  {
> > -	pmd_t *pmd;
> > +	pmd_t *pmd, *pmd_sv;
> > +	pte_t *pte;
> >  	int i;
> >  
> >  	if (pud_none(*pud))
> >  		return 1;
> >  
> >  	pmd = (pmd_t *)pud_page_vaddr(*pud);
> > +	pmd_sv = (pmd_t *)__get_free_page(GFP_KERNEL);
> 
> So you need to allocate a page to free a page? It is better to put the
> pages into a list with a list_head on the stack.

The code should have checked if pmd_sv is NULL...  I will update the
patch.

For performance, I do not think this page alloc is a problem.  Unlike
pmd_free_pte_page(), pud_free_pmd_page() covers an extremely rare case. 
  Since pud requires 1GB-alignment, pud and pmd/pte mappings do not
share the same ranges within the vmalloc space.  I had to instrument the
kernel to force them share the same ranges in order to test this patch.

> I am still on favour of just reverting the broken commit and do a
> correct and working fix for the/a merge window.

I will reorder the patch series, and change patch 3/3 to 1/3 so that we
can take it first to fix the BUG_ON on PAE.  This revert will disable
2MB ioremap on PAE in some cases, but I do not think it's important on
PAE anyway.

I do not think revert on x86/64 is necessary and I am more worried about
disabling 2MB ioremap in some cases, which can be seen as degradation. 
Patch 2/3 fixes a possible page-directory cache issue that I cannot hit
even though I put ioremap/iounmap with various sizes into a tight loop
for a day.

Thanks,
-Toshi

  reply	other threads:[~2018-05-15 16:34 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-30 17:59 [PATCH 0/3] fix free pmd/pte page handlings on x86 Toshi Kani
2018-04-30 17:59 ` Toshi Kani
2018-04-30 17:59 ` [PATCH 1/3] ioremap: Update pgtable free interfaces with addr Toshi Kani
2018-04-30 17:59   ` Toshi Kani
2018-04-30 17:59 ` [PATCH 2/3] x86/mm: add TLB purge to free pmd/pte page interfaces Toshi Kani
2018-04-30 17:59   ` Toshi Kani
2018-05-15 14:05   ` Joerg Roedel
2018-05-15 14:05     ` Joerg Roedel
2018-05-15 16:34     ` Kani, Toshi [this message]
2018-05-15 16:34       ` Kani, Toshi
2018-04-30 17:59 ` [PATCH 3/3] x86/mm: disable ioremap free page handling on x86-PAE Toshi Kani
2018-04-30 17:59   ` Toshi Kani
2018-05-09 15:47   ` Kani, Toshi
2018-05-09 15:47     ` Kani, Toshi

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=1526401993.2693.605.camel@hpe.com \
    --to=toshi.kani@hpe.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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.