All of lore.kernel.org
 help / color / mirror / Atom feed
From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
To: David Rientjes <rientjes@google.com>
Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
	linux-mm@kvack.org, Andi Kleen <andi@firstfloor.org>,
	Wu Fengguang <fengguang.wu@intel.com>,
	Andrea Arcangeli <aarcange@redhat.com>,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC][PATCH 1/3] pagemap: avoid splitting thp when reading /proc/pid/pagemap
Date: Mon, 19 Dec 2011 14:16:17 -0500	[thread overview]
Message-ID: <4EEF8D81.4080301@ah.jp.nec.com> (raw)
In-Reply-To: <alpine.DEB.2.00.1112191044300.19949@chino.kir.corp.google.com>

On Mon, Dec 19, 2011 at 10:48:17AM -0800, David Rientjes wrote:
> On Mon, 19 Dec 2011, Naoya Horiguchi wrote:
...
> > @@ -666,10 +685,33 @@ static int pagemap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
> >  	pte_t *pte;
> >  	int err = 0;
> >  
> > -	split_huge_page_pmd(walk->mm, pmd);
> > -
> >  	/* find the first VMA at or above 'addr' */
> >  	vma = find_vma(walk->mm, addr);
> > +
> > +	spin_lock(&walk->mm->page_table_lock);
> 
> pagemap_pte_range() could potentially be called a _lot_, so I'd recommend 
> optimizing this by checking for pmd_trans_huge() prior to taking 
> page_table_lock and then rechecking after grabbing it with likely().

OK, I'll try it.
Similar logic is used on smaps_pte_range() and gather_pte_stats(),
so I think it's better to write a separate patch for this optimization.

