lkml.org 
[lkml]   [2006]   [Oct]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[PATCH 6/6] taskstats: don't use tasklist_lock
    Remove tasklist_lock from taskstats.c. find_task_by_pid() is rcu-safe.
    ->siglock allows us to traverse subthread without tasklist.

    Q: delay accounting looks wrong to me. If sub-thread has already
    called taskstats_exit_send() but didn't call release_task(self) yet
    it will be accounted twice. The window is big. No?

    Signed-off-by: Oleg Nesterov <oleg@tv-sign.ru>

    --- STATS/kernel/taskstats.c~6_tasklist 2006-10-27 02:00:32.000000000 +0400
    +++ STATS/kernel/taskstats.c 2006-10-27 03:03:40.000000000 +0400
    @@ -174,21 +174,19 @@ static void send_cpu_listeners(struct sk
    up_write(&listeners->sem);
    }

    -static int fill_pid(pid_t pid, struct task_struct *pidtsk,
    +static int fill_pid(pid_t pid, struct task_struct *tsk,
    struct taskstats *stats)
    {
    int rc = 0;
    - struct task_struct *tsk = pidtsk;

    - if (!pidtsk) {
    - read_lock(&tasklist_lock);
    + if (!tsk) {
    + rcu_read_lock();
    tsk = find_task_by_pid(pid);
    - if (!tsk) {
    - read_unlock(&tasklist_lock);
    + if (tsk)
    + get_task_struct(tsk);
    + rcu_read_unlock();
    + if (!tsk)
    return -ESRCH;
    - }
    - get_task_struct(tsk);
    - read_unlock(&tasklist_lock);
    } else
    get_task_struct(tsk);

    @@ -214,40 +212,28 @@ static int fill_pid(pid_t pid, struct ta

    }

    -static int fill_tgid(pid_t tgid, struct task_struct *tgidtsk,
    +static int fill_tgid(pid_t tgid, struct task_struct *first,
    struct taskstats *stats)
    {
    - struct task_struct *tsk, *first;
    + struct task_struct *tsk;
    unsigned long flags;
    + int rc = -ESRCH;

    /*
    * Add additional stats from live tasks except zombie thread group
    * leaders who are already counted with the dead tasks
    */
    - first = tgidtsk;
    - if (!first) {
    - read_lock(&tasklist_lock);
    + rcu_read_lock();
    + if (!first)
    first = find_task_by_pid(tgid);
    - if (!first) {
    - read_unlock(&tasklist_lock);
    - return -ESRCH;
    - }
    - get_task_struct(first);
    - read_unlock(&tasklist_lock);
    - } else
    - get_task_struct(first);

    + if (!first || !lock_task_sighand(first, &flags))
    + goto out;

    - tsk = first;
    - read_lock(&tasklist_lock);
    - /* Start with stats from dead tasks */
    - if (first->sighand) {
    - spin_lock_irqsave(&first->sighand->siglock, flags);
    - if (first->signal->stats)
    - memcpy(stats, first->signal->stats, sizeof(*stats));
    - spin_unlock_irqrestore(&first->sighand->siglock, flags);
    - }
    + if (first->signal->stats)
    + memcpy(stats, first->signal->stats, sizeof(*stats));

    + tsk = first;
    do {
    if (tsk->exit_state == EXIT_ZOMBIE && thread_group_leader(tsk))
    continue;
    @@ -260,15 +246,18 @@ static int fill_tgid(pid_t tgid, struct
    delayacct_add_tsk(stats, tsk);

    } while_each_thread(first, tsk);
    - read_unlock(&tasklist_lock);
    - stats->version = TASKSTATS_VERSION;

    + unlock_task_sighand(first, &flags);
    + rc = 0;
    +out:
    + rcu_read_unlock();
    +
    + stats->version = TASKSTATS_VERSION;
    /*
    * Accounting subsytems can also add calls here to modify
    * fields of taskstats.
    */
    - put_task_struct(first);
    - return 0;
    + return rc;
    }


    -
    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: 2006-10-27 01:25    [W:7.406 / U:0.384 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site