On Tue, Oct 25, 2011 at 11:33 PM, David Rientjes <rientjes@xxxxxxxxxx> wrote: > On Tue, 25 Oct 2011, Colin Cross wrote: > >> Makes sense. What about this? Official patch to follow. >> >> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >> index fef8dc3..59cd4ff 100644 >> --- a/mm/page_alloc.c >> +++ b/mm/page_alloc.c >> @@ -1786,6 +1786,13 @@ should_alloc_retry(gfp_t gfp_mask, unsigned int order, >> return 0; >> >> /* >> + * If PM has disabled I/O, OOM is disabled and reclaim is unlikely >> + * to make any progress. To prevent a livelock, don't retry. >> + */ >> + if (!(gfp_allowed_mask & __GFP_FS)) >> + return 0; >> + >> + /* >> * In this implementation, order <= PAGE_ALLOC_COSTLY_ORDER >> * means __GFP_NOFAIL, but that may not be true in other >> * implementations. > > Eek, this is precisely what we don't want and is functionally the same as > what you initially proposed except it doesn't care about __GFP_NOFAIL. This is checking against gfp_allowed_mask, not gfp_mask. > You're trying to address a suspend issue where nothing on the system can > logically make progress because __GFP_FS seriously restricts the ability > of reclaim to do anything useful if it doesn't succeed the first time and > kswapd isn't effective. That's why I suggested a hook into > pm_restrict_gfp_mask() to set a variable and then treat it exactly as > __GFP_NORETRY in should_alloc_retry(). > > Consider if nobody is using suspend and they are allocating with GFP_NOFS. > There's potentially a lot of candidates: > > $ grep -r GFP_NOFS * | wc -l > 1016 > > and now we've just introduced a regression where the allocation would > eventually succeed because of either kswapd, a backing device that is no > longer congested, or an allocation on another cpu in a context where > direct reclaim can be more aggressive or the oom killer can at least free > some memory. > > So you definitely want to localize your change to only suspend and > pm_restrict_gfp_mask() is a very easy way to do it. So I'd suggest adding > a static bool that can be tested in should_alloc_retry() and identify such > situations and tag it as __read_mostly. > -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href