From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757108AbcA3QYH (ORCPT ); Sat, 30 Jan 2016 11:24:07 -0500 Received: from mail-wm0-f66.google.com ([74.125.82.66]:35056 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756917AbcA3QYF (ORCPT ); Sat, 30 Jan 2016 11:24:05 -0500 Date: Sat, 30 Jan 2016 17:24:01 +0100 From: Frederic Weisbecker To: riel@redhat.com Cc: linux-kernel@vger.kernel.org, tglx@linutronix.de, mingo@kernel.org, luto@amacapital.net, peterz@infradead.org, clark@redhat.com Subject: Re: [PATCH 3/4] time,acct: drop irq save & restore from __acct_update_integrals Message-ID: <20160130162400.GD32581@lerouge> References: <1454124965-13974-1-git-send-email-riel@redhat.com> <1454124965-13974-4-git-send-email-riel@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1454124965-13974-4-git-send-email-riel@redhat.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jan 29, 2016 at 10:36:04PM -0500, riel@redhat.com wrote: > From: Rik van Riel > > 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 > Signed-off-by: Rik van Riel > --- > 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); } /**