linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Lee Schermerhorn <Lee.Schermerhorn@hp.com>
To: Christoph Lameter <clameter@sgi.com>
Cc: akpm@osdl.org, linux-mm@kvack.org,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	Hugh Dickins <hugh@veritas.com>
Subject: Re: [PATCH 4/7] PM cleanup: Drop nr_refs in remove_references()
Date: Mon, 01 May 2006 13:51:05 -0400	[thread overview]
Message-ID: <1146505865.5216.57.camel@localhost.localdomain> (raw)
In-Reply-To: <Pine.LNX.4.64.0605010912140.15017@schroedinger.engr.sgi.com>

On Mon, 2006-05-01 at 09:15 -0700, Christoph Lameter wrote:
> On Mon, 1 May 2006, Lee Schermerhorn wrote:
> 
> > > Remove the early check for the number of references since we are
> > > checking page_mapcount() earlier. Ultimately only the refcount
> > > matters after the tree_lock has been obtained.
> > True for direct migration.  I'll still need to know whether we're in the
> > fault path for migrate-on-fault.  I don't think I can count on using the
> > mapcount as you now already remove the mapping before calling migrate_page(),
> > even for direct migration...
> 
> Well there is currently agreement that we wont include your patch because 
> it is not clear that the patch will be beneficial.

Ouch!  That's harsh!  Guess I missed that meeting... ;-) 

Seriously, of course, the onus is on me to show benefit.  And I hope to,
once the base migration code stabilizes...

> 
> And AFAIK your patch relies on only migrating pages with mapcount = 0. In 
> that case I think you can call the migration functions directly without 
> having to unmap. I thought this would actually be better for your case.

This only occurs if I find a cached, "misplaced" page in the fault path 
with mapcount==0.  But the fault path does add another ref on lookup,
so the refcounts are all one higher in this case.

> 
> > > -	if (!page_mapping(page) || page_count(page) != nr_refs ||
> > > +	if (!page_mapping(page) ||
> >                    ^^^^^^^^^^^^^^^^^
> > As part of patch 6/7, can you change this to just 'mapping'--i.e., the
> > added address_space argument?
> 
> No. The mapping may have been removed and this check is necessary to not 
> migrate a page that is already gone.

OK  I couldn't see how a page could be removed from its mapping while
we hold it locked.  I'll look closer...

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

--
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:[~2006-05-01 17:51 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-04-29  3:22 Page Migration patchsets overview Christoph Lameter
2006-04-29  3:22 ` [PATCH 1/7] PM cleanup: Rename "ignrefs" to "migration" Christoph Lameter
2006-04-29  3:22 ` [PATCH 2/7] PM cleanup: Group functions Christoph Lameter
2006-04-29  3:23 ` [PATCH 3/7] PM cleanup: Remove useless definitions Christoph Lameter
2006-04-29  3:23 ` [PATCH 4/7] PM cleanup: Drop nr_refs in remove_references() Christoph Lameter
2006-05-01 16:09   ` Lee Schermerhorn
2006-05-01 16:15     ` Christoph Lameter
2006-05-01 17:51       ` Lee Schermerhorn [this message]
2006-05-01 18:04         ` Christoph Lameter
2006-05-01 18:34           ` Lee Schermerhorn
2006-05-01 18:53             ` Christoph Lameter
2006-04-29  3:23 ` [PATCH 5/7] PM cleanup: Extract try_to_unmap from migration functions Christoph Lameter
2006-04-29  3:23 ` [PATCH 6/7] PM cleanup: Pass "mapping" to " Christoph Lameter
2006-04-29  3:23 ` [PATCH 7/7] PM cleanup: Move fallback handling into special function Christoph Lameter
2006-04-29  3:23 ` [PATCH 1/3] Swapless PM: add R/W migration entries Christoph Lameter
2006-04-29  3:23 ` [PATCH 2/3] Swapless PM: Rip out swap based logic Christoph Lameter
2006-04-29  3:23 ` [PATCH 3/3] Swapless PM: Modify core logic Christoph Lameter
2006-04-29  3:23 ` {PATCH 1/2} More PM: do not inc/dec rss counters Christoph Lameter
2006-04-29  3:23 ` {PATCH 2/2} More PM: use migration entries for file pages Christoph Lameter

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=1146505865.5216.57.camel@localhost.localdomain \
    --to=lee.schermerhorn@hp.com \
    --cc=akpm@osdl.org \
    --cc=clameter@sgi.com \
    --cc=hugh@veritas.com \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-mm@kvack.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox