On Sat, Dec 31, 2011 at 10:55:22PM +0800, Hillf Danton wrote: > From: Hillf Danton <dhillf@xxxxxxxxx> > Subject: [PATCH] mm: vmscam: check page order in isolating lru pages > > Before try to isolate physically contiguous pages, check for page order is > added, and if it is not regular page, we should give up the attempt. > > Signed-off-by: Hillf Danton <dhillf@xxxxxxxxx> > Acked-by: KOSAKI Motohiro <kosaki.motohiro@xxxxxxxxxxxxxx> > Cc: Michal Hocko <mhocko@xxxxxxx> > Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx> > Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> > Cc: David Rientjes <rientjes@xxxxxxxxxx> > Cc: Hugh Dickins <hughd@xxxxxxxxxx> > Cc: Andrea Arcangeli <aarcange@xxxxxxxxxx> > Cc: Mel Gorman <mgorman@xxxxxxx> > --- > > --- a/mm/vmscan.c Thu Dec 29 20:20:16 2011 > +++ b/mm/vmscan.c Sat Dec 31 22:44:16 2011 > @@ -1162,6 +1162,7 @@ static unsigned long isolate_lru_pages(u > unsigned long end_pfn; > unsigned long page_pfn; > int zone_id; > + unsigned int isolated_pages = 1; > > page = lru_to_page(src); > prefetchw_prev_lru_page(page, src, flags); > @@ -1172,7 +1173,7 @@ static unsigned long isolate_lru_pages(u > case 0: > mem_cgroup_lru_del(page); > list_move(&page->lru, dst); > - nr_taken += hpage_nr_pages(page); > + isolated_pages = hpage_nr_pages(page); > break; > > case -EBUSY: > @@ -1184,8 +1185,12 @@ static unsigned long isolate_lru_pages(u > BUG(); > } > > + nr_taken += isolated_pages; > if (!order) > continue; > + /* try pfn-based isolation only for regular page */ > + if (isolated_pages != 1) > + continue; > Please put more detail in your changelogs explaining the intention of your patch. Judging from it, this is a marginal performance improvement when THPs are being isolated from the LRU by bypassing lumpy reclaim. However, basing the check on "isolated_pages" is obscure and it also disables lumpy reclaim for the cases where order > HPAGE_SHIFT . This is very rare (might never even happen) but it's still broken. Minimally the check should have been something like if (!order || isolated_pages >= (1 << order)) continue; with a comment explaining that there is no point taking pages around a naturally-aligned region if we just isolated a page larger than it. This would look better, avoid reusing isolated_pages, be less obscure and still work for cases where the requested order is larger than a THP. Nak to this version. -- Mel Gorman SUSE Labs -- 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=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>