On Fri, 31 Aug 2007 14:35:35 +0200
Guillaume Chazarain <[email protected]> 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. This makes TASKSTATS_CMD_ATTR_TGID usable in a similar
> fashion to TASKSTATS_CMD_ATTR_PID, for commands like iotop -P
> (http://guichaz.free.fr/misc/iotop.py).
>
> Here is the output of the testcase before the patch:
>
> ...
> Documentation/accounting/dump-test.c | 314 +++++++++++++++++++++++++++++++++++
Another C file in the Documentation directory. Sigh. At kernel summit I
suggested that we should be putting these things in a place from where we
can actually build and install them. People said there was no need to do
that because kernel developers can now easily get new stuff into
util-linux. I don't believe them. Wanna be a guinea pig?
> +static void loop_reading(const char *filename)
> +{
> + int fd = open(filename, O_RDONLY);
> + char buffer[4096];
> +
> + if (fd < 0) {
> + perror(filename);
> + return;
> + }
> +
> + for (;;) {
> + lseek(fd, 0, SEEK_SET);
> + while (read(fd, buffer, sizeof(buffer)) > 0) ;
newline here. Just because it's userspace doesn't mean that it needs to
look crappy, despite all the code out there which disproves this ;)
I think you just invented pread().
> + }
> +}
> +
>
> ...
>
> --- a/kernel/taskstats.c Fri Aug 31 01:42:23 2007 -0700
> +++ b/kernel/taskstats.c Fri Aug 31 13:36:29 2007 +0200
> @@ -168,6 +168,60 @@ static void send_cpu_listeners(struct sk
> up_write(&listeners->sem);
> }
>
> +/*
> + * 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)
> +{
> + /*
> + * 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);
> +}
> +
> +/*
> + * Stats specific to each thread in the thread group. Stats of @task should be
> + * combined with those already present in @stats. add_tsk() works in
> + * conjunction with fill_threadgroup(), taskstats fields should not be touched
> + * by both functions.
> + */
It's odd to use kerneldoc-style markup in a non-kerneldoc comment.
> @@ -232,32 +272,21 @@ static int fill_tgid(pid_t tgid, struct
> else
> memset(stats, 0, sizeof(*stats));
>
> + leader = first->group_leader;
> + get_task_struct(leader);
> + fill_threadgroup(stats, leader);
> + put_task_struct(leader);
> +
Are the get_task_struct/put_task_struct here actually needed?
-
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/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
