From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1756552AbcGHX64 (ORCPT <rfc822;w@1wt.eu>);
	Fri, 8 Jul 2016 19:58:56 -0400
Received: from mail-wm0-f45.google.com ([74.125.82.45]:35515 "EHLO
	mail-wm0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752967AbcGHX6u (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Fri, 8 Jul 2016 19:58:50 -0400
Date: Sat, 9 Jul 2016 01:58:47 +0200
From: Frederic Weisbecker <fweisbec@gmail.com>
To: Rik van Riel <riel@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>, linux-kernel@vger.kernel.org,
        peterz@infradead.org, mingo@kernel.org, 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: <20160708235846.GA3529@lerouge>
References: <1467315350-3152-1-git-send-email-riel@redhat.com>
 <1467315350-3152-5-git-send-email-riel@redhat.com>
 <20160708123010.GD30200@lerouge>
 <1467983987.13253.25.camel@redhat.com>
 <4738023a-adcf-69b5-5856-12c4dc619363@redhat.com>
 <1467993409.13253.31.camel@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <1467993409.13253.31.camel@redhat.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Fri, Jul 08, 2016 at 11:56:49AM -0400, Rik van Riel wrote:
> On Fri, 2016-07-08 at 16:34 +0200, Paolo Bonzini wrote:
> > I'm looking at an upstream tree, without your patches applied,
> > but it seems to me that irqtime_account_irq is always called with
> > interrupts disabled:
> > 
> > irqtime_account_irq
> > 	-> account_irq_enter_time
> > 		-> __irq_enter
> > 			-> HARDIRQ_ENTER	[1]
> > 			-> irq_enter		[3]
> > 		-> __do_softirq			[1]
> > 	-> account_irq_exit_time
> > 		-> __do_softirq			[1]
> > 		-> __irq_exit
> > 			-> HARDIRQ_EXIT		[1]
> > 		-> irq_exit			[2]
> > 
> > 	[1] = does local_irq_disable/enable
> > 	[2] = contains WARN_ON_ONCE(!irqs_disabled())
> > 	[3] = calls rcu_irq_enter(), which checks irqs_disabled()
> > 
> > I don't think your first two patches change this, so perhaps it's
> > enough
> > to remove that local_irq-save/restore?  Either this, or
> > ENEEDWEEKEND...

Good catch Paolo!

> 
> I think you are right!
> 
> __do_softirq() calls account_irq_enter_time() with irqs
> already disabled, and also has irqs disabled when it
> calls account_irq_exit_time()
> 
> This appears to be true for both ksoftirqd and softirq
> from irq context.

Indeed! And irq_enter()/irq_exit() have irqs disabled requirements.

The other users are __irq_enter() and __irq_exit() called by
lockdep selftests which takes care about it too.

I just did a boot test with a WARN_ON_ONCE(!irqs_disabled())
on account_irq_enter_time() and it triggered no issue.

> 
> This could simplify my patch series a lot :)

Definetly! ;-)

Would you mind resending it?

Thanks.