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 S1753423AbbEGLxv (ORCPT <rfc822;w@1wt.eu>);
	Thu, 7 May 2015 07:53:51 -0400
Received: from mail-wi0-f177.google.com ([209.85.212.177]:32838 "EHLO
	mail-wi0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751061AbbEGLxq (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Thu, 7 May 2015 07:53:46 -0400
Date: Thu, 7 May 2015 13:53:43 +0200
From: Frederic Weisbecker <fweisbec@gmail.com>
To: Ingo Molnar <mingo@kernel.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
        "Rafael J . Wysocki" <rafael.j.wysocki@intel.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Mike Galbraith <umgwanakikbuti@gmail.com>,
        Chris Metcalf <cmetcalf@ezchip.com>, Dave Jones <davej@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>, Oleg Nesterov <oleg@redhat.com>,
        "Paul E . McKenney" <paulmck@linux.vnet.ibm.com>,
        Ingo Molnar <mingo@redhat.com>, Rik van Riel <riel@redhat.com>,
        Martin Schwidefsky <schwidefsky@de.ibm.com>
Subject: Re: [PATCH 1/4] context_tracking: Protect against recursion
Message-ID: <20150507115341.GA32271@lerouge>
References: <1430928266-24888-1-git-send-email-fweisbec@gmail.com>
 <1430928266-24888-2-git-send-email-fweisbec@gmail.com>
 <20150507095832.GA7976@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20150507095832.GA7976@gmail.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, May 07, 2015 at 11:58:32AM +0200, Ingo Molnar wrote:
> 
> * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> 
> > @@ -75,6 +94,11 @@ void context_tracking_enter(enum ctx_state state)
> >  	WARN_ON_ONCE(!current->mm);
> >  
> >  	local_irq_save(flags);
> > +	if (!context_tracking_recursion_enter()) {
> > +		local_irq_restore(flags);
> > +		return;
> > +	}
> > +
> >  	if ( __this_cpu_read(context_tracking.state) != state) {
> >  		if (__this_cpu_read(context_tracking.active)) {
> >  			/*
> > @@ -105,6 +129,7 @@ void context_tracking_enter(enum ctx_state state)
> >  		 */
> >  		__this_cpu_write(context_tracking.state, state);
> >  	}
> > +	context_tracking_recursion_exit();
> 
> >  	local_irq_restore(flags);
> >  }
> 
> So why not add an 'out_irq_restore:' label and use goto instead of 
> duplicating the return path in the recursion check?

Ah yeah. Sometimes people prefer that we just repeat the rollback code if
it's only one line. But I'm fine with the label as well.

> 
> >  NOKPROBE_SYMBOL(context_tracking_enter);
> > @@ -139,6 +164,10 @@ void context_tracking_exit(enum ctx_state state)
> >  		return;
> >  
> >  	local_irq_save(flags);
> > +	if (!context_tracking_recursion_enter()) {
> > +		local_irq_restore(flags);
> > +		return;
> 
> Ditto.
> 
> No need to resend, I fixed this up in the patch.
> 
> Thanks,
> 
> 	Ingo

Ah thanks a lot Ingo!