The patch titled Subject: mm: avoid livelock on !__GFP_FS allocations has been added to the -mm tree. Its filename is mm-avoid-livelock-on-__gfp_fs-allocations-v2.patch Before you just go and hit "reply", please: a) Consider who else should be cc'ed b) Prefer to cc a suitable mailing list as well c) Ideally: find the original patch on the mailing list and do a reply-to-all to that, adding suitable additional cc's *** Remember to use Documentation/SubmitChecklist when testing your code *** See http://userweb.kernel.org/~akpm/stuff/added-to-mm.txt to find out what to do about this The current -mm tree may be found at http://userweb.kernel.org/~akpm/mmotm/ ------------------------------------------------------ From: Mel Gorman <mgorman@xxxxxxx> Subject: mm: avoid livelock on !__GFP_FS allocations Colin Cross reported; Under the following conditions, __alloc_pages_slowpath can loop forever: gfp_mask & __GFP_WAIT is true gfp_mask & __GFP_FS is false reclaim and compaction make no progress order <= PAGE_ALLOC_COSTLY_ORDER These conditions happen very often during suspend and resume, when pm_restrict_gfp_mask() effectively converts all GFP_KERNEL allocations into __GFP_WAIT. The oom killer is not run because gfp_mask & __GFP_FS is false, but should_alloc_retry will always return true when order is less than PAGE_ALLOC_COSTLY_ORDER. In his fix, he avoided retrying the allocation if reclaim made no progress and __GFP_FS was not set. The problem is that this would result in GFP_NOIO allocations failing that previously succeeded which would be very unfortunate. The big difference between GFP_NOIO and suspend converting GFP_KERNEL to behave like GFP_NOIO is that normally flushers will be cleaning pages and kswapd reclaims pages allowing GFP_NOIO to succeed after a short delay. The same does not necessarily apply during suspend as the storage device may be suspended. This patch special cases the suspend case to fail the page allocation if reclaim cannot make progress and adds some documentation on how gfp_allowed_mask is currently used. Failing allocations like this may cause suspend to abort but that is better than a livelock. [mgorman@xxxxxxx: Rework fix to be suspend specific] [rientjes@xxxxxxxxxx: Move suspended device check to should_alloc_retry] Reported-by: Colin Cross <ccross@xxxxxxxxxxx> Signed-off-by: Mel Gorman <mgorman@xxxxxxx> Acked-by: David Rientjes <rientjes@xxxxxxxxxx> Cc: Minchan Kim <minchan.kim@xxxxxxxxx> Cc: Pekka Enberg <penberg@xxxxxxxxxxxxxx> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx> Cc: Andrea Arcangeli <aarcange@xxxxxxxxxx> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> --- include/linux/gfp.h | 16 ++++++++++++++++ mm/page_alloc.c | 30 ++++++++++++++++++++++-------- mm/swapfile.c | 6 +++--- 3 files changed, 41 insertions(+), 11 deletions(-) diff -puN include/linux/gfp.h~mm-avoid-livelock-on-__gfp_fs-allocations-v2 include/linux/gfp.h --- a/include/linux/gfp.h~mm-avoid-livelock-on-__gfp_fs-allocations-v2 +++ a/include/linux/gfp.h @@ -379,9 +379,25 @@ void drain_zone_pages(struct zone *zone, void drain_all_pages(void); void drain_local_pages(void *dummy); +/* + * gfp_allowed_mask is set to GFP_BOOT_MASK during early boot to restrict what + * GFP flags are used before interrupts are enabled. Once interrupts are + * enabled, it is set to __GFP_BITS_MASK while the system is running. During + * hibernation, it is used by PM to avoid I/O during memory allocation while + * devices are suspended. + */ extern gfp_t gfp_allowed_mask; extern void pm_restrict_gfp_mask(void); extern void pm_restore_gfp_mask(void); +#ifdef CONFIG_PM_SLEEP +extern bool pm_suspended_storage(void); +#else +static inline bool pm_suspended_storage(void) +{ + return false; +} +#endif /* CONFIG_PM_SLEEP */ + #endif /* __LINUX_GFP_H */ diff -puN mm/page_alloc.c~mm-avoid-livelock-on-__gfp_fs-allocations-v2 mm/page_alloc.c --- a/mm/page_alloc.c~mm-avoid-livelock-on-__gfp_fs-allocations-v2 +++ a/mm/page_alloc.c @@ -127,6 +127,13 @@ void pm_restrict_gfp_mask(void) saved_gfp_mask = gfp_allowed_mask; gfp_allowed_mask &= ~GFP_IOFS; } + +bool pm_suspended_storage(void) +{ + if ((gfp_allowed_mask & GFP_IOFS) == GFP_IOFS) + return false; + return true; +} #endif /* CONFIG_PM_SLEEP */ #ifdef CONFIG_HUGETLB_PAGE_SIZE_VARIABLE @@ -1821,12 +1828,25 @@ void warn_alloc_failed(gfp_t gfp_mask, i static inline int should_alloc_retry(gfp_t gfp_mask, unsigned int order, + unsigned long did_some_progress, unsigned long pages_reclaimed) { /* Do not loop if specifically requested */ if (gfp_mask & __GFP_NORETRY) return 0; + /* Always retry if specifically requested */ + if (gfp_mask & __GFP_NOFAIL) + return 1; + + /* + * Suspend converts GFP_KERNEL to __GFP_WAIT which can prevent reclaim + * making forward progress without invoking OOM. Suspend also disables + * storage devices so kswapd will not help. Bail if we are suspending. + */ + if (!did_some_progress && pm_suspended_storage()) + return 0; + /* * In this implementation, order <= PAGE_ALLOC_COSTLY_ORDER * means __GFP_NOFAIL, but that may not be true in other @@ -1845,13 +1865,6 @@ should_alloc_retry(gfp_t gfp_mask, unsig if (gfp_mask & __GFP_REPEAT && pages_reclaimed < (1 << order)) return 1; - /* - * Don't let big-order allocations loop unless the caller - * explicitly requests that. - */ - if (gfp_mask & __GFP_NOFAIL) - return 1; - return 0; } @@ -2251,7 +2264,8 @@ rebalance: /* Check if we should retry the allocation */ pages_reclaimed += did_some_progress; - if (should_alloc_retry(gfp_mask, order, pages_reclaimed)) { + if (should_alloc_retry(gfp_mask, order, did_some_progress, + pages_reclaimed)) { /* Wait for some write requests to complete then retry */ wait_iff_congested(preferred_zone, BLK_RW_ASYNC, HZ/50); goto rebalance; diff -puN mm/swapfile.c~mm-avoid-livelock-on-__gfp_fs-allocations-v2 mm/swapfile.c --- a/mm/swapfile.c~mm-avoid-livelock-on-__gfp_fs-allocations-v2 +++ a/mm/swapfile.c @@ -670,10 +670,10 @@ int try_to_free_swap(struct page *page) * original page might be freed under memory pressure, then * later read back in from swap, now with the wrong data. * - * Hibernation clears bits from gfp_allowed_mask to prevent - * memory reclaim from writing to disk, so check that here. + * Hibration suspends storage while it is writing the image + * to disk so check that here. */ - if (!(gfp_allowed_mask & __GFP_IO)) + if (pm_suspended_storage()) return 0; delete_from_swap_cache(page); _ Subject: Subject: mm: avoid livelock on !__GFP_FS allocations Patches currently in -mm which might be from mgorman@xxxxxxx are mm-page-writebackc-make-determine_dirtyable_memory-static-again.patch mm-do-not-stall-in-synchronous-compaction-for-thp-allocations.patch mm-do-not-stall-in-synchronous-compaction-for-thp-allocations-v3.patch mm-reduce-the-amount-of-work-done-when-updating-min_free_kbytes.patch mm-reduce-the-amount-of-work-done-when-updating-min_free_kbytes-checkpatch-fixes.patch mm-avoid-livelock-on-__gfp_fs-allocations-v2.patch -- To unsubscribe from this list: send the line "unsubscribe mm-commits" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html