linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: David Rientjes <rientjes@google.com>
To: Dave Hansen <dave@linux.vnet.ibm.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Michael J Wolf <mjwolf@us.ibm.com>,
	Andrea Arcangeli <aarcange@redhat.com>
Subject: Re: [RFC][PATCH 2/6] pagewalk: only split huge pages when necessary
Date: Thu, 3 Feb 2011 13:46:26 -0800 (PST)	[thread overview]
Message-ID: <alpine.DEB.2.00.1102031343530.1307@chino.kir.corp.google.com> (raw)
In-Reply-To: <1296768812.8299.1644.camel@nimitz>

On Thu, 3 Feb 2011, Dave Hansen wrote:

> > > diff -puN mm/pagewalk.c~pagewalk-dont-always-split-thp mm/pagewalk.c
> > > --- linux-2.6.git/mm/pagewalk.c~pagewalk-dont-always-split-thp	2011-01-27 10:57:02.309914973 -0800
> > > +++ linux-2.6.git-dave/mm/pagewalk.c	2011-01-27 10:57:02.317914965 -0800
> > > @@ -33,19 +33,35 @@ static int walk_pmd_range(pud_t *pud, un
> > >  
> > >  	pmd = pmd_offset(pud, addr);
> > >  	do {
> > > +	again:
> > 
> > checkpatch will warn about the indent.
> > 
> > >  		next = pmd_addr_end(addr, end);
> > > -		split_huge_page_pmd(walk->mm, pmd);
> > > -		if (pmd_none_or_clear_bad(pmd)) {
> > > +		if (pmd_none(*pmd)) {
> > 
> > Not sure why this has been changed from pmd_none_or_clear_bad(), that's 
> > been done even prior to THP.
> 
> The bad check will trigger on huge pmds.  We can not use it here.  We
> can, however, use pmd_none().  The bad check was moved below to where we
> actually dereference the pmd.
> 

Ah, right, thanks.

> > >  			if (walk->pte_hole)
> > >  				err = walk->pte_hole(addr, next, walk);
> > >  			if (err)
> > >  				break;
> > >  			continue;
> > >  		}
> > > +		/*
> > > +		 * This implies that each ->pmd_entry() handler
> > > +		 * needs to know about pmd_trans_huge() pmds
> > > +		 */
> > 
> > Probably needs to be documented somewhere for users of pagewalk?
> 
> Probably, but we don't currently have any central documentation for it.
> Guess we could make some, or just ensure that all the users got updated.
> Any ideas where to put it other than the mm_walk struct?
> 

I think noting it where struct mm_walk is declared would be best (just a 
"/* must handle pmd_trans_huge() */" would be sufficient) although 
eventually it might be cleaner to add a ->pmd_huge_entry().

--
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>

  reply	other threads:[~2011-02-03 21:46 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-01  0:33 [RFC][PATCH 0/6] more detailed per-process transparent hugepage statistics Dave Hansen
2011-02-01  0:33 ` [RFC][PATCH 1/6] count transparent hugepage splits Dave Hansen
2011-02-01  9:58   ` Johannes Weiner
2011-02-03 21:22   ` David Rientjes
2011-02-04 21:18     ` Andrea Arcangeli
2011-02-04 21:28       ` Dave Hansen
2011-02-01  0:33 ` [RFC][PATCH 2/6] pagewalk: only split huge pages when necessary Dave Hansen
2011-02-01 10:04   ` Johannes Weiner
2011-02-01 15:03     ` Dave Hansen
2011-02-03 21:22   ` David Rientjes
2011-02-03 21:33     ` Dave Hansen
2011-02-03 21:46       ` David Rientjes [this message]
2011-02-04 17:19         ` Dave Hansen
2011-02-04 21:10           ` Andrea Arcangeli
2011-02-01  0:34 ` [RFC][PATCH 3/6] break out smaps_pte_entry() from smaps_pte_range() Dave Hansen
2011-02-01 10:08   ` Johannes Weiner
2011-02-03 21:22   ` David Rientjes
2011-02-03 21:40     ` Dave Hansen
2011-02-01  0:34 ` [RFC][PATCH 4/6] pass pte size argument in to smaps_pte_entry() Dave Hansen
2011-02-01 10:09   ` Johannes Weiner
2011-02-03 21:22   ` David Rientjes
2011-02-01  0:34 ` [RFC][PATCH 5/6] teach smaps_pte_range() about THP pmds Dave Hansen
2011-02-01 10:11   ` Johannes Weiner
2011-02-01 15:02     ` Dave Hansen
2011-02-01 16:09       ` Andrea Arcangeli
2011-02-03 21:22   ` David Rientjes
2011-02-03 21:34     ` Dave Hansen
2011-02-01  0:34 ` [RFC][PATCH 6/6] have smaps show transparent huge pages Dave Hansen
2011-02-01 10:12   ` Johannes Weiner
2011-02-03 21:22   ` David Rientjes
2011-02-01 15:38 ` [RFC][PATCH 0/6] more detailed per-process transparent hugepage statistics Andrea Arcangeli
2011-02-01 17:15   ` Dave Hansen
2011-02-01 20:39     ` Andrea Arcangeli
2011-02-01 20:56       ` Dave Hansen
2011-02-02  0:07         ` Andrea Arcangeli
2011-02-08 17:54           ` Dave Hansen
2011-02-08 18:17             ` Andrea Arcangeli
2011-02-03 21:54 ` David Rientjes

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=alpine.DEB.2.00.1102031343530.1307@chino.kir.corp.google.com \
    --to=rientjes@google.com \
    --cc=aarcange@redhat.com \
    --cc=dave@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mjwolf@us.ibm.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox