lkml.org 
[lkml]   [2015]   [Jan]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [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@suse.cz>
    > Cc: Mel Gorman <mgorman@suse.de>
    > Cc: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
    > Cc: Minchan Kim <minchan@kernel.org>
    > Cc: David Rientjes <rientjes@google.com>
    > Cc: Rik van Riel <riel@redhat.com>
    > Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
    > Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
    > Cc: Johannes Weiner <hannes@cmpxchg.org>
    > Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
    > Cc: Michal Hocko <mhocko@suse.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@suse.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


    \
     
     \ /
      Last update: 2015-01-06 15:41    [W:3.736 / U:0.108 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site