lkml.org 
[lkml]   [2009]   [Aug]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 6/6] Makes procs file writable to move all threads by tgid at once
    Benjamin Blum wrote:
    > On Sun, Aug 2, 2009 at 8:00 PM, Li Zefan<lizf@cn.fujitsu.com> wrote:
    >> Ben Blum wrote:
    >>> + }
    >>> + write_unlock(&css_set_lock);
    >>> +
    >>> + /*
    >>> + * We just gained a reference on oldcg by taking it from the task. As
    >> This comment is incorrect, the ref we just got has been dropped by
    >> the above put_css_set(oldcg).
    >
    > No, the idea is that even though we had a reference that we already
    > dropped, we in effect "traded" the newcg to the task for its oldcg,
    > giving it our reference on newcg and gaining its reference on oldcg. I
    > believe the cgroup_mutex guarantees that it'll still be there when we
    > do the trade - perhaps a BUG_ON(tsk->cgroups != oldcg) is wanted
    > inside the second task_lock section there? At the very least, a
    > clearer comment.
    >

    Maybe my English sucks..

    By "gained a reference", doesn't it mean get_css_set()? But this
    put_css_set() is not against the get() just called.

    And in fact the ref can be 0 before this put(), because task_exit
    can drop the last ref, but put_css_set() will check this case,
    so it's Ok.

    >>> +static int css_set_check_fetched(struct cgroup *cgrp, struct task_struct *tsk,
    >>> + struct css_set *cg,
    >>> + struct list_head *newcg_list)
    >>> +{
    >>> + struct css_set *newcg;
    >>> + struct cg_list_entry *cg_entry;
    >>> + struct cgroup_subsys_state *template[CGROUP_SUBSYS_COUNT];
    >>> + read_lock(&css_set_lock);
    >>> + newcg = find_existing_css_set(cg, cgrp, template);
    >>> + if (newcg)
    >>> + get_css_set(newcg);
    >>> + read_unlock(&css_set_lock);
    >>> + /* doesn't exist at all? */
    >>> + if (!newcg)
    >>> + return 1;
    >> I think it's more intuitive to return 1 if found and 0 if not found.
    >
    > I was sticking with the convention of nonzero return values indicating
    > failure, as is used everywhere else in this context.
    >

    Quoted from Documentation/CodingStyle:

    ...Such a value can be represented as an error-code integer
    (-Exxx = failure, 0 = success) or a "succeeded" boolean (0 = failure,
    non-zero = success).


    \
     
     \ /
      Last update: 2009-08-04 03:13    [W:0.048 / U:236.044 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site