From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755181AbcGHMaW (ORCPT ); Fri, 8 Jul 2016 08:30:22 -0400 Received: from mail-lf0-f65.google.com ([209.85.215.65]:35657 "EHLO mail-lf0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754972AbcGHMaQ (ORCPT ); Fri, 8 Jul 2016 08:30:16 -0400 Date: Fri, 8 Jul 2016 14:30:11 +0200 From: Frederic Weisbecker To: riel@redhat.com Cc: linux-kernel@vger.kernel.org, peterz@infradead.org, mingo@kernel.org, pbonzini@redhat.com, fweisbec@redhat.com, wanpeng.li@hotmail.com, efault@gmx.de, tglx@linutronix.de, rkrcmar@redhat.com Subject: Re: [PATCH 4/4] irqtime: drop local_irq_save/restore from irqtime_account_irq Message-ID: <20160708123010.GD30200@lerouge> References: <1467315350-3152-1-git-send-email-riel@redhat.com> <1467315350-3152-5-git-send-email-riel@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1467315350-3152-5-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 Thu, Jun 30, 2016 at 03:35:50PM -0400, riel@redhat.com wrote: > From: Rik van Riel > > Drop local_irq_save/restore from irqtime_account_irq. > Instead, have softirq and hardirq track their time spent > independently, with the softirq code subtracting hardirq > time that happened during the duration of the softirq run. > > The softirq code can be interrupted by hardirq code at > any point in time, but it can check whether it got a > consistent snapshot of the timekeeping variables it wants, > and loop around in the unlikely case that it did not. > > Signed-off-by: Rik van Riel So the purpose is to get rid of local_irq_save/restore()? Is it really worth such complication? > --- > kernel/sched/cputime.c | 72 +++++++++++++++++++++++++++++++++++++++++--------- > kernel/sched/sched.h | 38 +++++++++++++++++++++----- > 2 files changed, 90 insertions(+), 20 deletions(-) > > diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c > index a0aefd4c7ea6..b78991fac228 100644 > --- a/kernel/sched/cputime.c > +++ b/kernel/sched/cputime.c > @@ -26,7 +26,9 @@ > DEFINE_PER_CPU(u64, cpu_hardirq_time); > DEFINE_PER_CPU(u64, cpu_softirq_time); > > -static DEFINE_PER_CPU(u64, irq_start_time); > +static DEFINE_PER_CPU(u64, hardirq_start_time); > +static DEFINE_PER_CPU(u64, softirq_start_time); > +static DEFINE_PER_CPU(u64, prev_hardirq_time); > static int sched_clock_irqtime; > > void enable_sched_clock_irqtime(void) > @@ -41,6 +43,7 @@ void disable_sched_clock_irqtime(void) > > #ifndef CONFIG_64BIT > DEFINE_PER_CPU(seqcount_t, irq_time_seq); > +DEFINE_PER_CPU(seqcount_t, softirq_time_seq); > #endif /* CONFIG_64BIT */ > > /* > @@ -53,36 +56,79 @@ DEFINE_PER_CPU(seqcount_t, irq_time_seq); > * softirq -> hardirq, hardirq -> softirq > * > * When exiting hardirq or softirq time, account the elapsed time. > + * > + * When exiting softirq time, subtract the amount of hardirq time that > + * interrupted this softirq run, to avoid double accounting of that time. > */ > void irqtime_account_irq(struct task_struct *curr, int irqtype) > { > - unsigned long flags; > + u64 prev_softirq_start; > + bool leaving_softirq; > + u64 prev_hardirq; > + u64 hardirq_time; > s64 delta; > int cpu; > > if (!sched_clock_irqtime) > return; > > - local_irq_save(flags); > - > cpu = smp_processor_id(); > - delta = sched_clock_cpu(cpu) - __this_cpu_read(irq_start_time); > - __this_cpu_add(irq_start_time, delta); > > - irq_time_write_begin(); > + /* > + * Hardirq time accounting is pretty straightforward. If not in > + * hardirq context yet (entering hardirq), set the start time. > + * If already in hardirq context (leaving), account the elapsed time. > + */ > + if (irqtype == HARDIRQ_OFFSET) { > + bool leaving_hardirq = hardirq_count(); > + delta = sched_clock_cpu(cpu) - __this_cpu_read(hardirq_start_time); > + __this_cpu_add(hardirq_start_time, delta); > + if (leaving_hardirq) { > + hardirq_time_write_begin(); > + __this_cpu_add(cpu_hardirq_time, delta); > + hardirq_time_write_end(); > + } This doesn't seem to work with nesting hardirqs. Thanks.