On 18.02.2014 [17:43:38 -0800], David Rientjes wrote:
> On Tue, 18 Feb 2014, Nishanth Aravamudan wrote:
>
> > How about the following?
> >
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 5de4337..1a0eced 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -1854,7 +1854,8 @@ static void __paginginit init_zone_allows_reclaim(int
> > nid)
> > int i;
> >
> > for_each_online_node(i)
> > - if (node_distance(nid, i) <= RECLAIM_DISTANCE)
> > + if (node_distance(nid, i) <= RECLAIM_DISTANCE ||
> > + !NODE_DATA(i)->node_present_pages)
> > node_set(i, NODE_DATA(nid)->reclaim_nodes);
> > else
> > zone_reclaim_mode = 1;
>
> [ I changed the above from NODE_DATA(nid) -> NODE_DATA(i) as you caught
> so we're looking at the right code. ]
>
> That can't be right, it would allow reclaiming from a memoryless node. I
> think what you want is
Gah, you're right.
> for_each_online_node(i) {
> if (!node_present_pages(i))
> continue;
> if (node_distance(nid, i) <= RECLAIM_DISTANCE) {
> node_set(i, NODE_DATA(nid)->reclaim_nodes);
> continue;
> }
> /* Always try to reclaim locally */
> zone_reclaim_mode = 1;
> }
>
> but we really should be able to do for_each_node_state(i, N_MEMORY) here
> and memoryless nodes should already be excluded from that mask.
Yep, I found that afterwards, which simplifies the logic. I'll add this
to my series :)
<snip>
> > I think it's safe to move init_zone_allows_reclaim, because I don't
> > think any allocates are occurring here that could cause us to reclaim
> > anyways, right? Moving it allows us to safely reference
> > node_present_pages.
> >
>
> Yeah, this is fine.
Thanks,
Nish
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
