| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <44CE66C3.7020804@watson.ibm.com>
Date: Mon, 31 Jul 2006 16:23:31 -0400
From: Shailabh Nagar <nagar@...son.ibm.com>
To: Jay Lan <jlan@....com>
CC: Andrew Morton <akpm@...l.org>, lkml <linux-kernel@...r.kernel.org>,
Balbir Singh <balbir@...ibm.com>, Jes Sorensen <jes@....com>,
Chris Sturtivant <csturtiv@....com>, Tony Ernst <tee@....com>
Subject: Re: [patch 1/3] add basic accounting fields to taskstats
Jay Lan wrote:
> This patch adds a few basic accounting fields to the taskstat
> struct and a bacct_add_tsk() routine to fill the data on
> exit.
>
>
> Signed-off-by: Jay Lan <jlan@....com>
>
>
> ------------------------------------------------------------------------
>
> Index: linux/include/linux/taskstats.h
> ===================================================================
> --- linux.orig/include/linux/taskstats.h 2006-07-31 11:38:54.132326042 -0700
> +++ linux/include/linux/taskstats.h 2006-07-31 11:42:10.634609610 -0700
> @@ -2,6 +2,7 @@
> *
> * Copyright (C) Shailabh Nagar, IBM Corp. 2006
> * (C) Balbir Singh, IBM Corp. 2006
> + * (C) Jay Lan, SGI, 2006
> *
> * This program is free software; you can redistribute it and/or modify it
> * under the terms of version 2.1 of the GNU Lesser General Public License
> @@ -29,13 +30,18 @@
> * c) add new fields after version comment; maintain 64-bit alignment
> */
>
> -#define TASKSTATS_VERSION 1
> +#define TASKSTATS_VERSION 2
> +#define TASK_COMM_LEN 16
>
> struct taskstats {
>
> /* Version 1 */
> __u16 version;
> - __u16 padding[3]; /* Userspace should not interpret the padding
> + __u8 ac_flag; /* Record flags */
> + __u8 ac_nice; /* task_nice */
> + __u8 ac_sched; /* Scheduling discipline */
> + __u8 ac_pad;
[Minor comment] The choice of fields to fill the padding is visually separate
from the delay accounting fields which follow. Can a single field of the same
length, say ac_uid, be put here instead ?
Also, is the ac_ prefix needed for the common fields like comm, uid, gid, pid
that are pretty much coming straight out of task_struct ?
> + __u16 padding; /* Userspace should not interpret the padding
Combine the padding's ?
> * field which can be replaced by useful
> * fields if struct taskstats is extended.
> */
> @@ -88,6 +94,19 @@ struct taskstats {
> __u64 cpu_run_virtual_total;
> /* Delay accounting fields end */
> /* version 1 ends here */
> +
> + /* Basic Accounting Fields start */
> + char ac_comm[TASK_COMM_LEN]; /* Command name */
> + __u32 ac_exitcode; /* Exit status */
> + __u32 ac_uid; /* User ID */
> + __u32 ac_gid; /* Group ID */
> + __u32 ac_pid; /* Process ID */
> + __u32 ac_ppid; /* Parent process ID */
> + __u32 ac_btime; /* Begin time [sec sinec 1970] */
s/sinec/since
> + __u64 ac_etime; /* Elapsed time [usec] */
> + __u64 ac_utime; /* User CPU time [usec] */
> + __u64 ac_stime; /* SYstem CPU time [usec] */
> + /* Basic Accounting Fields end */
> };
>
>
> Index: linux/kernel/taskstats.c
> ===================================================================
> --- linux.orig/kernel/taskstats.c 2006-07-31 11:38:54.160326367 -0700
> +++ linux/kernel/taskstats.c 2006-07-31 11:44:54.952523699 -0700
> @@ -3,6 +3,7 @@
> *
> * Copyright (C) Shailabh Nagar, IBM Corp. 2006
> * (C) Balbir Singh, IBM Corp. 2006
> + * (C) Jay Lan, SGI, 2006
> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License as published by
> @@ -18,6 +19,7 @@
>
> #include <linux/kernel.h>
> #include <linux/taskstats_kern.h>
> +#include <linux/acct.h>
> #include <linux/delayacct.h>
> #include <linux/cpumask.h>
> #include <linux/percpu.h>
> @@ -172,6 +174,53 @@ static void send_cpu_listeners(struct sk
> up_write(&listeners->sem);
> }
>
> +
> +#define USEC_PER_TICK (USEC_PER_SEC/HZ)
> +/*
> + * fill in basic accounting fields
> + */
> +static void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
> +{
> + u64 run_time;
> + struct timespec uptime;
> +
> + /* calculate run_time in nsec */
> + do_posix_clock_monotonic_gettime(&uptime);
> + run_time = (u64)uptime.tv_sec*NSEC_PER_SEC + uptime.tv_nsec;
> + run_time -= (u64)current->group_leader->start_time.tv_sec * NSEC_PER_SEC
> + + current->group_leader->start_time.tv_nsec;
> + do_div(run_time, NSEC_PER_USEC); /* rebase run_time to usec */
> + stats->ac_etime = run_time;
> + do_div(run_time, USEC_PER_SEC); /* rebase run_time to sec */
> + stats->ac_btime = xtime.tv_sec - run_time;
Above chunk can be written more succintly using the new timespec_sub
function added.
ts = timespec_sub(¤t->group_leader->start_time, &uptime);
stats->ac_etime = timespec_to_ns(&ts);
stats->ac_btime = xtime.tv_sec - ts.tv_sec;
Also, if there's any chance of run_time going negative due to overflow, that
needs to be handled in both original and succinct code (in latter you can
check for ts->tv_sec being negative).
> + if (thread_group_leader(tsk)) {
> + stats->ac_exitcode = tsk->exit_code;
> + if (tsk->flags & PF_FORKNOEXEC)
> + stats->ac_flag |= AFORK;
> + }
> + 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_nice = task_nice(tsk);
> + stats->ac_sched = tsk->policy;
> + stats->ac_uid = tsk->uid;
> + stats->ac_gid = tsk->gid;
> + stats->ac_pid = tsk->pid;
> + stats->ac_ppid = (tsk->parent) ? tsk->parent->pid : 0;
> + stats->ac_utime = tsk->utime * USEC_PER_TICK;
> + stats->ac_stime = tsk->stime * USEC_PER_TICK;
> + /* Each process gets a minimum of a half tick cpu time */
> + if ((stats->ac_utime == 0) && (stats->ac_stime == 0)) {
> + stats->ac_stime = USEC_PER_TICK/2;
> + }
> +
> + strncpy(stats->ac_comm, tsk->comm, sizeof(stats->ac_comm));
> +}
> +
> +
> static int fill_pid(pid_t pid, struct task_struct *pidtsk,
> struct taskstats *stats)
> {
> @@ -200,6 +249,9 @@ static int fill_pid(pid_t pid, struct ta
> delayacct_add_tsk(stats, tsk);
> stats->version = TASKSTATS_VERSION;
>
> + /* fill in basic acct fields */
> + bacct_add_tsk(stats, tsk);
> +
Should be added before stats->version is filled since that is
a generic field, not delay accounting specific.
> /* Define err: label here if needed */
> put_task_struct(tsk);
> return rc;
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists