lkml.org 
[lkml]   [2011]   [Feb]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 2/4] cpuset: Remove unneeded NODEMASK_ALLOC() in cpuset_attch()
    On Wed, Feb 16, 2011 at 5:49 PM, Li Zefan <lizf@cn.fujitsu.com> wrote:
    > oldcs->mems_allowed is not modified during cpuset_attch(), so
    > we don't have to copy it to a buffer allocated by NODEMASK_ALLOC().
    > Just pass it to cpuset_migrate_mm().
    >
    > Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>

    I'd be inclined to skip this one - we're already allocating one
    nodemask, so one more isn't really any extra complexity, and we're
    doing horrendously complicated stuff in cpuset_migrate_mm() that's
    much more likely to fail in low-memory situations.

    It's true that mems_allowed can't change during the call to
    cpuset_attach(), but that's due to the fact that both cgroup_attach()
    and the cpuset.mems write paths take cgroup_mutex. I might prefer to
    leave the allocated nodemask here and wrap callback_mutex around the
    places in cpuset_attach() where we're reading from a cpuset's
    mems_allowed - that would remove the implicit synchronization via
    cgroup_mutex and leave the code a little more understandable.

    > ---
    >  kernel/cpuset.c |    7 ++-----
    >  1 files changed, 2 insertions(+), 5 deletions(-)
    >
    > diff --git a/kernel/cpuset.c b/kernel/cpuset.c
    > index f13ff2e..70c9ca2 100644
    > --- a/kernel/cpuset.c
    > +++ b/kernel/cpuset.c
    > @@ -1438,10 +1438,9 @@ static void cpuset_attach(struct cgroup_subsys *ss, struct cgroup *cont,
    >        struct mm_struct *mm;
    >        struct cpuset *cs = cgroup_cs(cont);
    >        struct cpuset *oldcs = cgroup_cs(oldcont);
    > -       NODEMASK_ALLOC(nodemask_t, from, GFP_KERNEL);
    >        NODEMASK_ALLOC(nodemask_t, to, GFP_KERNEL);
    >
    > -       if (from == NULL || to == NULL)
    > +       if (to == NULL)
    >                goto alloc_fail;
    >
    >        if (cs == &top_cpuset) {
    > @@ -1463,18 +1462,16 @@ static void cpuset_attach(struct cgroup_subsys *ss, struct cgroup *cont,
    >        }
    >
    >        /* change mm; only needs to be done once even if threadgroup */
    > -       *from = oldcs->mems_allowed;
    >        *to = cs->mems_allowed;
    >        mm = get_task_mm(tsk);
    >        if (mm) {
    >                mpol_rebind_mm(mm, to);
    >                if (is_memory_migrate(cs))
    > -                       cpuset_migrate_mm(mm, from, to);
    > +                       cpuset_migrate_mm(mm, &oldcs->mems_allowed, to);
    >                mmput(mm);
    >        }
    >
    >  alloc_fail:
    > -       NODEMASK_FREE(from);
    >        NODEMASK_FREE(to);
    >  }
    >
    > --
    > 1.7.3.1
    >
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2011-02-18 00:49    [W:0.040 / U:0.268 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site