Messages in this thread Patch in this message | | | Date | Sat, 30 Jan 2016 17:24:01 +0100 | From | Frederic Weisbecker <> | Subject | Re: [PATCH 3/4] time,acct: drop irq save & restore from __acct_update_integrals |
| |
On Fri, Jan 29, 2016 at 10:36:04PM -0500, riel@redhat.com wrote: > From: Rik van Riel <riel@redhat.com> > > It looks like all the call paths that lead to __acct_update_integrals > already have irqs disabled, and __acct_update_integrals does not need > to disable irqs itself. > > This is very convenient since about half the CPU time left in this > function was spent in local_irq_save alone. > > Performance of a microbenchmark that calls an invalid syscall > ten million times in a row on a nohz_full CPU improves 21% vs. > 4.5-rc1 with both the removal of divisions from __acct_update_integrals > and this patch, with runtime dropping from 3.7 to 2.9 seconds. > > With these patches applied, the highest remaining cpu user in > the trace is native_sched_clock, which is addressed in the next > patch. > > Suggested-by: Peter Zijlstra <peterz@infradead.org> > Signed-off-by: Rik van Riel <riel@redhat.com> > --- > kernel/tsacct.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/kernel/tsacct.c b/kernel/tsacct.c > index 8908f8b1d26e..b2663d699a72 100644 > --- a/kernel/tsacct.c > +++ b/kernel/tsacct.c > @@ -124,26 +124,22 @@ static void __acct_update_integrals(struct task_struct *tsk, > cputime_t utime, cputime_t stime) > { > cputime_t time, dtime; > - unsigned long flags; > u64 delta; > > if (unlikely(!tsk->mm)) > return; > > - local_irq_save(flags); > time = stime + utime; > dtime = time - tsk->acct_timexpd; > delta = cputime_to_nsecs(dtime); > > if (delta < TICK_NSEC) > - goto out; > + return; > > tsk->acct_timexpd = time; > /* The final unit will be Mbyte-usecs, see xacct_add_tsk */ > tsk->acct_rss_mem1 += delta * get_mm_rss(tsk->mm) / 1024; > tsk->acct_vm_mem1 += delta * tsk->mm->total_vm / 1024; > -out: > - local_irq_restore(flags); > }
I think you need this as well, because do_exit() probably doesn't have irqs disabled at that point:
diff --git a/kernel/tsacct.c b/kernel/tsacct.c index 975cb49..12c6047 100644 --- a/kernel/tsacct.c +++ b/kernel/tsacct.c @@ -153,9 +153,12 @@ static void __acct_update_integrals(struct task_struct *tsk, void acct_update_integrals(struct task_struct *tsk) { cputime_t utime, stime; + unsigned long flags; task_cputime(tsk, &utime, &stime); + local_irq_save(flags); __acct_update_integrals(tsk, utime, stime); + local_irq_restore(flags); } /**
| |