Re: [PATCH 1/3] taskstats: separate PID/TGID stats producers to complete the TGID ones
Le Sat, 22 Sep 2007 23:36:29 +0530, Balbir Singh <[EMAIL PROTECTED]> a écrit : [reordered] > How about calling this one fill_threadgroup_stats()? > How about we call function add_tsk_stats()? > I still prefer braces around do <--> while, I think the code is easier > to read with them. > Could we further split the tsacct and delayacct/taskstats patches. Hi Balbir, Thank for your review, hopefully I addressed all your items in this series. > So we always call fill_threadgroup later, is there a reason for > that. Can the order of calls be arbitrary or is there a dependency? Yes, as you saw there is this dependency, it is needed only for [PATCH 7/8] taskstats: fix stats->ac_exitcode to work on threads and use group_exit_code but I found it simpler to put the calls in the right order from the beginning. Thanks again, and sorry for the double mail bomb. -- Guillaume - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] taskstats: separate PID/TGID stats producers to complete the TGID ones
Le Sat, 22 Sep 2007 23:36:29 +0530, Balbir Singh [EMAIL PROTECTED] a écrit : [reordered] How about calling this one fill_threadgroup_stats()? How about we call function add_tsk_stats()? I still prefer braces around do -- while, I think the code is easier to read with them. Could we further split the tsacct and delayacct/taskstats patches. Hi Balbir, Thank for your review, hopefully I addressed all your items in this series. So we always call fill_threadgroup later, is there a reason for that. Can the order of calls be arbitrary or is there a dependency? Yes, as you saw there is this dependency, it is needed only for [PATCH 7/8] taskstats: fix stats-ac_exitcode to work on threads and use group_exit_code but I found it simpler to put the calls in the right order from the beginning. Thanks again, and sorry for the double mail bomb. -- Guillaume - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] taskstats: separate PID/TGID stats producers to complete the TGID ones
Guillaume Chazarain wrote:
> 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() and add_tsk().
> 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() and fill_threadgroup() 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 | 131
> ++
> kernel/tsacct.c | 106
> 4 files changed, 155 insertions(+), 96 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..93dffc2 100644
> --- a/include/linux/tsacct_kern.h
> +++ b/include/linux/tsacct_kern.h
> @@ -10,17 +10,23 @@
> #include
>
> #ifdef CONFIG_TASKSTATS
> -extern void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk);
> +void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct
> *task);
> +void bacct_add_tsk(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_fill_threadgroup(struct taskstats *stats, struct
> task_struct *task)
> +{}
> +static inline void bacct_add_tsk(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_fill_threadgroup(struct taskstats *stats, struct task_struct
> *task);
> +void xacct_add_tsk(struct taskstats *stats, struct task_struct *p);
> extern void acct_update_integrals(struct task_struct *tsk);
> extern void acct_clear_integrals(struct task_struct *tsk);
> #else
> +static inline void xacct_fill_threadgroup(struct taskstats *stats, struct
> task_struct *task)
> +{}
> static inline void xacct_add_tsk(struct taskstats *stats, struct task_struct
> *p)
> {}
> static inline void acct_update_integrals(struct task_struct *tsk)
> diff --git a/kernel/taskstats.c b/kernel/taskstats.c
> index 059431e..ce43fae 100644
> --- a/kernel/taskstats.c
> +++ b/kernel/taskstats.c
> @@ -168,6 +168,68 @@ static void send_cpu_listeners(struct sk_buff *skb,
> up_write(>sem);
> }
>
> +/**
> + * fill_threadgroup - initialize some common stats for the thread group
> + * @stats: the taskstats to write into
> + * @task: the thread representing the whole group
> + *
> + * 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_threadgroup() methods deal with the first type while XXX_add_tsk()
> with
> + * the second.
> + */
> +static void fill_threadgroup(struct taskstats *stats, struct task_struct
> *task)
> +{
How about calling this one fill_threadgroup_stats()?
> + /*
> + * 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);
> + */
> +
> + stats->version = TASKSTATS_VERSION;
> +
> + /* fill in basic acct fields */
> + bacct_fill_threadgroup(stats, task);
> +
> + /* fill in extended acct fields */
> + xacct_fill_threadgroup(stats, task);
> +}
> +
> +/**
> + * add_tsk - combine some thread
Re: [PATCH 1/3] taskstats: separate PID/TGID stats producers to complete the TGID ones
Guillaume Chazarain wrote:
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() and add_tsk().
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() and fill_threadgroup() 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 | 131
++
kernel/tsacct.c | 106
4 files changed, 155 insertions(+), 96 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..93dffc2 100644
--- a/include/linux/tsacct_kern.h
+++ b/include/linux/tsacct_kern.h
@@ -10,17 +10,23 @@
#include linux/taskstats.h
#ifdef CONFIG_TASKSTATS
-extern void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk);
+void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct
*task);
+void bacct_add_tsk(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_fill_threadgroup(struct taskstats *stats, struct
task_struct *task)
+{}
+static inline void bacct_add_tsk(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_fill_threadgroup(struct taskstats *stats, struct task_struct
*task);
+void xacct_add_tsk(struct taskstats *stats, struct task_struct *p);
extern void acct_update_integrals(struct task_struct *tsk);
extern void acct_clear_integrals(struct task_struct *tsk);
#else
+static inline void xacct_fill_threadgroup(struct taskstats *stats, struct
task_struct *task)
+{}
static inline void xacct_add_tsk(struct taskstats *stats, struct task_struct
*p)
{}
static inline void acct_update_integrals(struct task_struct *tsk)
diff --git a/kernel/taskstats.c b/kernel/taskstats.c
index 059431e..ce43fae 100644
--- a/kernel/taskstats.c
+++ b/kernel/taskstats.c
@@ -168,6 +168,68 @@ static void send_cpu_listeners(struct sk_buff *skb,
up_write(listeners-sem);
}
+/**
+ * fill_threadgroup - initialize some common stats for the thread group
+ * @stats: the taskstats to write into
+ * @task: the thread representing the whole group
+ *
+ * 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_threadgroup() methods deal with the first type while XXX_add_tsk()
with
+ * the second.
+ */
+static void fill_threadgroup(struct taskstats *stats, struct task_struct
*task)
+{
How about calling this one fill_threadgroup_stats()?
+ /*
+ * 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);
+ */
+
+ stats-version = TASKSTATS_VERSION;
+
+ /* fill in basic acct fields */
+ bacct_fill_threadgroup(stats, task);
+
+ /* fill in extended acct fields */
+ xacct_fill_threadgroup(stats, task);
+}
+
+/**
+ * add_tsk - combine some thread specific stats in a taskstats
+ * @stats: the taskstats to write into
+ * @task: the thread to combine
+ *
+ * Stats
