: Clean up of __alloc_pages

[PATCH]: Below is the cleaning up of __alloc_pages code. Few
things different from original version are

1: remove the initial direct reclaim logic
2: order zero pages are now first looked into pcp list upfront 3: GFP_HIGH pages are allowed to go little below low watermark sooner 4: Search for free pages unconditionally after direct reclaim

Signed-off-by Rohit Seth <[email protected]>

Perhaps splitting it into 2 would be a good idea - ie. first patch does the cleanup, second does the direct pcp list alloc.

Regarding the direct pcp list allocation - I think it is a good idea, because we're currently already accounting pcp list pages as being 'allocated' for the purposes of the reclaim watermarks.

Also, the structure is there to avoid touching cachelines whenever possible so it makes sense to use it early here. Do you have any performance numbers or allocation statistics (e.g. %pcp hits) to show?

Also, I would really think about uninlining get_page_from_freelist, and inlining buffered_rmqueue, so that the constant 'replenish' argument can be propogated into buffered_rmqueue and should allow for some nice optimisations. While not bloating the code too much because your get_page_from_freelist becomes out of line.

mm/page_alloc.c line 925

Quoted 9 lines · Rohit

mm/page_alloc.c +5 -3

-				goto zone_reclaim_retry;
-			}
+	if (order == 0) {
+		for (i = 0; (z = zones[i]) != NULL; i++) {
+			page = buffered_rmqueue(z, 0, gfp_mask, 0);
+			if (page) 
+				goto got_pg;
 		}
-

This is checking all zones for pages on the pcp before going the more expensive route?

Seems that this removes the logic intended to prefer local allocations over remote pages present in the existing alloc_pages? There is the danger that this modification will lead to the allocation of remote pages even if local pages are available. Thus reducing performance.

I would suggest to just check the first zone's pcp instead of all zones.

Quoted 4 lines

Seems that this removes the logic intended to prefer local allocations over remote pages present in the existing alloc_pages? There is the danger that this modification will lead to the allocation of remote pages even if local pages are available. Thus reducing performance.

Good catch. I will up level the cpuset check in buffered_rmqueue rather then doing it in get_page_from_freelist. That should retain the current preferences for local pages.

Quoted 2 lines

I would suggest to just check the first zone's pcp instead of all zones.

Na. This for most cases will be ZONE_DMA pcp list having nothing much most of the time. And picking any other zone randomly will be exposed to faulty behavior.

Quoted 4 lines · Rohit Seth

Quoted 4 lines

Seems that this removes the logic intended to prefer local allocations over remote pages present in the existing alloc_pages? There is the danger that this modification will lead to the allocation of remote pages even if local pages are available. Thus reducing performance.

Quoted 3 lines

Good catch. I will up level the cpuset check in buffered_rmqueue rather then doing it in get_page_from_freelist. That should retain the current preferences for local pages.

This is not only the cpuset check. If there is memory available in an earlier zone then it needs to be taken regardless of later pcp's the zonelist containing pages. Otherwise we did not take the pages nearest to the requested node.

Quoted 6 lines

Quoted 2 lines

I would suggest to just check the first zone's pcp instead of all zones.

Na. This for most cases will be ZONE_DMA pcp list having nothing much most of the time. And picking any other zone randomly will be exposed to faulty behavior.

Maybe only check the first node?

I think conceptually this ask for a new flag __GFP_NODEONLY that indicate allocations to come from current node only.

This definitely though means I will need to separate out the allocation from pcp patch (as Nick suggested earlier).

Quoted 5 lines · Rohit Seth

I think conceptually this ask for a new flag __GFP_NODEONLY that indicate allocations to come from current node only.

This definitely though means I will need to separate out the allocation from pcp patch (as Nick suggested earlier).

This reminds me - the current logic is currently a bit suboptimal on many NUMA systems. Often it would be better to be a bit more aggressive at freeing memory (maybe do a very low overhead light try to free pages) in the first node before falling back to other nodes. What right now happens is that when you have even minor memory pressure because e.g. you node is filled up with disk cache the local memory affinity doesn't work too well anymore.

-Andi

Quoted 15 lines

Quoted 5 lines · Rohit Seth

I think conceptually this ask for a new flag __GFP_NODEONLY that indicate allocations to come from current node only.

This definitely though means I will need to separate out the allocation from pcp patch (as Nick suggested earlier).

This reminds me - the current logic is currently a bit suboptimal on many NUMA systems. Often it would be better to be a bit more aggressive at freeing memory (maybe do a very low overhead light try to free pages) in the first node before falling back to other nodes. What right now happens is that when you have even minor memory pressure because e.g. you node is filled up with disk cache the local memory affinity doesn't work too well anymore.

Quoted 2 lines

-Andi

That's exactly what Martin Hick's additions to __alloc_pages() were trying to
achieve. However, we've never figured out how to make the "very low
overhead light try to free pages" thing work with low enough overhead that it
can be left on all of the time. As soon as we make this the least bit more
expensive, then this hurts those workloads (file servers being one example) who don't care about local, but who need the fastest possible allocations.

This problem is often a showstopper on larger NUMA systems, at least for HPC type applications, where the inability to guarantee local storage allocation when it is requested can make the application run significantly slower.

--Ray Bryant <[email protected]> wrote (on Tuesday, October 04, 2005

Quoted 9 lines

That's exactly what Martin Hick's additions to __alloc_pages() were
trying to achieve. However, we've never figured out how to make the
"very low overhead light try to free pages" thing work with low enough
overhead that it can be left on all of the time. As soon as we make
this the least bit more expensive, then this hurts those workloads (file servers being one example) who don't care about local, but who need the fastest possible allocations.

Quoted 4 lines

This problem is often a showstopper on larger NUMA systems, at least for HPC type applications, where the inability to guarantee local storage allocation when it is requested can make the application run significantly slower.

Can we not do some migration / more targeted pressure balancing in kswapd? Ie if we had a constant measure of per-node pressure, we could notice an imbalance, and start migrating the least recently used pages from the node under most pressure to the node under least ...

M.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>