cputime: Cleanups on adjusted cputime code

PATCH In review Frederic Weisbecker 3 participants 11 messages Nov 23, 2012 – Nov 28, 2012

Subsystems (4)

MM core Scheduler exec and binfmt Proc filesystem

lore

Share & tools

lore

Frederic Weisbecker

author

2012-11-23 14:21 UTC #0 lore raw

cover cputime: Cleanups on adjusted cputime code

Hi,

Not vtime related that time, just a few supplementary cleanups on the part that computes the adjusted cputime values.

Thanks.

Frederic Weisbecker (3):
cputime: Move thread_group_cputime() to sched code
cputime: Rename thread_group_times to thread_group_cputime_adjusted
cputime: Consolidate cputime adjustment code

 fs/proc/array.c           |    4 +-
 include/linux/sched.h     |   13 +++++--
 kernel/exit.c             |    4 +-
 kernel/fork.c             |    2 +-
 kernel/posix-cpu-timers.c |   24 -------------
 kernel/sched/cputime.c    |   80 ++++++++++++++++++++++++++++++--------------
 kernel/sys.c              |    6 ++--
 7 files changed, 71 insertions(+), 62 deletions(-)

Signature · 1 line

1.7.5.4

Frederic Weisbecker

author

2012-11-23 14:22 UTC #1 lore raw

PATCH 1/3 cputime: Move thread_group_cputime() to sched code

thread_group_cputime() is a general cputime API that is not only used by posix cpu timer. Let's move this helper to sched code.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Steven Rostedt <[email protected]>

---
 kernel/posix-cpu-timers.c |   24 ------------------------
 kernel/sched/cputime.c    |   28 ++++++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 24 deletions(-)

kernel/posix-cpu-timers.c +0 ~~-24~~

index 125cb67..d738402 100644
@@ -217,30 +217,6 @@ static int cpu_clock_sample(const clockid_t which_clock, struct task_struct *p,
 	return 0;
 }
 
-void thread_group_cputime(struct task_struct *tsk, struct task_cputime *times)
-{
-	struct signal_struct *sig = tsk->signal;
-	struct task_struct *t;
-
-	times->utime = sig->utime;
-	times->stime = sig->stime;
-	times->sum_exec_runtime = sig->sum_sched_runtime;
-
-	rcu_read_lock();
-	/* make sure we can trust tsk->thread_group list */
-	if (!likely(pid_alive(tsk)))
-		goto out;
-
-	t = tsk;
-	do {
-		times->utime += t->utime;
-		times->stime += t->stime;
-		times->sum_exec_runtime += task_sched_runtime(t);
-	} while_each_thread(tsk, t);
-out:
-	rcu_read_unlock();
-}
-
 static void update_gt_cputime(struct task_cputime *a, struct task_cputime *b)
 {
 	if (b->utime > a->utime)

kernel/sched/cputime.c +28 -0

index 8d859da..e56f138 100644
@@ -288,6 +288,34 @@ static __always_inline bool steal_account_process_tick(void)
 	return false;
 }
 
+/*
+ * Accumulate raw cputime values of dead tasks (sig->[us]time) and live
+ * tasks (sum on group iteration) belonging to @tsk's group.
+ */
+void thread_group_cputime(struct task_struct *tsk, struct task_cputime *times)
+{
+	struct signal_struct *sig = tsk->signal;
+	struct task_struct *t;
+
+	times->utime = sig->utime;
+	times->stime = sig->stime;
+	times->sum_exec_runtime = sig->sum_sched_runtime;
+
+	rcu_read_lock();
+	/* make sure we can trust tsk->thread_group list */
+	if (!likely(pid_alive(tsk)))
+		goto out;
+
+	t = tsk;
+	do {
+		times->utime += t->utime;
+		times->stime += t->stime;
+		times->sum_exec_runtime += task_sched_runtime(t);
+	} while_each_thread(tsk, t);
+out:
+	rcu_read_unlock();
+}
+
 #ifndef CONFIG_VIRT_CPU_ACCOUNTING
 
 #ifdef CONFIG_IRQ_TIME_ACCOUNTING

Signature · 1 line

1.7.5.4

