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 14:34:27 -0400	[thread overview]
Message-ID: <1146508468.5216.88.camel@localhost.localdomain> (raw)
In-Reply-To: <Pine.LNX.4.64.0605011056420.15588@schroedinger.engr.sgi.com>
On Mon, 2006-05-01 at 11:04 -0700, Christoph Lameter wrote:
> On Mon, 1 May 2006, Lee Schermerhorn wrote:
> 
> > > 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.
> 
> I send you a set of patches that split migrate_pages(). Maybe that is what 
> you are looking for?
> 
> You only need one additional refcount to hold the page. This is the same 
> as in the case of migrate_pages(). Where does the second refcount come from?
One from the cache [where I find the page on fault], one from
find_get_page().
Plus 1 from page_private(page) buf refs, if any.  Also, to match the
page
state for direct migration, I isolate the page from the lru so it can't
be found, except through the cache, while I'm migrating it; and that
adds
yet another ref.  Net is 1 extra ref in the fault path.
> 
> > > 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...
> 
> zap_pte_range() can remove a mapcount without obtaining a lock. Hmmm... 
> Seems to do nothing with the mapping though. Removal of anonymous mappings 
> are deferred until we reach free_page() so that does not apply. Check the 
> file I/O functions.
I did.  Looked like page->mapping only gets NULLed out in 
[__]remove_from_page_cache().  I backtracked all of the
refs I could find, and all seemed to hold page lock.  But, again,
I could have missed some [cscope can lie, as can my eyes].
Lee
--
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>
next prev parent reply	other threads:[~2006-05-01 18:34 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
2006-05-01 18:04         ` Christoph Lameter
2006-05-01 18:34           ` Lee Schermerhorn [this message]
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=1146508468.5216.88.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;
as well as URLs for NNTP newsgroup(s).