On Mon 05-01-15 18:17:40, Vlastimil Babka wrote: > The function prep_new_page() sets almost everything in the struct page of the > page being allocated, except page->pfmemalloc. This is not obvious and has at > least once led to a bug where page->pfmemalloc was forgotten to be set > correctly, see commit 8fb74b9fb2b1 ("mm: compaction: partially revert capture > of suitable high-order page"). > > This patch moves the pfmemalloc setting to prep_new_page(), which means it > needs to gain alloc_flags parameter. The call to prep_new_page is moved from > buffered_rmqueue() to get_page_from_freelist(), which also leads to simpler > code. An obsolete comment for buffered_rmqueue() is replaced. > > In addition to better maintainability there is a small reduction of code and > stack usage for get_page_from_freelist(), which inlines the other functions > involved. > > add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-145 (-145) > function old new delta > get_page_from_freelist 2670 2525 -145 > > Stack usage is reduced from 184 to 168 bytes. > > Signed-off-by: Vlastimil Babka <vbabka@xxxxxxx> > Cc: Mel Gorman <mgorman@xxxxxxx> > Cc: Zhang Yanfei <zhangyanfei@xxxxxxxxxxxxxx> > Cc: Minchan Kim <minchan@xxxxxxxxxx> > Cc: David Rientjes <rientjes@xxxxxxxxxx> > Cc: Rik van Riel <riel@xxxxxxxxxx> > Cc: "Aneesh Kumar K.V" <aneesh.kumar@xxxxxxxxxxxxxxxxxx> > Cc: "Kirill A. Shutemov" <kirill.shutemov@xxxxxxxxxxxxxxx> > Cc: Johannes Weiner <hannes@xxxxxxxxxxx> > Cc: Joonsoo Kim <iamjoonsoo.kim@xxxxxxx> > Cc: Michal Hocko <mhocko@xxxxxxx> get_page_from_freelist has grown too hairy. I agree that it is tiny less confusing now because we are not breaking out of the loop in the successful case. Acked-by: Michal Hocko <mhocko@xxxxxxx> [...] > @@ -2177,25 +2181,16 @@ zonelist_scan: > try_this_zone: > page = buffered_rmqueue(preferred_zone, zone, order, > gfp_mask, migratetype); > - if (page) > - break; > + if (page) { > + if (prep_new_page(page, order, gfp_mask, alloc_flags)) > + goto try_this_zone; > + return page; > + } I would probably liked `do {} while ()' more because it wouldn't use the goto, but this is up to you: diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 1bb65e6f48dd..1682d766cb8e 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -2175,10 +2175,11 @@ zonelist_scan: } try_this_zone: - page = buffered_rmqueue(preferred_zone, zone, order, + do { + page = buffered_rmqueue(preferred_zone, zone, order, gfp_mask, migratetype); - if (page) - break; + } while (page && prep_new_page(page, order, gfp_mask, + alloc_flags)); this_zone_full: if (IS_ENABLED(CONFIG_NUMA) && zlc_active) zlc_mark_zone_full(zonelist, z); [...] -- Michal Hocko 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/ . Don't email: <a href=mailto:"dont@xxxxxxxxx"> email@xxxxxxxxx </a>