[PATCH 1/8] taskstats: fix indentation of long argument lists - Kernel

This is a discussion on [PATCH 1/8] taskstats: fix indentation of long argument lists - Kernel ; Align with the opening parenthesis. Changelog since V1 ( http://lkml.org/lkml/2007/9/21/527): - renamed fill_threadgroup() and add_tsk() to respectively fill_threadgroup_stats() and add_tsk_stats() as suggested by Balbir Singh. - added braces around do/while. - added patch to unbreak binary compatibility between taskstats v5/v6. ...

+ Reply to Thread
Results 1 to 9 of 9

Thread: [PATCH 1/8] taskstats: fix indentation of long argument lists

  1. [PATCH 1/8] taskstats: fix indentation of long argument lists

    Align with the opening parenthesis.

    Changelog since V1 (http://lkml.org/lkml/2007/9/21/527):
    - renamed fill_threadgroup() and add_tsk() to respectively
    fill_threadgroup_stats() and add_tsk_stats() as suggested by Balbir Singh.
    - added braces around do/while.
    - added patch to unbreak binary compatibility between taskstats v5/v6.
    - split further by preparing the bacct/xacct before the main patch.
    - some indentation fixes.

    Signed-off-by: Guillaume Chazarain
    Cc: Balbir Singh
    Cc: Jay Lan
    Cc: Jonathan Lim
    Cc: Oleg Nesterov
    ---

    kernel/taskstats.c | 9 ++++-----
    1 files changed, 4 insertions(+), 5 deletions(-)

    diff --git a/kernel/taskstats.c b/kernel/taskstats.c
    index 059431e..a42b6c0 100644
    --- a/kernel/taskstats.c
    +++ b/kernel/taskstats.c
    @@ -69,7 +69,7 @@ enum actions {
    };

    static int prepare_reply(struct genl_info *info, u8 cmd, struct sk_buff **skbp,
    - size_t size)
    + size_t size)
    {
    struct sk_buff *skb;
    void *reply;
    @@ -119,7 +119,7 @@ static int send_reply(struct sk_buff *skb, pid_t pid)
    * Send taskstats data in @skb to listeners registered for @cpu's exit data
    */
    static void send_cpu_listeners(struct sk_buff *skb,
    - struct listener_list *listeners)
    + struct listener_list *listeners)
    {
    struct genlmsghdr *genlhdr = nlmsg_data(nlmsg_hdr(skb));
    struct listener *s, *tmp;
    @@ -168,8 +168,7 @@ static void send_cpu_listeners(struct sk_buff *skb,
    up_write(&listeners->sem);
    }

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

    @@ -210,7 +209,7 @@ static int fill_pid(pid_t pid, struct task_struct *tsk,
    }

    static int fill_tgid(pid_t tgid, struct task_struct *first,
    - struct taskstats *stats)
    + struct taskstats *stats)
    {
    struct task_struct *tsk;
    unsigned long flags;
    -
    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/

  2. [PATCH 2/8] taskstats: split the basic accounting fields

    Split the basic accounting taskstats fields into the threadgroup specific ones
    and the thread specific ones. This should have no effect on the execution.

    Signed-off-by: Guillaume Chazarain
    Cc: Balbir Singh
    Cc: Jay Lan
    Cc: Jonathan Lim
    Cc: Oleg Nesterov
    ---

    kernel/tsacct.c | 53 +++++++++++++++++++++++++++++++++++++----------------
    1 files changed, 37 insertions(+), 16 deletions(-)

    diff --git a/kernel/tsacct.c b/kernel/tsacct.c
    index 4ab1b58..d32378f 100644
    --- a/kernel/tsacct.c
    +++ b/kernel/tsacct.c
    @@ -25,13 +25,11 @@
    /*
    * fill in basic accounting fields
    */
    -void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
    +static void fill_wall_times(struct taskstats *stats, struct task_struct *tsk)
    {
    struct timespec uptime, ts;
    s64 ac_etime;

    - BUILD_BUG_ON(TS_COMM_LEN < TASK_COMM_LEN);
    -
    /* calculate task elapsed time in timespec */
    do_posix_clock_monotonic_gettime(&uptime);
    ts = timespec_sub(uptime, tsk->start_time);
    @@ -40,17 +38,47 @@ void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
    do_div(ac_etime, NSEC_PER_USEC);
    stats->ac_etime = ac_etime;
    stats->ac_btime = get_seconds() - ts.tv_sec;
    - if (thread_group_leader(tsk)) {
    - stats->ac_exitcode = tsk->exit_code;
    - if (tsk->flags & PF_FORKNOEXEC)
    - stats->ac_flag |= AFORK;
    - }
    +}
    +
    +/*
    + * Later on, XXX_add_tsk() will need to be called after XXX_fill_threadgroup(),
    + * so put the functions in this order
    + */
    +static void bacct_fill_threadgroup(struct taskstats *stats,
    + struct task_struct *tsk);
    +
    +void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
    +{
    if (tsk->flags & PF_SUPERPRIV)
    stats->ac_flag |= ASU;
    if (tsk->flags & PF_DUMPCORE)
    stats->ac_flag |= ACORE;
    if (tsk->flags & PF_SIGNALED)
    stats->ac_flag |= AXSIG;
    +
    + stats->ac_utime += cputime_to_msecs(tsk->utime) * USEC_PER_MSEC;
    + stats->ac_stime += cputime_to_msecs(tsk->stime) * USEC_PER_MSEC;
    + stats->ac_utimescaled +=
    + cputime_to_msecs(tsk->utimescaled) * USEC_PER_MSEC;
    + stats->ac_stimescaled +=
    + cputime_to_msecs(tsk->stimescaled) * USEC_PER_MSEC;
    + stats->ac_minflt += tsk->min_flt;
    + stats->ac_majflt += tsk->maj_flt;
    +
    + bacct_fill_threadgroup(stats, tsk);
    +}
    +
    +static void bacct_fill_threadgroup(struct taskstats *stats,
    + struct task_struct *tsk)
    +{
    + fill_wall_times(stats, tsk);
    +
    + if (thread_group_leader(tsk)) {
    + stats->ac_exitcode = tsk->exit_code;
    + if (tsk->flags & PF_FORKNOEXEC)
    + stats->ac_flag |= AFORK;
    + }
    +
    stats->ac_nice = task_nice(tsk);
    stats->ac_sched = tsk->policy;
    stats->ac_uid = tsk->uid;
    @@ -60,15 +88,8 @@ void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
    stats->ac_ppid = pid_alive(tsk) ?
    rcu_dereference(tsk->real_parent)->tgid : 0;
    rcu_read_unlock();
    - stats->ac_utime = cputime_to_msecs(tsk->utime) * USEC_PER_MSEC;
    - stats->ac_stime = cputime_to_msecs(tsk->stime) * USEC_PER_MSEC;
    - stats->ac_utimescaled =
    - cputime_to_msecs(tsk->utimescaled) * USEC_PER_MSEC;
    - stats->ac_stimescaled =
    - cputime_to_msecs(tsk->stimescaled) * USEC_PER_MSEC;
    - stats->ac_minflt = tsk->min_flt;
    - stats->ac_majflt = tsk->maj_flt;

    + BUILD_BUG_ON(TS_COMM_LEN < TASK_COMM_LEN);
    strncpy(stats->ac_comm, tsk->comm, sizeof(stats->ac_comm));
    }

    -
    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/

  3. [PATCH 6/8] taskstats: tell fill_threadgroup_stats() if it replies with PID or TGID stats

    fill_threadgroup_stats() may want to know if it is filling
    TASKSTATS_CMD_ATTR_TGID or TASKSTATS_CMD_ATTR_PID stats, so give it this
    information in the tg_stats boolean.

    Signed-off-by: Guillaume Chazarain
    Cc: Balbir Singh
    Cc: Jay Lan
    Cc: Jonathan Lim
    Cc: Oleg Nesterov
    ---

    include/linux/tsacct_kern.h | 4 ++--
    kernel/taskstats.c | 11 ++++++-----
    kernel/tsacct.c | 3 ++-
    3 files changed, 10 insertions(+), 8 deletions(-)

    diff --git a/include/linux/tsacct_kern.h b/include/linux/tsacct_kern.h
    index 1568a3d..e8d1ad6 100644
    --- a/include/linux/tsacct_kern.h
    +++ b/include/linux/tsacct_kern.h
    @@ -11,11 +11,11 @@

    #ifdef CONFIG_TASKSTATS
    void bacct_add_tsk(struct taskstats *stats, struct task_struct *task);
    -void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct *task);
    +void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct *task, bool tg_stats);
    #else
    static inline void bacct_add_tsk(struct taskstats *stats, struct task_struct *task)
    {}
    -static inline void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct *task)
    +static inline void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct *task, bool tg_stats)
    {}
    #endif /* CONFIG_TASKSTATS */

    diff --git a/kernel/taskstats.c b/kernel/taskstats.c
    index f25bf03..59fe1d8 100644
    --- a/kernel/taskstats.c
    +++ b/kernel/taskstats.c
    @@ -204,12 +204,13 @@ static void add_tsk_stats(struct taskstats *stats, struct task_struct *task)
    * fill_threadgroup_stats - initialize some common stats for the thread group
    * @stats: the taskstats to write into
    * @task: the thread representing the whole group
    + * @tg_stats: whether in the end thread group stats are requested
    *
    * Will be called on the thread group leader if requesting stats for the whole
    * thread group.
    */
    static void fill_threadgroup_stats(struct taskstats *stats,
    - struct task_struct *task)
    + struct task_struct *task, bool tg_stats)
    {
    /*
    * Each accounting subsystem adds calls to its functions to initialize
    @@ -221,7 +222,7 @@ static void fill_threadgroup_stats(struct taskstats *stats,
    stats->version = TASKSTATS_VERSION;

    /* fill in basic acct fields */
    - bacct_fill_threadgroup(stats, task);
    + bacct_fill_threadgroup(stats, task, tg_stats);

    /* fill in extended acct fields */
    xacct_fill_threadgroup(stats, task);
    @@ -244,7 +245,7 @@ static int fill_pid(pid_t pid, struct task_struct *tsk, struct taskstats *stats)

    memset(stats, 0, sizeof(*stats));
    add_tsk_stats(stats, tsk);
    - fill_threadgroup_stats(stats, tsk);
    + fill_threadgroup_stats(stats, tsk, false);

    /* Define err: label here if needed */
    put_task_struct(tsk);
    @@ -285,7 +286,7 @@ static int fill_tgid(pid_t tgid, struct task_struct *first,
    add_tsk_stats(stats, tsk);
    } while_each_thread(first, tsk);

    - fill_threadgroup_stats(stats, first->group_leader);
    + fill_threadgroup_stats(stats, first->group_leader, true);
    unlock_task_sighand(first, &flags);
    rc = 0;
    out:
    @@ -541,7 +542,7 @@ void taskstats_exit(struct task_struct *tsk, int group_dead)
    */

    memcpy(stats, tsk->signal->stats, sizeof(*stats));
    - fill_threadgroup_stats(stats, tsk->group_leader);
    + fill_threadgroup_stats(stats, tsk->group_leader, true);

    send:
    send_cpu_listeners(rep_skb, listeners);
    diff --git a/kernel/tsacct.c b/kernel/tsacct.c
    index 73b4c69..8c2f470 100644
    --- a/kernel/tsacct.c
    +++ b/kernel/tsacct.c
    @@ -62,7 +62,8 @@ void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
    stats->nivcsw += tsk->nivcsw;
    }

    -void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct *tsk)
    +void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct *tsk,
    + bool tg_stats)
    {
    fill_wall_times(stats, tsk);

    -
    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/

  4. [PATCH 4/8] taskstats: separate PID/TGID stats producers to complete the TGID ones

    TASKSTATS_CMD_ATTR_TGID used to return only the delay accounting stats, not
    the basic and extended accounting. With this patch,
    TASKSTATS_CMD_ATTR_TGID also aggregates the accounting info for all threads
    of a thread group.

    TASKSTATS_CMD_ATTR_PID output should be unchanged
    TASKSTATS_CMD_ATTR_TGID output should have all fields set, unlike before the
    patch where most of the fiels were set to 0.

    To this aim, two functions were introduced: fill_threadgroup_stats() and
    add_tsk_stats(). These functions are responsible for aggregating the
    subsystem specific accounting information. Taskstats requesters (fill_pid(),
    fill_tgid() and fill_tgid_exit()) should only call add_tsk_stats() and
    fill_threadgroup_stats() to get the stats.

    Signed-off-by: Guillaume Chazarain
    Cc: Balbir Singh
    Cc: Jay Lan
    Cc: Jonathan Lim
    Cc: Oleg Nesterov
    ---

    Documentation/accounting/getdelays.c | 2 -
    include/linux/tsacct_kern.h | 12 +++-
    kernel/taskstats.c | 107 +++++++++++++++++++++++-----------
    kernel/tsacct.c | 20 +-----
    4 files changed, 85 insertions(+), 56 deletions(-)

    diff --git a/Documentation/accounting/getdelays.c b/Documentation/accounting/getdelays.c
    index cbee3a2..78773c0 100644
    --- a/Documentation/accounting/getdelays.c
    +++ b/Documentation/accounting/getdelays.c
    @@ -76,7 +76,7 @@ static void usage(void)
    fprintf(stderr, "getdelays [-dilv] [-w logfile] [-r bufsize] "
    "[-m cpumask] [-t tgid] [-p pid]\n");
    fprintf(stderr, " -d: print delayacct stats\n");
    - fprintf(stderr, " -i: print IO accounting (works only with -p)\n");
    + fprintf(stderr, " -i: print IO accounting\n");
    fprintf(stderr, " -l: listen forever\n");
    fprintf(stderr, " -v: debug on\n");
    }
    diff --git a/include/linux/tsacct_kern.h b/include/linux/tsacct_kern.h
    index 7e50ac7..1568a3d 100644
    --- a/include/linux/tsacct_kern.h
    +++ b/include/linux/tsacct_kern.h
    @@ -10,19 +10,25 @@
    #include

    #ifdef CONFIG_TASKSTATS
    -extern void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk);
    +void bacct_add_tsk(struct taskstats *stats, struct task_struct *task);
    +void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct *task);
    #else
    -static inline void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
    +static inline void bacct_add_tsk(struct taskstats *stats, struct task_struct *task)
    +{}
    +static inline void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct *task)
    {}
    #endif /* CONFIG_TASKSTATS */

    #ifdef CONFIG_TASK_XACCT
    -extern void xacct_add_tsk(struct taskstats *stats, struct task_struct *p);
    +void xacct_add_tsk(struct taskstats *stats, struct task_struct *p);
    +void xacct_fill_threadgroup(struct taskstats *stats, struct task_struct *task);
    extern void acct_update_integrals(struct task_struct *tsk);
    extern void acct_clear_integrals(struct task_struct *tsk);
    #else
    static inline void xacct_add_tsk(struct taskstats *stats, struct task_struct *p)
    {}
    +static inline void xacct_fill_threadgroup(struct taskstats *stats, struct task_struct *task)
    +{}
    static inline void acct_update_integrals(struct task_struct *tsk)
    {}
    static inline void acct_clear_integrals(struct task_struct *tsk)
    diff --git a/kernel/taskstats.c b/kernel/taskstats.c
    index a42b6c0..972bbfa 100644
    --- a/kernel/taskstats.c
    +++ b/kernel/taskstats.c
    @@ -168,6 +168,63 @@ static void send_cpu_listeners(struct sk_buff *skb,
    up_write(&listeners->sem);
    }

    +/**
    + * add_tsk_stats - combine some thread specific stats in a taskstats
    + * @stats: the taskstats to write into
    + * @task: the thread to combine
    + *
    + * There are two types of taskstats fields when considering a thread group:
    + * - those that can be aggregated from each thread in the group (like CPU
    + * times),
    + * - those that cannot be aggregated (like UID) or are identical (like
    + * memory usage), so are taken from the group leader.
    + * XXX_add_tsk() methods deal with the first type while XXX_fill_threadgroup()
    + * with the second one.
    + */
    +static void add_tsk_stats(struct taskstats *stats, struct task_struct *task)
    +{
    + /*
    + * Each accounting subsystem adds calls to its functions to combine
    + * relevant parts of struct taskstsats for a single pid as follows:
    + *
    + * per-task-foo-add_tsk_stats(stats, task);
    + */
    +
    + /* fill in delay acct fields */
    + delayacct_add_tsk(stats, task);
    +
    + /* fill in basic acct fields */
    + bacct_add_tsk(stats, task);
    +
    + /* fill in extended acct fields */
    + xacct_add_tsk(stats, task);
    +}
    +
    +/**
    + * fill_threadgroup_stats - initialize some common stats for the thread group
    + * @stats: the taskstats to write into
    + * @task: the thread representing the whole group
    + *
    + * Will be called on the thread group leader if requesting stats for the whole
    + * thread group.
    + */
    +static void fill_threadgroup_stats(struct taskstats *stats,
    + struct task_struct *task)
    +{
    + /*
    + * Each accounting subsystem adds calls to its functions to initialize
    + * relevant parts of struct taskstsats for a single tgid as follows:
    + *
    + * per-task-foo-fill_threadgroup(stats, task);
    + */
    +
    + /* fill in basic acct fields */
    + bacct_fill_threadgroup(stats, task);
    +
    + /* fill in extended acct fields */
    + xacct_fill_threadgroup(stats, task);
    +}
    +
    static int fill_pid(pid_t pid, struct task_struct *tsk, struct taskstats *stats)
    {
    int rc = 0;
    @@ -184,23 +241,11 @@ static int fill_pid(pid_t pid, struct task_struct *tsk, struct taskstats *stats)
    get_task_struct(tsk);

    memset(stats, 0, sizeof(*stats));
    - /*
    - * Each accounting subsystem adds calls to its functions to
    - * fill in relevant parts of struct taskstsats as follows
    - *
    - * per-task-foo(stats, tsk);
    - */
    -
    - delayacct_add_tsk(stats, tsk);
    -
    - /* fill in basic acct fields */
    stats->version = TASKSTATS_VERSION;
    stats->nvcsw = tsk->nvcsw;
    stats->nivcsw = tsk->nivcsw;
    - bacct_add_tsk(stats, tsk);
    -
    - /* fill in extended acct fields */
    - xacct_add_tsk(stats, tsk);
    + add_tsk_stats(stats, tsk);
    + fill_threadgroup_stats(stats, tsk);

    /* Define err: label here if needed */
    put_task_struct(tsk);
    @@ -236,27 +281,20 @@ static int fill_tgid(pid_t tgid, struct task_struct *first,
    if (tsk->exit_state)
    continue;
    /*
    - * Accounting subsystem can call its functions here to
    - * fill in relevant parts of struct taskstsats as follows
    - *
    - * per-task-foo(stats, tsk);
    + * This check is racy as a thread could exit just right
    + * now and have its statistics accounted twice.
    */
    - delayacct_add_tsk(stats, tsk);
    -
    + add_tsk_stats(stats, tsk);
    stats->nvcsw += tsk->nvcsw;
    stats->nivcsw += tsk->nivcsw;
    } while_each_thread(first, tsk);

    + fill_threadgroup_stats(stats, first->group_leader);
    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.
    - */
    return rc;
    }

    @@ -266,19 +304,14 @@ static void fill_tgid_exit(struct task_struct *tsk)
    unsigned long flags;

    spin_lock_irqsave(&tsk->sighand->siglock, flags);
    - if (!tsk->signal->stats)
    - goto ret;

    /*
    - * Each accounting subsystem calls its functions here to
    - * accumalate its per-task stats for tsk, into the per-tgid structure
    - *
    - * per-task-foo(tsk->signal->stats, tsk);
    + * The fill_threadgroup_stats() part of the statistics will be added by
    + * the stats requester, i.e. fill_tgid() or taskstats_exit()
    */
    - delayacct_add_tsk(tsk->signal->stats, tsk);
    -ret:
    + add_tsk_stats(tsk->signal->stats, tsk);
    +
    spin_unlock_irqrestore(&tsk->sighand->siglock, flags);
    - return;
    }

    static int add_del_listener(pid_t pid, cpumask_t *maskp, int isadd)
    @@ -507,7 +540,13 @@ void taskstats_exit(struct task_struct *tsk, int group_dead)
    if (!stats)
    goto err;

    + /*
    + * Race here: the last thread may have not finished its
    + * fill_tgid_exit(), leaving an incomplete tsk->signal->stats
    + */
    +
    memcpy(stats, tsk->signal->stats, sizeof(*stats));
    + fill_threadgroup_stats(stats, tsk->group_leader);

    send:
    send_cpu_listeners(rep_skb, listeners);
    diff --git a/kernel/tsacct.c b/kernel/tsacct.c
    index e8c25d2..4caea73 100644
    --- a/kernel/tsacct.c
    +++ b/kernel/tsacct.c
    @@ -40,13 +40,6 @@ static void fill_wall_times(struct taskstats *stats, struct task_struct *tsk)
    stats->ac_btime = get_seconds() - ts.tv_sec;
    }

    -/*
    - * Later on, XXX_add_tsk() will need to be called after XXX_fill_threadgroup(),
    - * so put the functions in this order
    - */
    -static void bacct_fill_threadgroup(struct taskstats *stats,
    - struct task_struct *tsk);
    -
    void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
    {
    if (tsk->flags & PF_SUPERPRIV)
    @@ -64,12 +57,9 @@ void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
    cputime_to_msecs(tsk->stimescaled) * USEC_PER_MSEC;
    stats->ac_minflt += tsk->min_flt;
    stats->ac_majflt += tsk->maj_flt;
    -
    - bacct_fill_threadgroup(stats, tsk);
    }

    -static void bacct_fill_threadgroup(struct taskstats *stats,
    - struct task_struct *tsk)
    +void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct *tsk)
    {
    fill_wall_times(stats, tsk);

    @@ -102,9 +92,6 @@ static void bacct_fill_threadgroup(struct taskstats *stats,
    * fill in extended accounting fields
    */

    -static void xacct_fill_threadgroup(struct taskstats *stats,
    - struct task_struct *tsk);
    -
    void xacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
    {
    /* convert pages-jiffies to Mbyte-usec */
    @@ -120,12 +107,9 @@ void xacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
    stats->write_bytes += tsk->ioac.write_bytes;
    stats->cancelled_write_bytes += tsk->ioac.cancelled_write_bytes;
    #endif
    -
    - xacct_fill_threadgroup(stats, tsk);
    }

    -static void xacct_fill_threadgroup(struct taskstats *stats,
    - struct task_struct *tsk)
    +void xacct_fill_threadgroup(struct taskstats *stats, struct task_struct *tsk)
    {
    struct mm_struct *mm;

    -
    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/

  5. [PATCH 3/8] taskstats: split the extended accounting fields

    Split the extended accounting taskstats fields into the threadgroup specific
    ones and the thread specific ones. This should have no effect on the execution.

    Signed-off-by: Guillaume Chazarain
    Cc: Balbir Singh
    Cc: Jay Lan
    Cc: Jonathan Lim
    Cc: Oleg Nesterov
    ---

    kernel/tsacct.c | 48 ++++++++++++++++++++++++++++--------------------
    1 files changed, 28 insertions(+), 20 deletions(-)

    diff --git a/kernel/tsacct.c b/kernel/tsacct.c
    index d32378f..e8c25d2 100644
    --- a/kernel/tsacct.c
    +++ b/kernel/tsacct.c
    @@ -101,33 +101,41 @@ static void bacct_fill_threadgroup(struct taskstats *stats,
    /*
    * fill in extended accounting fields
    */
    -void xacct_add_tsk(struct taskstats *stats, struct task_struct *p)
    +
    +static void xacct_fill_threadgroup(struct taskstats *stats,
    + struct task_struct *tsk);
    +
    +void xacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
    +{
    + /* convert pages-jiffies to Mbyte-usec */
    + stats->coremem += jiffies_to_usecs(tsk->acct_rss_mem1) * PAGE_SIZE / MB;
    + stats->virtmem += jiffies_to_usecs(tsk->acct_vm_mem1) * PAGE_SIZE / MB;
    +
    + stats->read_char += tsk->rchar;
    + stats->write_char += tsk->wchar;
    + stats->read_syscalls += tsk->syscr;
    + stats->write_syscalls += tsk->syscw;
    +#ifdef CONFIG_TASK_IO_ACCOUNTING
    + stats->read_bytes += tsk->ioac.read_bytes;
    + stats->write_bytes += tsk->ioac.write_bytes;
    + stats->cancelled_write_bytes += tsk->ioac.cancelled_write_bytes;
    +#endif
    +
    + xacct_fill_threadgroup(stats, tsk);
    +}
    +
    +static void xacct_fill_threadgroup(struct taskstats *stats,
    + struct task_struct *tsk)
    {
    struct mm_struct *mm;

    - /* convert pages-jiffies to Mbyte-usec */
    - stats->coremem = jiffies_to_usecs(p->acct_rss_mem1) * PAGE_SIZE / MB;
    - stats->virtmem = jiffies_to_usecs(p->acct_vm_mem1) * PAGE_SIZE / MB;
    - mm = get_task_mm(p);
    + mm = get_task_mm(tsk);
    if (mm) {
    /* adjust to KB unit */
    - stats->hiwater_rss = mm->hiwater_rss * PAGE_SIZE / KB;
    - stats->hiwater_vm = mm->hiwater_vm * PAGE_SIZE / KB;
    + stats->hiwater_rss = mm->hiwater_rss * PAGE_SIZE / KB;
    + stats->hiwater_vm = mm->hiwater_vm * PAGE_SIZE / KB;
    mmput(mm);
    }
    - stats->read_char = p->rchar;
    - stats->write_char = p->wchar;
    - stats->read_syscalls = p->syscr;
    - stats->write_syscalls = p->syscw;
    -#ifdef CONFIG_TASK_IO_ACCOUNTING
    - stats->read_bytes = p->ioac.read_bytes;
    - stats->write_bytes = p->ioac.write_bytes;
    - stats->cancelled_write_bytes = p->ioac.cancelled_write_bytes;
    -#else
    - stats->read_bytes = 0;
    - stats->write_bytes = 0;
    - stats->cancelled_write_bytes = 0;
    -#endif
    }
    #undef KB
    #undef MB
    -
    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/

  6. [PATCH 7/8] taskstats: fix stats->ac_exitcode to work on threads and use group_exit_code

    Threads also have an exit code on their own, so report it in
    TASKSTATS_CMD_ATTR_PID.

    For TASKSTATS_CMD_ATTR_TGID, instead of relying only on the exit code of the
    leader, we use task->signal->group_exit_code if not null as suggested by
    Oleg Nesterov.

    Also, document that as of this patch, fill_threadgroup_stats() must be called
    after add_tsk_stats() as it may overwrite some stats.

    Signed-off-by: Guillaume Chazarain
    Cc: Balbir Singh
    Cc: Jay Lan
    Cc: Jonathan Lim
    Cc: Oleg Nesterov
    ---

    kernel/taskstats.c | 2 ++
    kernel/tsacct.c | 12 +++++++-----
    2 files changed, 9 insertions(+), 5 deletions(-)

    diff --git a/kernel/taskstats.c b/kernel/taskstats.c
    index 59fe1d8..938c0b0 100644
    --- a/kernel/taskstats.c
    +++ b/kernel/taskstats.c
    @@ -208,6 +208,8 @@ static void add_tsk_stats(struct taskstats *stats, struct task_struct *task)
    *
    * Will be called on the thread group leader if requesting stats for the whole
    * thread group.
    + * fill_threadgroup_stats() may overwrite stats from add_tsk_stats(), so it
    + * must be called after add_tsk_stats().
    */
    static void fill_threadgroup_stats(struct taskstats *stats,
    struct task_struct *task, bool tg_stats)
    diff --git a/kernel/tsacct.c b/kernel/tsacct.c
    index 8c2f470..e83f53b 100644
    --- a/kernel/tsacct.c
    +++ b/kernel/tsacct.c
    @@ -65,13 +65,15 @@ void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
    void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct *tsk,
    bool tg_stats)
    {
    + int group_exit_code;
    +
    fill_wall_times(stats, tsk);

    - if (thread_group_leader(tsk)) {
    - stats->ac_exitcode = tsk->exit_code;
    - if (tsk->flags & PF_FORKNOEXEC)
    - stats->ac_flag |= AFORK;
    - }
    + if (thread_group_leader(tsk) && ((tsk->flags & PF_FORKNOEXEC)))
    + stats->ac_flag |= AFORK;
    +
    + group_exit_code = tg_stats ? tsk->signal->group_exit_code : 0;
    + stats->ac_exitcode = group_exit_code ? : tsk->exit_code;

    stats->ac_nice = task_nice(tsk);
    stats->ac_sched = tsk->policy;
    -
    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/

  7. [PATCH 8/8] taskstats: avoid breaking binary compatibility between taskstats v6 and before

    Place fields added in v6 at the end of the struct.

    Signed-off-by: Guillaume Chazarain
    Cc: Michael Neuling
    Cc: Balbir Singh
    Cc: Jay Lan
    Cc: Jonathan Lim
    Cc: Oleg Nesterov
    ---

    include/linux/taskstats.h | 18 +++++++++++-------
    1 files changed, 11 insertions(+), 7 deletions(-)

    diff --git a/include/linux/taskstats.h b/include/linux/taskstats.h
    index 92bfd1c..dc6850c 100644
    --- a/include/linux/taskstats.h
    +++ b/include/linux/taskstats.h
    @@ -85,12 +85,9 @@ struct taskstats {
    * On some architectures, value will adjust for cpu time stolen
    * from the kernel in involuntary waits due to virtualization.
    * Value is cumulative, in nanoseconds, without a corresponding count
    - * and wraps around to zero silently on overflow. The
    - * _scaled_ version accounts for cpus which can scale the
    - * number of instructions executed each cycle.
    + * and wraps around to zero silently on overflow.
    */
    __u64 cpu_run_real_total;
    - __u64 cpu_scaled_run_real_total;

    /* cpu "virtual" running time
    * Uses time intervals seen by the kernel i.e. no adjustment
    @@ -146,9 +143,6 @@ struct taskstats {
    __u64 read_syscalls; /* read syscalls */
    __u64 write_syscalls; /* write syscalls */

    - /* time accounting for SMT machines */
    - __u64 ac_utimescaled; /* utime scaled on frequency etc */
    - __u64 ac_stimescaled; /* stime scaled on frequency etc */
    /* Extended accounting fields end */

    #define TASKSTATS_HAS_IO_ACCOUNTING
    @@ -159,6 +153,16 @@ struct taskstats {

    __u64 nvcsw; /* voluntary_ctxt_switches */
    __u64 nivcsw; /* nonvoluntary_ctxt_switches */
    +
    + /*
    + * Same as cpu_run_real_total but accounts for cpus which can scale the
    + * number of instructions executed each cycle.
    + */
    + __u64 cpu_scaled_run_real_total;
    +
    + /* time accounting for SMT machines */
    + __u64 ac_utimescaled; /* utime scaled on frequency etc */
    + __u64 ac_stimescaled; /* stime scaled on frequency etc */
    };


    -
    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/

  8. Re: [PATCH 7/8] taskstats: fix stats->ac_exitcode to work on threads and use group_exit_code

    Guillaume Chazarain wrote:

    [...]

    > @@ -65,13 +65,15 @@ void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
    > void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct *tsk,
    > bool tg_stats)
    > {
    > + int group_exit_code;
    > +
    > fill_wall_times(stats, tsk);
    >
    > - if (thread_group_leader(tsk)) {
    > - stats->ac_exitcode = tsk->exit_code;
    > - if (tsk->flags & PF_FORKNOEXEC)
    > - stats->ac_flag |= AFORK;
    > - }
    > + if (thread_group_leader(tsk) && ((tsk->flags & PF_FORKNOEXEC)))


    if (thread_group_leader(tsk) && (tsk->flags & PF_FORKNOEXEC))

    > + stats->ac_flag |= AFORK;
    > +
    > + group_exit_code = tg_stats ? tsk->signal->group_exit_code : 0;
    > + stats->ac_exitcode = group_exit_code ? : tsk->exit_code;


    Isn't this just confusing? why not

    if (tg_stats) {
    group_exit_code = tsk->signal->group_exit_code;
    stats->ac_exitcode = group_exit_code;

    } else {
    group_exit_code = 0;
    stats->ac_exitcode = tsk->exit_code;
    }
    -
    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/

  9. Re: [PATCH 7/8] taskstats: fix stats->ac_exitcode to work on threads and use group_exit_code

    Le Wed, 26 Sep 2007 22:47:54 +0200,
    roel <12o3l@tiscali.nl> a écrit :

    > > + if (thread_group_leader(tsk) && ((tsk->flags & PF_FORKNOEXEC)))

    >
    > if (thread_group_leader(tsk) && (tsk->flags & PF_FORKNOEXEC))


    Yeah, right, good catch.

    > > + group_exit_code = tg_stats ? tsk->signal->group_exit_code : 0;
    > > + stats->ac_exitcode = group_exit_code ? : tsk->exit_code;

    >
    > Isn't this just confusing? why not
    >
    > if (tg_stats) {
    > group_exit_code = tsk->signal->group_exit_code;
    > stats->ac_exitcode = group_exit_code;


    Because in this case if group_exit_code is null, we want
    tsk->exit_code, not 0.

    >
    > } else {
    > group_exit_code = 0;
    > stats->ac_exitcode = tsk->exit_code;
    > }


    Andrew is not interested at the moment in this series (that replaces
    all my previous patches on taskstats, for info), but thank you for the
    review.


    --
    Guillaume
    -
    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/

+ Reply to Thread