Frederic Weisbecker

author

2012-11-23 14:21 UTC #2 lore raw

PATCH 2/3 cputime: Rename thread_group_times to thread_group_cputime_adjusted

In reply to Frederic Weisbecker

We have thread_group_cputime() and thread_group_times(). The naming doesn't provide enough information about the difference between these two APIs.

To lower the confusion, rename thread_group_times() to thread_group_cputime_adjusted(). This name better suggests that it's a version of thread_group_cputime() that does some stabilization on the raw cputime values. ie here: scale on top of CFS runtime stats and bound lower value for monotonicity.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Steven Rostedt <[email protected]>

---
 fs/proc/array.c        |    4 ++--
 include/linux/sched.h  |    4 ++--
 kernel/exit.c          |    4 ++--
 kernel/sched/cputime.c |    8 ++++----
 kernel/sys.c           |    6 +++---
 5 files changed, 13 insertions(+), 13 deletions(-)

fs/proc/array.c +2 -2

index c1c207c..d369670 100644
@@ -438,7 +438,7 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 
 			min_flt += sig->min_flt;
 			maj_flt += sig->maj_flt;
-			thread_group_times(task, &utime, &stime);
+			thread_group_cputime_adjusted(task, &utime, &stime);
 			gtime += sig->gtime;
 		}

@@ -454,7 +454,7 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 	if (!whole) {
 		min_flt = task->min_flt;
 		maj_flt = task->maj_flt;
-		task_times(task, &utime, &stime);
+		task_cputime_adjusted(task, &utime, &stime);
 		gtime = task->gtime;
 	}

include/linux/sched.h +2 -2

index e1581a0..e75cab5 100644
@@ -1751,8 +1751,8 @@ static inline void put_task_struct(struct task_struct *t)
 		__put_task_struct(t);
 }
 
-extern void task_times(struct task_struct *p, cputime_t *ut, cputime_t *st);
-extern void thread_group_times(struct task_struct *p, cputime_t *ut, cputime_t *st);
+extern void task_cputime_adjusted(struct task_struct *p, cputime_t *ut, cputime_t *st);
+extern void thread_group_cputime_adjusted(struct task_struct *p, cputime_t *ut, cputime_t *st);
 
 /*
  * Per process flags

kernel/exit.c +2 -2

index 346616c..618f7ee 100644
@@ -1186,11 +1186,11 @@ static int wait_task_zombie(struct wait_opts *wo, struct task_struct *p)
 		 * as other threads in the parent group can be right
 		 * here reaping other children at the same time.
 		 *
-		 * We use thread_group_times() to get times for the thread
+		 * We use thread_group_cputime_adjusted() to get times for the thread
 		 * group, which consolidates times for all threads in the
 		 * group including the group leader.
 		 */
-		thread_group_times(p, &tgutime, &tgstime);
+		thread_group_cputime_adjusted(p, &tgutime, &tgstime);
 		spin_lock_irq(&p->real_parent->sighand->siglock);
 		psig = p->real_parent->signal;
 		sig = p->signal;

kernel/sched/cputime.c +4 -4

index e56f138..7dc1553 100644
@@ -445,13 +445,13 @@ void account_idle_ticks(unsigned long ticks)
  * Use precise platform statistics if available:
  */
 #ifdef CONFIG_VIRT_CPU_ACCOUNTING
-void task_times(struct task_struct *p, cputime_t *ut, cputime_t *st)
+void task_cputime_adjusted(struct task_struct *p, cputime_t *ut, cputime_t *st)
 {
 	*ut = p->utime;
 	*st = p->stime;
 }
 
-void thread_group_times(struct task_struct *p, cputime_t *ut, cputime_t *st)
+void thread_group_cputime_adjusted(struct task_struct *p, cputime_t *ut, cputime_t *st)
 {
 	struct task_cputime cputime;

@@ -516,7 +516,7 @@ static cputime_t scale_utime(cputime_t utime, cputime_t rtime, cputime_t total)
 	return (__force cputime_t) temp;
 }
 
