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: AndiKleen <ak@suse.de>,
	linux-mm@kvack.org, Eric Whitney <eric.whitney@hp.com>,
	David Rientjes <rientjes@google.com>, Paul Jackson <pj@sgi.com>
Subject: Re: [NUMA] Fix memory policy refcounting
Date: Tue, 06 Nov 2007 13:56:17 -0500	[thread overview]
Message-ID: <1194375377.5317.42.camel@localhost> (raw)
In-Reply-To: <Pine.LNX.4.64.0710301136410.11531@schroedinger.engr.sgi.com>

On Tue, 2007-10-30 at 11:42 -0700, Christoph Lameter wrote:
> On Tue, 30 Oct 2007, Lee Schermerhorn wrote:
> 
> > As part of my shared policy cleanup and enhancement series, I "fixed"
> > numa_maps to display the sub-ranges of policies in a shm segment mapped
> > by a single vma. As part of this fix, I also modified mempolicy.c so
> > that it does not split vmas that support set_policy vm_ops, because
> > handling both split vmas and non-split vmas for a single shm segment
> > would have complicated the code more than I thought necessary.  This is
> > still at prototype stage--altho' it works against 23-rc8-mm2.
> 
> I have not looked at that yet. Maybe you could post another patch?
> 
> > Re:  'ref = 3' -- One reference for the rbtree--the shm segment and it's
> > policies continue to exist independent of any vma mappings--and one for
> > each attached vma.  Because the vma references are protected by the
> > respective task/mm_struct's  mmap_sem, we won't need to add an
> > additional reference during lookup, nor release it when finished with
> > the policy.  And, we won't need to mess with any other task's mm data
> > structures when installing/removing shmem policies.  Of course, munmap()
> > of a vma will need to decrement the ref count of all policies in a
> > shared policy tree, but this is not a "fast path".  Unfortunately, we
> > don't have a unmap file operation, so I'd have to add one, or otherwise
> > arrange to remove the unmapping vma's ref--perhaps via a vm_op so that
> > we only need to call it on vmas that support it--i.e., that support
> > shared policy.
> 
> Yup that sounds like it is going to be a good solution.
> 

Christoph:

After looking at this and attempting to implement it, I find that it
won't work.  The reason is that I can't tell from just vma references
whether an mempolicy in the shared policy rbtree is actually in use.  A
task is allowed to change the policies in the rbtree at any time--a
feature that I understand you have no use for and therefore don't like,
but which is fundamental to shared policy semantics.  If I try to
install a policy that completely covers/replaces an existing policy, I
need to be able to do this, regardless of how many vmas have the shared
region attached/mapped.  So, this doesn't protect any task that is
currently examining the policy for page allocation, get_mempolicy() or
show_numa_maps() without the extra ref.  Andi had probably figured this
out back when he implemented shared policies.

I have another approach that still involves adding a ref to shared
policies at lookup time, and dropping the ref when finished with the
policy.  I know you don't like the idea of taking references in the vma
policy lookup path.  However, the 'get() is already there [for shared
policies].  I just need to add the 'free() [which Mel G would like to
see renamed at mpol_put()].  I have a patch that does the unref only for
shared policies, along with the other cleanups necessary in this area.

I hope to post soon, but I've said that before.  I'll also rerun the pft
tests with and without this change when I can.

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>

  parent reply	other threads:[~2007-11-06 18:56 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-10-26 23:41 [NUMA] Fix memory policy refcounting Christoph Lameter
2007-10-29 15:48 ` Lee Schermerhorn
2007-10-29 20:24   ` Christoph Lameter
2007-10-29 21:34     ` Lee Schermerhorn
2007-10-29 21:43       ` Christoph Lameter
2007-10-30 16:39         ` Lee Schermerhorn
2007-10-30 18:42           ` Christoph Lameter
2007-10-30 20:18             ` Lee Schermerhorn
2007-11-06 18:56             ` Lee Schermerhorn [this message]
2007-11-06 19:15               ` Christoph Lameter
2007-11-06 19:35                 ` Lee Schermerhorn
2007-11-06 19:43                   ` Christoph Lameter
2007-11-06 20:08                     ` Lee Schermerhorn
2007-11-06 20:19                       ` 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=1194375377.5317.42.camel@localhost \
    --to=lee.schermerhorn@hp.com \
    --cc=ak@suse.de \
    --cc=clameter@sgi.com \
    --cc=eric.whitney@hp.com \
    --cc=linux-mm@kvack.org \
    --cc=pj@sgi.com \
    --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 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).