Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756134AbaGIQYX (ORCPT ); Wed, 9 Jul 2014 12:24:23 -0400 Received: from mail-wg0-f51.google.com ([74.125.82.51]:58955 "EHLO mail-wg0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756016AbaGIQYU (ORCPT ); Wed, 9 Jul 2014 12:24:20 -0400 Date: Wed, 9 Jul 2014 18:24:16 +0200 From: Frederic Weisbecker To: Thomas Gleixner Cc: LKML , Russell King , Ingo Molnar , "H. Peter Anvin" , Peter Zijlstra , Venkatesh Pallipadi , "Paul E. McKenney" Subject: Re: [PATCH] sched: Sanitize irq accounting madness Message-ID: <20140709162414.GB23881@localhost.localdomain> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, May 02, 2014 at 11:26:24PM +0200, Thomas Gleixner wrote: > Russell reported, that irqtime_account_idle_ticks() takes ages due to: > > for (i = 0; i < ticks; i++) > irqtime_account_process_tick(current, 0, rq); > > It's sad, that this code was written way _AFTER_ the NOHZ idle > functionality was available. I charge myself guitly for not paying > attention when that crap got merged with commit abb74cefa (sched: > Export ns irqtimes through /proc/stat) > > So instead of looping nr_ticks times just apply the whole thing at > once. > > As a side note: The whole cputime_t vs. u64 business in that context > wants to be cleaned up as well. There is no point in having all these > back and forth conversions. Lets standardise on u64 nsec for all > kernel internal accounting and be done with it. Everything else does > not make sense at all for fine grained accounting. Frederic, can you > please take care of that? Oops I missed that. Yeah I can try something to that direction. Thanks. > > Reported-by: Russell King > Signed-off-by: Thomas Gleixner > Cc: stable@vger.kernel.org > --- > kernel/sched/cputime.c | 32 ++++++++++++++++---------------- > 1 file changed, 16 insertions(+), 16 deletions(-) > > Index: linux-2.6/kernel/sched/cputime.c > =================================================================== > --- linux-2.6.orig/kernel/sched/cputime.c > +++ linux-2.6/kernel/sched/cputime.c > @@ -332,50 +332,50 @@ out: > * softirq as those do not count in task exec_runtime any more. > */ > static void irqtime_account_process_tick(struct task_struct *p, int user_tick, > - struct rq *rq) > + struct rq *rq, int ticks) > { > - cputime_t one_jiffy_scaled = cputime_to_scaled(cputime_one_jiffy); > + cputime_t scaled = cputime_to_scaled(cputime_one_jiffy); > + u64 cputime = (__force u64) cputime_one_jiffy; > u64 *cpustat = kcpustat_this_cpu->cpustat; > > if (steal_account_process_tick()) > return; > > + cputime *= ticks; > + scaled *= ticks; > + > if (irqtime_account_hi_update()) { > - cpustat[CPUTIME_IRQ] += (__force u64) cputime_one_jiffy; > + cpustat[CPUTIME_IRQ] += cputime; > } else if (irqtime_account_si_update()) { > - cpustat[CPUTIME_SOFTIRQ] += (__force u64) cputime_one_jiffy; > + cpustat[CPUTIME_SOFTIRQ] += cputime; > } else if (this_cpu_ksoftirqd() == p) { > /* > * ksoftirqd time do not get accounted in cpu_softirq_time. > * So, we have to handle it separately here. > * Also, p->stime needs to be updated for ksoftirqd. > */ > - __account_system_time(p, cputime_one_jiffy, one_jiffy_scaled, > - CPUTIME_SOFTIRQ); > + __account_system_time(p, cputime, scaled, CPUTIME_SOFTIRQ); > } else if (user_tick) { > - account_user_time(p, cputime_one_jiffy, one_jiffy_scaled); > + account_user_time(p, cputime, scaled); > } else if (p == rq->idle) { > - account_idle_time(cputime_one_jiffy); > + account_idle_time(cputime); > } else if (p->flags & PF_VCPU) { /* System time or guest time */ > - account_guest_time(p, cputime_one_jiffy, one_jiffy_scaled); > + account_guest_time(p, cputime, scaled); > } else { > - __account_system_time(p, cputime_one_jiffy, one_jiffy_scaled, > - CPUTIME_SYSTEM); > + __account_system_time(p, cputime, scaled, CPUTIME_SYSTEM); > } > } > > static void irqtime_account_idle_ticks(int ticks) > { > - int i; > struct rq *rq = this_rq(); > > - for (i = 0; i < ticks; i++) > - irqtime_account_process_tick(current, 0, rq); > + irqtime_account_process_tick(current, 0, rq, ticks); > } > #else /* CONFIG_IRQ_TIME_ACCOUNTING */ > static inline void irqtime_account_idle_ticks(int ticks) {} > static inline void irqtime_account_process_tick(struct task_struct *p, int user_tick, > - struct rq *rq) {} > + struct rq *rq, int nr_ticks) {} > #endif /* CONFIG_IRQ_TIME_ACCOUNTING */ > > /* > @@ -464,7 +464,7 @@ void account_process_tick(struct task_st > return; > > if (sched_clock_irqtime) { > - irqtime_account_process_tick(p, user_tick, rq); > + irqtime_account_process_tick(p, user_tick, rq, 1); > return; > } > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/