[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150106143008.GA20860@dhcp22.suse.cz>
Date: Tue, 6 Jan 2015 15:30:08 +0100
From: Michal Hocko <mhocko@...e.cz>
To: Vlastimil Babka <vbabka@...e.cz>
Cc: Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, Mel Gorman <mgorman@...e.de>,
Zhang Yanfei <zhangyanfei@...fujitsu.com>,
Minchan Kim <minchan@...nel.org>,
David Rientjes <rientjes@...gle.com>,
Rik van Riel <riel@...hat.com>,
"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>,
"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
Johannes Weiner <hannes@...xchg.org>,
Joonsoo Kim <iamjoonsoo.kim@....com>
Subject: Re: [PATCH V4 1/4] mm: set page->pfmemalloc in prep_new_page()
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@...e.cz>
> Cc: Mel Gorman <mgorman@...e.de>
> Cc: Zhang Yanfei <zhangyanfei@...fujitsu.com>
> Cc: Minchan Kim <minchan@...nel.org>
> Cc: David Rientjes <rientjes@...gle.com>
> Cc: Rik van Riel <riel@...hat.com>
> Cc: "Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
> Cc: "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
> Cc: Johannes Weiner <hannes@...xchg.org>
> Cc: Joonsoo Kim <iamjoonsoo.kim@....com>
> Cc: Michal Hocko <mhocko@...e.cz>
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@...e.cz>
[...]
> @@ -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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists