RE: Possible deadloop in direct reclaim?

[Lisa Du <cldu@marvell.com>]

>-----Original Message-----
>From: Minchan Kim [mailto:minchan@kernel.org]
>Sent: 2013年8月2日 10:26
>To: Lisa Du
>Cc: linux-mm@kvack.org; KOSAKI Motohiro
>Subject: Re: Possible deadloop in direct reclaim?
>
>Hello Lisa and KOSAKI,
>
>Lisa's quote style is very hard to follow so I'd like to write at bottom
>as ignoring line by line rule.
>
>Lisa, please correct your MUA.
I'm really sorry for my quote style, will improve it in my following mails.
>
>
>I reviewed current mmotm because recently Mel changed kswapd a lot and
>all_unreclaimable patch history today.
>What I see is recent mmotm has a same problem, too if system have no swap
>and no compaction. Of course, compaction is default yes option so we could
>recommend to enable if system works well but it's up to user and we should
>avoid direct reclaim hang although user disable compaction.
>
>When I see the patch history, real culprit is 929bea7c.
>
>"  zone->all_unreclaimable and zone->pages_scanned are neigher atomic
>    variables nor protected by lock.  Therefore zones can become a state of
>    zone->page_scanned=0 and zone->all_unreclaimable=1.  In this case, current
>    all_unreclaimable() return false even though zone->all_unreclaimabe=1."
>
>I understand the problem but apparently, it makes Lisa's problem because
>kswapd can give up balancing when high order allocation happens to prevent
>excessive reclaim with assuming the process requested high order allocation
>can do direct reclaim/compaction. But what if the process can't reclaim
>by no swap but lots of anon pages and can't compact by !CONFIG_COMPACTION?
>
>In such system, OOM kill is natural but not hang.
>So, a solution we can fix simply introduces zone_reclaimable check again in
>all_unreclaimabe() like this.
>
>What do you think about it?
>
>It's a same patch Lisa posted so we should give a credit
>to her/him(Sorry I'm not sure) if we agree thie approach.
>
>Lisa, If KOSAKI agree with this, could you resend this patch with your SOB?
>
>Thanks.
>
>diff --git a/mm/vmscan.c b/mm/vmscan.c
>index a3bf7fd..78f46d8 100644
>--- a/mm/vmscan.c
>+++ b/mm/vmscan.c
>@@ -2367,7 +2367,15 @@ static bool all_unreclaimable(struct zonelist *zonelist,
> 			continue;
> 		if (!cpuset_zone_allowed_hardwall(zone, GFP_KERNEL))
> 			continue;
>-		if (!zone->all_unreclaimable)
>+		/*
>+		 * zone->page_scanned and could be raced so we need
>+		 * dobule check by zone->all_unreclaimable. Morever, kswapd
>+		 * could skip (zone->all_unreclaimable = 1) if the zone
>+		 * is heavily fragmented but enough free pages to meet
>+		 * high watermark. In such case, kswapd never set
>+		 * all_unreclaimable to 1 so we need zone_reclaimable, too.
>+		 */
>+		if (!zone->all_unreclaimable || zone_reclaimable(zone))
> 			return false;
> 	}
   I'm afraid this patch may can't help.
   zone->all_unreclaimable = 0 will always result the false return,
   zone_reclaimable(zone) check wouldn't take effect no matter
   it's true of false right?

Also Bob found below thread, seems Kosaki also found same issue:
mm, vmscan: fix do_try_to_free_pages() livelock
https://lkml.org/lkml/2012/6/14/74

>
>
>
>--
>Kind regards,
>Minchan Kim