-void task_times(struct task_struct *p, cputime_t *ut, cputime_t *st)
+void task_cputime_adjusted(struct task_struct *p, cputime_t *ut, cputime_t *st)
 {
 	cputime_t rtime, utime = p->utime, total = utime + p->stime;

@@ -543,7 +543,7 @@ void task_times(struct task_struct *p, cputime_t *ut, cputime_t *st)
 /*
  * Must be called with siglock held.
  */
-void thread_group_times(struct task_struct *p, cputime_t *ut, cputime_t *st)
+void thread_group_cputime_adjusted(struct task_struct *p, cputime_t *ut, cputime_t *st)
 {
 	struct signal_struct *sig = p->signal;
 	struct task_cputime cputime;

kernel/sys.c +3 -3

index e6e0ece..265b376 100644
@@ -1046,7 +1046,7 @@ void do_sys_times(struct tms *tms)
 	cputime_t tgutime, tgstime, cutime, cstime;
 
 	spin_lock_irq(&current->sighand->siglock);
-	thread_group_times(current, &tgutime, &tgstime);
+	thread_group_cputime_adjusted(current, &tgutime, &tgstime);
 	cutime = current->signal->cutime;
 	cstime = current->signal->cstime;
 	spin_unlock_irq(&current->sighand->siglock);

@@ -1704,7 +1704,7 @@ static void k_getrusage(struct task_struct *p, int who, struct rusage *r)
 	utime = stime = 0;
 
 	if (who == RUSAGE_THREAD) {
-		task_times(current, &utime, &stime);
+		task_cputime_adjusted(current, &utime, &stime);
 		accumulate_thread_rusage(p, r);
 		maxrss = p->signal->maxrss;
 		goto out;

@@ -1730,7 +1730,7 @@ static void k_getrusage(struct task_struct *p, int who, struct rusage *r)
 				break;
 
 		case RUSAGE_SELF:
-			thread_group_times(p, &tgutime, &tgstime);
+			thread_group_cputime_adjusted(p, &tgutime, &tgstime);
 			utime += tgutime;
 			stime += tgstime;
 			r->ru_nvcsw += p->signal->nvcsw;

Signature · 1 line

1.7.5.4

Steven Rostedt

reviewer

2012-11-26 18:56 UTC #3 lore raw

Re: [PATCH 2/3] cputime: Rename thread_group_times to thread_group_cputime_adjusted

Quoted 9 lines · Frederic Weisbecker

We have thread_group_cputime() and thread_group_times(). The naming doesn't provide enough information about the difference between these two APIs.
To lower the confusion, rename thread_group_times() to thread_group_cputime_adjusted(). This name better suggests that it's a version of thread_group_cputime() that does some stabilization on the raw cputime values. ie here: scale on top of CFS runtime stats and bound lower value for monotonicity.

But, thread_group_times() does not do any type of adjustment. It only retrieves the cpu times:

void thread_group_times(struct task_struct *p, cputime_t *ut, cputime_t *st)

{
        struct task_cputime cputime;

        thread_group_cputime(p, &cputime);

        *ut = cputime.utime;
        *st = cputime.stime;
}

It retrieves the current times, it doesn't adjust them.

I'm thinking the current name is more accurate.

-- Steve

Frederic Weisbecker

author

2012-11-26 19:24 UTC #4 lore raw

Re: [PATCH 2/3] cputime: Rename thread_group_times to thread_group_cputime_adjusted

2012/11/26 Steven Rostedt <[email protected]>:

Quoted 15 lines · Frederic Weisbecker

Quoted 9 lines
We have thread_group_cputime() and thread_group_times(). The naming doesn't provide enough information about the difference between these two APIs.
To lower the confusion, rename thread_group_times() to thread_group_cputime_adjusted(). This name better suggests that it's a version of thread_group_cputime() that does some stabilization on the raw cputime values. ie here: scale on top of CFS runtime stats and bound lower value for monotonicity.
But, thread_group_times() does not do any type of adjustment. It only retrieves the cpu times:
void thread_group_times(struct task_struct *p, cputime_t *ut, cputime_t *st)

Quoted 8 lines

{
        struct task_cputime cputime;

        thread_group_cputime(p, &cputime);

        *ut = cputime.utime;
        *st = cputime.stime;
}

This is the CONFIG_VIRT_CPU_ACCOUNTING only version. It also needs some monotonicity guard IMO but that's another issue. But please look at the other version.

Quoted 3 lines

It retrieves the current times, it doesn't adjust them.
I'm thinking the current name is more accurate.

Steven Rostedt

reviewer

2012-11-26 19:33 UTC #5 lore raw

Re: [PATCH 2/3] cputime: Rename thread_group_times to thread_group_cputime_adjusted

Quoted 21 lines · Frederic Weisbecker

2012/11/26 Steven Rostedt <[email protected]>:
Quoted 20 lines · Frederic Weisbecker
Quoted 9 lines
We have thread_group_cputime() and thread_group_times(). The naming doesn't provide enough information about the difference between these two APIs.
To lower the confusion, rename thread_group_times() to thread_group_cputime_adjusted(). This name better suggests that it's a version of thread_group_cputime() that does some stabilization on the raw cputime values. ie here: scale on top of CFS runtime stats and bound lower value for monotonicity.
But, thread_group_times() does not do any type of adjustment. It only retrieves the cpu times:
void thread_group_times(struct task_struct *p, cputime_t *ut, cputime_t *st)
{
        struct task_cputime cputime;

        thread_group_cputime(p, &cputime);

Quoted 7 lines

Quoted 3 lines
        *ut = cputime.utime;
        *st = cputime.stime;
}
This is the CONFIG_VIRT_CPU_ACCOUNTING only version. It also needs some monotonicity guard IMO but that's another issue. But please look at the other version.

Yeah, I just noticed. That turkey juice is still having an affect on me ;-)

Quoted 4 lines

Quoted 4 lines
It retrieves the current times, it doesn't adjust them.
I'm thinking the current name is more accurate.

OK, let's take a look at the other version now:

void thread_group_times(struct task_struct *p, cputime_t *ut, cputime_t *st)

{
        struct signal_struct *sig = p->signal;
        struct task_cputime cputime;
        cputime_t rtime, utime, total;

        thread_group_cputime(p, &cputime);

        total = cputime.utime + cputime.stime;
        rtime = nsecs_to_cputime(cputime.sum_exec_runtime);

        if (total)
                utime = scale_utime(cputime.utime, rtime, total);
        else
                utime = rtime;

        sig->prev_utime = max(sig->prev_utime, utime);
        sig->prev_stime = max(sig->prev_stime, rtime - sig->prev_utime);

        *ut = sig->prev_utime;
        *st = sig->prev_stime;
}

So this version also updates the task's signal->prev_[us]times as well.

I guess I'll wait for you to explain to me more about what is going on :-)

-- Steve

Frederic Weisbecker

author

2012-11-27 23:51 UTC #6 lore raw

Re: [PATCH 2/3] cputime: Rename thread_group_times to thread_group_cputime_adjusted

2012/11/26 Steven Rostedt <[email protected]>:

Quoted 3 lines

OK, let's take a look at the other version now:
void thread_group_times(struct task_struct *p, cputime_t *ut, cputime_t *st)

So this does the same thing than thread_group_cputime(), ie: fetch the raw cputime stats from the task/signal struct, with a two adjustments:

* It scales the raw values with the CFS stats.
* It ensures the stats are increased monotonically across thread_group_times() calls

More details below:

Quoted 7 lines

{
        struct signal_struct *sig = p->signal;
        struct task_cputime cputime;
        cputime_t rtime, utime, total;

        thread_group_cputime(p, &cputime);

Quoted 7 lines

        total = cputime.utime + cputime.stime;
        rtime = nsecs_to_cputime(cputime.sum_exec_runtime);

        if (total)
                utime = scale_utime(cputime.utime, rtime, total);
        else
                utime = rtime;

raw cputime values (tsk->utime and tsk->stime) have a per tick granularity. So the precision is not the best. For example a tick can interrupt the same task 5 times while that task has actually ran for no more than a jiffy overall. This can happen if the task runs for short slices and get unlucky enough to often run at the same time the tick fired.

The opposite can also happen: the task has ran for 5 jiffies but wasn't much interrupted by the tick.

To fix this we scale utime and stime values against CFS accumulated runtime for the task. As follows:

total_ticks_runtime = utime + stime
utime = utime * (total_cfs_runtime / total_ticks_runtime) stime = total_cfs_runtime - utime

Quoted 3 lines

        sig->prev_utime = max(sig->prev_utime, utime);
        sig->prev_stime = max(sig->prev_stime, rtime - sig->prev_utime);

Now this scaling brings another problem. If between two calls of thread_group_times(), tsk->utime has increased a lot and the cfs tsk runtime hasn't increased that much, the resulting value of adjusted stime may decrease from the 2nd to the 1st call of the function. But userspace relies on the monotonicity of cputime. The same can happen with utime if tsk->stime has increased a lot.

To fix this we apply the above monotonicity fixup.

I can add these explanations on comments in a new patch.

Quoted 13 lines

        *ut = sig->prev_utime;
        *st = sig->prev_stime;
}
So this version also updates the task's signal->prev_[us]times as well.
I guess I'll wait for you to explain to me more about what is going on :-)
-- Steve

Steven Rostedt

reviewer

2012-11-28 00:52 UTC #7 lore raw

Re: [PATCH 2/3] cputime: Rename thread_group_times to thread_group_cputime_adjusted

Quoted 1 line · Frederic Weisbecker

To fix this we apply the above monotonicity fixup.

Thanks for the explanation, although I kind of figured it out already ;-)

Quoted 2 lines

I can add these explanations on comments in a new patch.

Comments are always good.

-- Steve

Frederic Weisbecker

author

2012-11-23 14:21 UTC #8 lore raw

PATCH 3/3 cputime: Consolidate cputime adjustment code

In reply to Frederic Weisbecker

task_cputime_adjusted() and thread_group_cputime_adjusted() essentially share the same code. They just don't use the same source:

* The first function uses the cputime in the task struct and the previous adjusted snapshot that ensures monotonicity.

* The second adds the cputime of all tasks in the group and the previous adjusted snapshot of the whole group from the signal structure.

Just consolidate the common code that does the adjustment. These functions just need to fetch the values from the appropriate source.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Steven Rostedt <[email protected]>

---
 include/linux/sched.h  |    9 +++++++--
 kernel/fork.c          |    2 +-
 kernel/sched/cputime.c |   46 +++++++++++++++++++++++-----------------------
 3 files changed, 31 insertions(+), 26 deletions(-)

include/linux/sched.h +7 -2

index e75cab5..d23204f 100644
@@ -433,6 +433,11 @@ struct cpu_itimer {
 	u32 incr_error;
 };
 
+struct cputime {
+	cputime_t utime;
+	cputime_t stime;
+};
+
 /**
  * struct task_cputime - collected CPU time counts
  * @utime:		time spent in user mode, in &cputime_t units

@@ -581,7 +586,7 @@ struct signal_struct {
 	cputime_t gtime;
 	cputime_t cgtime;
 #ifndef CONFIG_VIRT_CPU_ACCOUNTING
-	cputime_t prev_utime, prev_stime;
+	struct cputime prev_cputime;
 #endif
 	unsigned long nvcsw, nivcsw, cnvcsw, cnivcsw;
 	unsigned long min_flt, maj_flt, cmin_flt, cmaj_flt;

@@ -1340,7 +1345,7 @@ struct task_struct {
 	cputime_t utime, stime, utimescaled, stimescaled;
 	cputime_t gtime;
 #ifndef CONFIG_VIRT_CPU_ACCOUNTING
-	cputime_t prev_utime, prev_stime;
+	struct cputime prev_cputime;
 #endif
 	unsigned long nvcsw, nivcsw; /* context switch counts */
 	struct timespec start_time; 		/* monotonic time */

kernel/fork.c +1 -1

index 8b20ab7..0e7cdb9 100644
@@ -1222,7 +1222,7 @@ static struct task_struct *copy_process(unsigned long clone_flags,
 	p->utime = p->stime = p->gtime = 0;
 	p->utimescaled = p->stimescaled = 0;
 #ifndef CONFIG_VIRT_CPU_ACCOUNTING
-	p->prev_utime = p->prev_stime = 0;
+	p->prev_cputime.utime = p->prev_cputime.stime = 0;
 #endif
 #if defined(SPLIT_RSS_COUNTING)
 	memset(&p->rss_stat, 0, sizeof(p->rss_stat));

kernel/sched/cputime.c +23 ~~-23~~

72 lines not on this page · raw message

Signature · 1 line

1.7.5.4

Paul Gortmaker

2012-11-25 19:09 UTC #9 lore raw

Re: [PATCH 3/3] cputime: Consolidate cputime adjustment code

Quoted 31 lines · [[PATCH 3/3] cputime: Consolidate cputime adjustment code] On 23/11/2012 (Fri…

task_cputime_adjusted() and thread_group_cputime_adjusted() essentially share the same code. They just don't use the same source:
* The first function uses the cputime in the task struct and the previous adjusted snapshot that ensures monotonicity.
* The second adds the cputime of all tasks in the group and the previous adjusted snapshot of the whole group from the signal structure.
Just consolidate the common code that does the adjustment. These functions just need to fetch the values from the appropriate source.
Signed-off-by Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Steven Rostedt <[email protected]>
---
 include/linux/sched.h  |    9 +++++++--
 kernel/fork.c          |    2 +-
 kernel/sched/cputime.c |   46 +++++++++++++++++++++++-----------------------
 3 files changed, 31 insertions(+), 26 deletions(-)
include/linux/sched.h +0 -0
index e75cab5..d23204f 100644

include/linux/sched.h struct cpu_itimer { · line 440

Quoted 8 lines

include/linux/sched.h +5 -0

@@ -433,6 +433,11 @@ struct cpu_itimer {
 	u32 incr_error;
 };
 
+struct cputime {
+	cputime_t utime;
+	cputime_t stime;
+};
+

Hi Frederic,

This new struct cputime is a 2/3 subset of the three variable struct task_cputime we see right below. Maybe this is a stupid question, but I was wondering why you didn't re-use task_cputime, and ignore the sum_exec_runtime field -- vs. introducing this very similar struct?

Or maybe there is another way to consolidate the structs? With the two being so similar, I wonder if it will be confusing when to use which one of the two.

Thanks,
Paul.
--

/**
* struct task_cputime - collected CPU time counts
* @utime: time spent in user mode, in &cputime_t units
* @stime: time spent in kernel mode, in &cputime_t units
* @sum_exec_runtime: total time spent on the CPU, in nanoseconds
*
* This structure groups together three kinds of CPU time that are
* tracked for threads and thread groups. Most things considering
* CPU time want to group these counts together and treat all three
* of them in parallel.
*/

struct task_cputime {
        cputime_t utime;
        cputime_t stime;
        unsigned long long sum_exec_runtime;
};

Frederic Weisbecker

author

2012-11-26 00:16 UTC #10 lore raw

Re: [PATCH 3/3] cputime: Consolidate cputime adjustment code

2012/11/25 Paul Gortmaker <[email protected]>:

Quoted 14 lines

include/linux/sched.h struct cpu_itimer { · line 440
Quoted 11 lines
include/linux/sched.h +5 -0
@@ -433,6 +433,11 @@ struct cpu_itimer {
      u32 incr_error;
 };
 
+struct cputime {
+     cputime_t utime;
+     cputime_t stime;
+};
+
Hi Frederic,

Quoted 4 lines

This new struct cputime is a 2/3 subset of the three variable struct task_cputime we see right below. Maybe this is a stupid question, but I was wondering why you didn't re-use task_cputime, and ignore the sum_exec_runtime field -- vs. introducing this very similar struct?

Not a stupid question, I need to add a comment on that. I avoided to reuse struct task_cputime because sum_exec_runtime is unused and 8 wasted bytes in the middle of task_struct is very undesirable. As in signal struct.

Quoted 4 lines

Or maybe there is another way to consolidate the structs? With the two being so similar, I wonder if it will be confusing when to use which one of the two.

I can add a comment that tells when to use which. Other than that I'm short on ideas to consolidate both without creating a mess with long dereferencing expressions like tsk->cputime.utime and so.