Re: [patch 2/4] mempolicy: support optional mode flags

[Lee Schermerhorn <Lee.Schermerhorn@hp.com>]

On Mon, 2008-02-11 at 11:34 -0800, David Rientjes wrote:
> On Mon, 11 Feb 2008, Lee Schermerhorn wrote:
> 
> > These patches look good--well, interesting, anyway.  I'm "off on
> > assignment" this week, so I won't get to review in detail, merge and
> > test them until next...   
> > 
> 
> If, by "interesting", you mean that they give the most power to the user 
> in setting up their mempolicies than they have ever had before, then I 
> agree.

Hi David:  to clarify:  I added the "interesting" because I didn't want
the "look good" to be interpreted as an informed judgement.  I hadn't
time to review in detail.  I do have some comments, which I'll send in
response to the original patch messages [or any later posting thereof,
should that occur before I have time].  

> 
> > This helper functions introduced by this patch are similar in nature
> > [but go further] to one I introduced in the reference counting cleanup
> > RFC [http://marc.info/?l=linux-mm&m=119697614520515&w=4] I posted a
> > while back.  I've been holding these cleanup patches until Andrew starts
> > accepting this sort of thing again.  I have my series based atop Mel
> > Gorman's [added to cc] "two zonelist" series, as it depends on removing
> > the custom zonelist from the mempolicy.
> > 
> 
> If my helper functions are similar to yours then basing either of our 
> patchsets on top of the other should not be difficult.
> 
> > We need to sort out with Andrew, Mel, Paul, ... the order in which these
> > interdependent changes go in.  Given such an order, I'm willing to merge
> > them all up, test them, and post them [after running checkpatch, of
> > course].
> > 
> 
> Thanks for volunteering to test the changes.  I don't know how many 
> patchsets are currently outstanding that touch mempolicies.  So far we 
> have mine and the refcounting cleanup of yours that you mentioned.
> 
> I think the best way of dealing with it would be for the author of 
> whatever patchset is merged second to rebase off the current -mm just like 
> I based this entire patchset on your V3 contextualize_policy() patch from 
> a couple days ago.
> 
> > One other thing:  In the recent mempolicy patch to "silently restrict
> > nodemask], I mentioned the issue with regards to whether and when to
> > "contextualize" tmpfs/hugetlbfs policies--if/when we fold
> > mpol_check_policy() into mpol_new(), as you suggested.  Once we can
> > agree on the desired semantics, I had been thinking that an additional
> > mode flag could be added to policies obtained from the superblock, and
> > passed via mpol_shared_policy_init() [which calls mpol_new()] could be
> > used for this purpose.  Your change here seems to lay the foundation for
> > implementing that.
> > 
> 
> My patchset already supports contextualized tmpfs mempolicies with a 
> template for how to specify them (see patch 4 in this series for the 
> documentation update).  For example, mpol=interleave:1-3 is the equivalent 
> of MPOL_INTERLEAVE over nodes 1-3 while mpol=interleave=static:1-3 is the
> equivalent of MPOL_INTERLEAVE | MPOL_F_STATIC_NODES.

Hmmm, so 'static' means "don't contexutalize"--i.e., don't mask off
disallowed or memoryless nodes?  That means we'll need to skip them in
the interleave node calculation in the allocation path.  Perhaps your
patch already addresses this, but I haven't had time to look.  

Later,
Lee