2007-09-26 17:08:31

by Guillaume Chazarain

[permalink] [raw]
Subject: [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 <[email protected]>
Cc: Balbir Singh <[email protected]>
Cc: Jay Lan <[email protected]>
Cc: Jonathan Lim <[email protected]>
Cc: Oleg Nesterov <[email protected]>
---

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;


2007-09-26 17:08:44

by Guillaume Chazarain

[permalink] [raw]
Subject: [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 <[email protected]>
Cc: Balbir Singh <[email protected]>
Cc: Jay Lan <[email protected]>
Cc: Jonathan Lim <[email protected]>
Cc: Oleg Nesterov <[email protected]>
---

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));
}

2007-09-26 17:09:04

by Guillaume Chazarain

[permalink] [raw]
Subject: [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 <[email protected]>
Cc: Balbir Singh <[email protected]>
Cc: Jay Lan <[email protected]>
Cc: Jonathan Lim <[email protected]>
Cc: Oleg Nesterov <[email protected]>
---

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

2007-09-26 17:09:25

by Guillaume Chazarain

[permalink] [raw]
Subject: [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 <[email protected]>
Cc: Balbir Singh <[email protected]>
Cc: Jay Lan <[email protected]>
Cc: Jonathan Lim <[email protected]>
Cc: Oleg Nesterov <[email protected]>
---

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 <linux/taskstats.h>

#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;

2007-09-26 17:09:44

by Guillaume Chazarain

[permalink] [raw]
Subject: [PATCH 5/8] taskstats: factor out version and context switch accounting

All the specific taskstats fields should only be manipulated in
{add_tsk,fill_threadgroup}_stats().

Signed-off-by: Guillaume Chazarain <[email protected]>
Cc: Balbir Singh <[email protected]>
Cc: Jay Lan <[email protected]>
Cc: Jonathan Lim <[email protected]>
Cc: Oleg Nesterov <[email protected]>
---

kernel/taskstats.c | 21 ++++++++-------------
kernel/tsacct.c | 3 +++
2 files changed, 11 insertions(+), 13 deletions(-)


diff --git a/kernel/taskstats.c b/kernel/taskstats.c
index 972bbfa..f25bf03 100644
--- a/kernel/taskstats.c
+++ b/kernel/taskstats.c
@@ -218,6 +218,8 @@ static void fill_threadgroup_stats(struct taskstats *stats,
* per-task-foo-fill_threadgroup(stats, task);
*/

+ stats->version = TASKSTATS_VERSION;
+
/* fill in basic acct fields */
bacct_fill_threadgroup(stats, task);

@@ -241,9 +243,6 @@ static int fill_pid(pid_t pid, struct task_struct *tsk, struct taskstats *stats)
get_task_struct(tsk);

memset(stats, 0, sizeof(*stats));
- stats->version = TASKSTATS_VERSION;
- stats->nvcsw = tsk->nvcsw;
- stats->nivcsw = tsk->nivcsw;
add_tsk_stats(stats, tsk);
fill_threadgroup_stats(stats, tsk);

@@ -278,15 +277,12 @@ static int fill_tgid(pid_t tgid, struct task_struct *first,

tsk = first;
do {
- if (tsk->exit_state)
- continue;
- /*
- * This check is racy as a thread could exit just right
- * now and have its statistics accounted twice.
- */
- add_tsk_stats(stats, tsk);
- stats->nvcsw += tsk->nvcsw;
- stats->nivcsw += tsk->nivcsw;
+ if (!tsk->exit_state)
+ /*
+ * This check is racy as a thread could exit just right
+ * now and have its statistics accounted twice.
+ */
+ add_tsk_stats(stats, tsk);
} while_each_thread(first, tsk);

fill_threadgroup_stats(stats, first->group_leader);
@@ -294,7 +290,6 @@ static int fill_tgid(pid_t tgid, struct task_struct *first,
rc = 0;
out:
rcu_read_unlock();
- stats->version = TASKSTATS_VERSION;
return rc;
}

diff --git a/kernel/tsacct.c b/kernel/tsacct.c
index 4caea73..73b4c69 100644
--- a/kernel/tsacct.c
+++ b/kernel/tsacct.c
@@ -57,6 +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;
+
+ stats->nvcsw += tsk->nvcsw;
+ stats->nivcsw += tsk->nivcsw;
}

void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct *tsk)

2007-09-26 17:09:57

by Guillaume Chazarain

[permalink] [raw]
Subject: [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 <[email protected]>
Cc: Balbir Singh <[email protected]>
Cc: Jay Lan <[email protected]>
Cc: Jonathan Lim <[email protected]>
Cc: Oleg Nesterov <[email protected]>
---

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);

2007-09-26 17:10:35

by Guillaume Chazarain

[permalink] [raw]
Subject: [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 <[email protected]>
Cc: Balbir Singh <[email protected]>
Cc: Jay Lan <[email protected]>
Cc: Jonathan Lim <[email protected]>
Cc: Oleg Nesterov <[email protected]>
---

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;

2007-09-26 17:10:50

by Guillaume Chazarain

[permalink] [raw]
Subject: [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 <[email protected]>
Cc: Michael Neuling <[email protected]>
Cc: Balbir Singh <[email protected]>
Cc: Jay Lan <[email protected]>
Cc: Jonathan Lim <[email protected]>
Cc: Oleg Nesterov <[email protected]>
---

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 */
};


2007-09-26 17:13:06

by Guillaume Chazarain

[permalink] [raw]
Subject: Re: [PATCH 1/8] taskstats: fix indentation of long argument lists

Sorry for the resend, this is the same version as few minutes ago, but
stgit 0.13 correctly handles the mail --auto flag (unlike stgit
0.12.1 :-( ) so all recipients are actually Cced.

--
Guillaume

2007-09-26 20:02:20

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 1/8] taskstats: fix indentation of long argument lists

On Wed, 26 Sep 2007 19:08:18 +0200
Guillaume Chazarain <[email protected]> wrote:

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

It is unclear to me whether this patch series replaces, augments or
conflicts with your earlier patch series starting with "taskstats: separate
PID/TGID stats producers to complete the TGID ones".

But it doesn't matter much - I think I'll duck both patch series for now.
The gap between mainline and -mm is so large that it is becoming
unmanageable so let's concentrate on the review and stabilisation work for
now please, rather than adding more stuff.

Also, I don't think we're seeing enough review and test from people on
these patch series - I don't have time to do it all. (Well, apparently I
do, but I don't think it's a good situation).

2007-09-27 08:29:20

by Balbir Singh

[permalink] [raw]
Subject: Re: [PATCH 1/8] taskstats: fix indentation of long argument lists

Andrew Morton wrote:
> On Wed, 26 Sep 2007 19:08:18 +0200
> Guillaume Chazarain <[email protected]> wrote:
>
>> 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.
>
> It is unclear to me whether this patch series replaces, augments or
> conflicts with your earlier patch series starting with "taskstats: separate
> PID/TGID stats producers to complete the TGID ones".
>
> But it doesn't matter much - I think I'll duck both patch series for now.
> The gap between mainline and -mm is so large that it is becoming
> unmanageable so let's concentrate on the review and stabilisation work for
> now please, rather than adding more stuff.
>
> Also, I don't think we're seeing enough review and test from people on
> these patch series - I don't have time to do it all. (Well, apparently I
> do, but I don't think it's a good situation).

Hi, Andrew,

I've been able to review some of these patches, but not test them.
I'll find some time to test these patches.

--
Warm Regards,
Balbir Singh
Linux Technology Center
IBM, ISTL

2007-09-27 08:40:10

by Guillaume Chazarain

[permalink] [raw]
Subject: Re: [PATCH 1/8] taskstats: fix indentation of long argument lists

Le Wed, 26 Sep 2007 13:01:44 -0700,
Andrew Morton <[email protected]> a écrit :

> Also, I don't think we're seeing enough review and test from people on
> these patch series - I don't have time to do it all. (Well, apparently I
> do, but I don't think it's a good situation).

I don't object to your decision, but I think it's a bit unfair to
Balbir and Oleg "sharp eye" Nesterov.

Going to hammer 2.6.23-rc8-mm1 ;-)

--
Guillaume