lkml.org 
[lkml]   [2012]   [May]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH] mm: Optimize put_mems_allowed() usage
    On Tue, 27 Mar 2012 15:14:30 +0200
    Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:

    > Subject: mm: Optimize put_mems_allowed() usage
    > From: Peter Zijlstra <a.p.zijlstra@chello.nl>
    > Date: Mon Mar 26 14:13:05 CEST 2012
    >
    > Since put_mems_allowed() is strictly optional, its a seqcount retry,
    > we don't need to evaluate the function if the allocation was in fact
    > successful, saving a smp_rmb some loads and comparisons on some
    > relative fast-paths.
    >
    > Since the naming, get/put_mems_allowed() does suggest a mandatory
    > pairing, rename the interface, as suggested by Mel, to resemble the
    > seqcount interface.
    >
    > This gives us: read_mems_allowed_begin() and
    > read_mems_allowed_retry(), where it is important to note that the
    > return value of the latter call is inverted from its previous
    > incarnation.
    >
    > ...
    >
    > --- a/mm/slub.c
    > +++ b/mm/slub.c
    > @@ -1606,7 +1606,7 @@ static struct page *get_any_partial(stru
    > return NULL;
    >
    > do {
    > - cpuset_mems_cookie = get_mems_allowed();
    > + cpuset_mems_cookie = read_mems_allowed_begin();
    > zonelist = node_zonelist(slab_node(current->mempolicy), flags);
    > for_each_zone_zonelist(zone, z, zonelist, high_zoneidx) {
    > struct kmem_cache_node *n;
    > @@ -1616,21 +1616,11 @@ static struct page *get_any_partial(stru
    > if (n && cpuset_zone_allowed_hardwall(zone, flags) &&
    > n->nr_partial > s->min_partial) {
    > object = get_partial_node(s, n, c);
    > - if (object) {
    > - /*
    > - * Return the object even if
    > - * put_mems_allowed indicated that
    > - * the cpuset mems_allowed was
    > - * updated in parallel. It's a
    > - * harmless race between the alloc
    > - * and the cpuset update.
    > - */
    > - put_mems_allowed(cpuset_mems_cookie);
    > + if (object)
    > return object;
    > - }
    > }
    > }
    > - } while (!put_mems_allowed(cpuset_mems_cookie));
    > + } while (read_mems_allowed_retry(cpuset_mems_cookie));

    I do think it was a bad idea to remove that comment. As it stands, the
    reader will be wondering why we did the read_mems_allowed_begin() at
    all, and whether failing to check for a change is a bug.

    --- a/mm/slub.c~mm-optimize-put_mems_allowed-usage-fix
    +++ a/mm/slub.c
    @@ -1624,8 +1624,16 @@ static struct page *get_any_partial(stru
    if (n && cpuset_zone_allowed_hardwall(zone, flags) &&
    n->nr_partial > s->min_partial) {
    object = get_partial_node(s, n, c);
    - if (object)
    + if (object) {
    + /*
    + * Don't check read_mems_allowed_retry()
    + * here - if mems_allowed was updated in
    + * parallel, that was a harmless race
    + * between allocation and the cpuset
    + * update
    + */
    return object;
    + }
    }
    }
    } while (read_mems_allowed_retry(cpuset_mems_cookie));
    _


    \
     
     \ /
      Last update: 2012-05-17 22:41    [W:0.035 / U:0.640 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site