> > +	if (pmd_trans_huge(*pmd)) {
> > +		if (pmd_trans_splitting(*pmd)) {
> > +			spin_unlock(&walk->mm->page_table_lock);
> > +			wait_split_huge_page(vma->anon_vma, pmd);
> > +		} else {
> > +			u64 pfn = PM_NOT_PRESENT;
> 
> This doesn't need to be initialized and it would probably be better to 
> declare "pfn" at the top-level of this function since it's later used for 
> the non-thp case.

Agreed.
I unify two declarations of "pfn" at the beginning of the function.

Thank you for nice feedback.
Naoya

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
To: David Rientjes <rientjes@google.com>
Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
	linux-mm@kvack.org, Andi Kleen <andi@firstfloor.org>,
	Wu Fengguang <fengguang.wu@intel.com>,
	Andrea Arcangeli <aarcange@redhat.com>,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC][PATCH 1/3] pagemap: avoid splitting thp when reading /proc/pid/pagemap
Date: Mon, 19 Dec 2011 14:16:17 -0500	[thread overview]
Message-ID: <4EEF8D81.4080301@ah.jp.nec.com> (raw)
In-Reply-To: <alpine.DEB.2.00.1112191044300.19949@chino.kir.corp.google.com>

On Mon, Dec 19, 2011 at 10:48:17AM -0800, David Rientjes wrote:
> On Mon, 19 Dec 2011, Naoya Horiguchi wrote:
...
> > @@ -666,10 +685,33 @@ static int pagemap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
> >  	pte_t *pte;
> >  	int err = 0;
> >  
> > -	split_huge_page_pmd(walk->mm, pmd);
> > -
> >  	/* find the first VMA at or above 'addr' */
> >  	vma = find_vma(walk->mm, addr);
> > +
> > +	spin_lock(&walk->mm->page_table_lock);
> 
> pagemap_pte_range() could potentially be called a _lot_, so I'd recommend 
> optimizing this by checking for pmd_trans_huge() prior to taking 
> page_table_lock and then rechecking after grabbing it with likely().

OK, I'll try it.
Similar logic is used on smaps_pte_range() and gather_pte_stats(),
so I think it's better to write a separate patch for this optimization.

> > +	if (pmd_trans_huge(*pmd)) {
> > +		if (pmd_trans_splitting(*pmd)) {
> > +			spin_unlock(&walk->mm->page_table_lock);
> > +			wait_split_huge_page(vma->anon_vma, pmd);
> > +		} else {
> > +			u64 pfn = PM_NOT_PRESENT;
> 
> This doesn't need to be initialized and it would probably be better to 
> declare "pfn" at the top-level of this function since it's later used for 
> the non-thp case.

Agreed.
I unify two declarations of "pfn" at the beginning of the function.

Thank you for nice feedback.
Naoya

  reply	other threads:[~2011-12-19 19:17 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-19 18:38 [RFC][PATCH 0/3] pagemap handles transparent hugepage Naoya Horiguchi
2011-12-19 18:38 ` Naoya Horiguchi
2011-12-19 18:38 ` [RFC][PATCH 1/3] pagemap: avoid splitting thp when reading /proc/pid/pagemap Naoya Horiguchi
2011-12-19 18:38   ` Naoya Horiguchi
2011-12-19 18:45   ` Andi Kleen
2011-12-19 18:45     ` Andi Kleen
2011-12-19 18:48   ` David Rientjes
2011-12-19 18:48     ` David Rientjes
2011-12-19 19:16     ` Naoya Horiguchi [this message]
2011-12-19 19:16       ` Naoya Horiguchi
2011-12-19 18:38 ` [RFC][PATCH 2/3] pagemap: export KPF_THP Naoya Horiguchi
2011-12-19 18:38   ` Naoya Horiguchi
2011-12-19 18:40   ` Andi Kleen
2011-12-19 18:40     ` Andi Kleen
2011-12-19 18:49     ` David Rientjes
2011-12-19 18:49       ` David Rientjes
2011-12-19 18:55     ` Naoya Horiguchi
2011-12-19 18:55       ` Naoya Horiguchi
2011-12-19 19:05     ` Andrea Arcangeli
2011-12-19 19:05       ` Andrea Arcangeli
2011-12-19 19:24   ` KOSAKI Motohiro
2011-12-19 19:24     ` KOSAKI Motohiro
2011-12-19 20:26     ` Naoya Horiguchi
2011-12-19 20:26       ` Naoya Horiguchi
2011-12-19 20:48       ` KOSAKI Motohiro
2011-12-19 20:48         ` KOSAKI Motohiro
2011-12-19 21:20         ` Naoya Horiguchi
2011-12-19 21:20           ` Naoya Horiguchi
2011-12-19 20:31     ` Dave Hansen
2011-12-19 20:31       ` Dave Hansen
2011-12-19 20:45       ` KOSAKI Motohiro
2011-12-19 20:45         ` KOSAKI Motohiro
2011-12-19 20:57         ` Dave Hansen
2011-12-19 20:57           ` Dave Hansen
2011-12-19 21:23           ` Andrea Arcangeli
2011-12-19 21:23             ` Andrea Arcangeli
2011-12-20  3:35   ` Wu Fengguang
2011-12-20  3:35     ` Wu Fengguang
2011-12-20 18:10     ` Naoya Horiguchi
2011-12-20 18:10       ` Naoya Horiguchi
2011-12-19 18:38 ` [RFC][PATCH 3/3] pagemap: document KPF_THP and show make page-types aware of it Naoya Horiguchi
2011-12-19 18:38   ` Naoya Horiguchi
2011-12-19 18:51   ` David Rientjes
2011-12-19 18:51     ` David Rientjes
2011-12-20  3:41   ` Wu Fengguang
2011-12-20  3:41     ` Wu Fengguang

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=4EEF8D81.4080301@ah.jp.nec.com \
    --to=n-horiguchi@ah.jp.nec.com \
    --cc=aarcange@redhat.com \
    --cc=andi@firstfloor.org \
    --cc=fengguang.wu@intel.com \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=rientjes@google.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.