On Fri, Oct 23, 2009 at 02:36:53AM -0700, David Rientjes wrote:

On Fri, 23 Oct 2009, Mel Gorman wrote:

quoted

quoted

Hmm, is this really supposed to be added to __alloc_pages_high_priority()?  
By the patch description I was expecting kswapd to be woken up 
preemptively whenever the preferred zone is below ALLOC_WMARK_LOW and 
we're known to have just allocated at a higher order, not just when 
current was oom killed (when we should already be freeing a _lot_ of 
memory soon) or is doing a higher order allocation during direct reclaim.

It was a somewhat arbitrary choice to have it trigger in the event high
priority allocations were happening frequently.

I don't quite understand, users of PF_MEMALLOC shouldn't be doing these 
higher order allocations and if ALLOC_NO_WATERMARKS is by way of the oom 
killer, we should be freeing a substantial amount of memory imminently 
when it exits that waking up kswapd would be irrelevant.

I agree. I think it's highly unlikely this patch will make any
difference but I wanted to eliminate it as a possibility. Patch 3 and 4
were previously one patch that were tested together.

quoted

quoted

If this is moved to the fastpath, why is this wake_all_kswapd() and not
wakeup_kswapd(preferred_zone, order)?  Do we need to kick kswapd in all 
zones even though they may be free just because preferred_zone is now 
below the watermark?

It probably makes no difference as zones are checked for their watermarks
before any real work happens. However, even if this patch makes a difference,
I don't want to see it merged.  At best, it is an extremely heavy-handed
hack which is why I asked for it to be tested in isolation. It shouldn't
be necessary at all because sort of pre-emptive waking of kswapd was never
necessary before.

Ahh, that makes a ton more sense: this particular patch is a debugging 
effort while the first two are candidates for 2.6.32 and -stable.  Gotcha.

Yep.

quoted

quoted

Wouldn't it be better to do this on page_zone(page) instead of 
preferred_zone anyway?

No. The preferred_zone is the zone we should be allocating from. If we
failed to allocate from it, it implies the watermarks are not being met
so we want to wake it.

Oops, I'm even more confused now :)  I thought the existing 
wake_all_kswapd() in the slowpath was doing that and that this patch was 
waking them prematurely because it speculates that a subsequent high 
order allocation will fail unless memory is reclaimed. 

It should be doing that. This patch should be junk but because it was tested
previously, I needed to be sure it was actually junk.

I thought we'd  
want to reclaim from the zone we just did a high order allocation from so 
that the fastpath could find the memory next time with ALLOC_WMARK_LOW.

The fastpath should be getting the pages it needs from the
preferred_zone. If it's not, we still want to get pages back in that
zone and the zone we actually ended up getting pages from.

It's probably best to ignore this patch except in the unlikely event Tobias
says it makes a difference to his testing. I'm hoping he's covered by patches
1+2 and maybe 3 and that patches 4 and 5 of this set get consigned to
the bit bucket.

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab