Re: [PATCH] populated_map: fix !NUMA case, remove comment

[Nishanth Aravamudan <nacc@us.ibm.com>]

On 12.06.2007 [11:06:22 -0400], Lee Schermerhorn wrote:
> On Mon, 2007-06-11 at 20:20 -0700, Nishanth Aravamudan wrote:
> > On 11.06.2007 [19:54:13 -0700], Christoph Lameter wrote:
> > > On Mon, 11 Jun 2007, Nishanth Aravamudan wrote:
> > > 
> > > > On 11.06.2007 [19:20:58 -0700], Christoph Lameter wrote:
> > > > > On Mon, 11 Jun 2007, Nishanth Aravamudan wrote:
> > > > > 
> > > > > > [PATCH v6][RFC] Fix hugetlb pool allocation with empty nodes
> > > > > 
> > > > > There is no point in compiling the interleave logic for !NUMA.
> > > > > There needs to be some sort of !NUMA fallback in hugetlb. It would
> > > > > be better to call a interleave function in mempolicy.c that
> > > > > provides an appropriate shim for !NUMA.
> > > > 
> > > > Hrm, if !NUMA, is the nid of the only node guaranteed to be 0? If so, I
> > > > can just
> > > 
> > > Yes.
> > > 
> > > > Make alloc_fresh_huge_page() and other generic variants call into
> > > > the _node() versions with nid=0, if !NUMA.
> > > > 
> > > > Would that be ok?
> > > 
> > > I am not sure what you are up to. Just make sure that the changes are
> > > minimal. Look in the source code for other examples on how !NUMA
> > > situations were handled.
> > 
> > I swear I'm trying to make the code do the right thing, and understand
> > the NUMA intricacies better. Sorry for the flood of e-mails and such. I
> > asked about specific other cases because they are used in !NUMA
> > situations too and I wasn't sure why node_populated_map should be
> > different.
> > 
> > But ok, I will rely on the source to be correct and make my changelog
> > indicate where I got the ideas from.
> 
> Nish:  when this all settles down, I still need to make sure it works
> on our platforms with the funny DMA-only node.  What that comes down
> to is that when alloc_fresh_huge_page() calls:

Ok, thanks for these details.

Would you be ok with stabilizing the generic definition of
node_populated_map as is (any present pages, regardless of location),
and then trying to figure out how to get your platform to work with
that?

> 		page = alloc_pages_node(nid,
>                                GFP_HIGHUSER|__GFP_COMP|GFP_THISNODE,
>                                HUGETLB_PAGE_ORDER);
> 
> I need to get a page that is on nid.  On our platform, GFP_HIGHUSER is
> going to specify the zonelist for ZONE_NORMAL.  The first zone on this
> list needs to be on-node for nid.  With the changes you've made to the
> definition of populated map, I think this won't be the case.  I need
> to test your latest patches and fix that, if it's broken.

Ok. But that means your platform is broken now too, right? As in, it's
not a regression, per se?

I'm much more concerned in the short term about the whole
memoryless-node issue, which I think is more straight-forward, and
generic to fix.

> I still think using policy zone is the "right way" to go, here.  After
> all, only pages in the policy zone are controlled by policy, and
> that's the goal of spreading out the huge pages across nodes--to make
> them available to satisfy memory policy at allocation time.  But that
> would need some adjustments for x86_64 systems that have some nodes
> that are all/mostly DMA32 and other nodes that are populated in zones
> > DMA32, if we want to allocate huge pages out of the DMA32 zone.   

Well, as of right now, I'm *only* trying to deal with memoryless nodes.
So then this whole notion of policy_zone is relatively moot. It matters
for your platform, I understand, but I think the fix there is more
complex and probably should be stacked on the current set, once it is
stabilized.

> As far as the static variable, and round-robin allocation:  the current
> method "works" both for huge pages allocated at boot time and for huge
> pages allocated at run-time vi the vm.nr_hugepages sysctl.  By "works",
> I mean that it continues to spread the pages evenly across the
> "populated" nodes.  If, however, you use the task local counter to
> interleave fresh huge pages, each write to the nr_hugepages from a
> different task ["echo NN >.../nr_hugepages"] will start at node zero or
> the first populated node--assuming you're interleaving across populated
> nodes and not on-line nodes.  That's probably OK if you always change
> nr_hugepages by a multiple of the number of populated nodes.  And, if
> things get out of balance, we'll have your per node attribute, I hope,
> to adjust any individual node.

Yes, I will reply about the il_next thing in a sec. Maybe Christoph has
some cleverness.

And yes, I think the per-node attribute will fix most of the interface
problems for 'odd' NUMA systems.

Thanks,
Nish

-- 
Nishanth Aravamudan <nacc@us.ibm.com>
IBM Linux Technology Center

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