Messages in this thread Patch in this message |  | | Date | Wed, 9 Jan 2013 00:22:33 +0100 | Subject | Re: [PATCH 04/33] cputime: Allow dynamic switch between tick/virtual based cputime accounting | From | Frederic Weisbecker <> |
| |
2013/1/8 Steven Rostedt <rostedt@goodmis.org>: > On Tue, 2013-01-08 at 03:08 +0100, Frederic Weisbecker wrote: > >> @@ -439,29 +443,13 @@ void account_idle_ticks(unsigned long ticks) >> >> account_idle_time(jiffies_to_cputime(ticks)); >> } >> - >> #endif >> >> + > > Spurious newline.
There may be even more around on the other patches :)
>> +#ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE >> +void task_cputime_adjusted(struct task_struct *p, cputime_t *ut, cputime_t *st) >> +{ >> + *ut = p->utime; >> + *st = p->stime; >> +} > > Why not keep this out in the open like: > > static void __task_cputime_adjusted() { > ... > } > > #ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE > void task_cputime_adjusted(...) > { > __task_cputime_adjusted(p, ut, st); > } > >> + >> +void thread_group_cputime_adjusted(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; >> +} >> +#else /* !CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */ >> >> #ifndef nsecs_to_cputime >> # define nsecs_to_cputime(__nsecs) nsecs_to_jiffies(__nsecs) >> @@ -548,6 +553,12 @@ static void cputime_adjust(struct task_cputime *curr, >> { >> cputime_t rtime, utime, total; >> >> + if (vtime_accounting_enabled()) { >> + *ut = curr->utime; >> + *st = curr->stime; > > Then here, we can do: > > __task_cputime_adjusted(curr, ut, st); >> + return;
Note curr above is not a task but a struct task_cputime. But it could be:
__task_cputime_adjusted(curr->utime, curr->stime, ut, st);
But thinking more about it, I should just remove this:
+ if (vtime_accounting_enabled()) { + *ut = curr->utime; + *st = curr->stime;
It concerns CONFIG_VIRT_CPU_ACCOUNTING_GEN which is actually not enough precise (it's jiffies based) and thus needs the same adjusting performed on normal tick based cputime.
> > Isn't suppose to do basically the same thing, when > vtime_accounting_enabled() is set? > >> + } >> + >> utime = curr->utime; >> total = utime + curr->stime; >> >> @@ -601,7 +612,7 @@ void thread_group_cputime_adjusted(struct task_struct *p, cputime_t *ut, cputime >> thread_group_cputime(p, &cputime); >> cputime_adjust(&cputime, &p->signal->prev_cputime, ut, st); >> } >> -#endif >> +#endif /* !CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */ >> >> #ifdef CONFIG_VIRT_CPU_ACCOUNTING_GEN >> static DEFINE_PER_CPU(long, last_jiffies) = INITIAL_JIFFIES; >> @@ -643,6 +654,11 @@ void vtime_account_idle(struct task_struct *tsk) >> account_idle_time(delta_cpu); >> } >> >> +bool vtime_accounting_enabled(void) >> +{ >> + return context_tracking_active(); >> +} >> + >> static int __cpuinit vtime_cpu_notify(struct notifier_block *self, >> unsigned long action, void *hcpu) >> { >> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c >> index fb8e5e4..314b9ee 100644 >> --- a/kernel/time/tick-sched.c >> +++ b/kernel/time/tick-sched.c >> @@ -632,8 +632,11 @@ static void tick_nohz_restart_sched_tick(struct tick_sched *ts, ktime_t now) >> >> static void tick_nohz_account_idle_ticks(struct tick_sched *ts) >> { >> -#ifndef CONFIG_VIRT_CPU_ACCOUNTING >> +#ifndef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE >> unsigned long ticks; >> + >> + if (vtime_accounting_enabled()) >> + return; > > If this can be dynamically changed at runtime, wouldn't some of these > accounting variables get corrupted? Like the last_jiffies per_cpu > variable?
It can't yet be dynamically changed at runtime because full dynticks CPUs are defined through boot parameters. But it's indeed something we'll need to care about when we'll have a runtime interface.
|  |