From patchwork Mon Sep 19 05:49:33 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Yong Zhang X-Patchwork-Id: 4168 Return-Path: X-Original-To: patchwork@peony.canonical.com Delivered-To: patchwork@peony.canonical.com Received: from fiordland.canonical.com (fiordland.canonical.com [91.189.94.145]) by peony.canonical.com (Postfix) with ESMTP id 43A7823EFD for ; Mon, 19 Sep 2011 05:49:36 +0000 (UTC) Received: from mail-fx0-f52.google.com (mail-fx0-f52.google.com [209.85.161.52]) by fiordland.canonical.com (Postfix) with ESMTP id 2A3D0A1848E for ; Mon, 19 Sep 2011 05:49:36 +0000 (UTC) Received: by fxe23 with SMTP id 23so5049973fxe.11 for ; Sun, 18 Sep 2011 22:49:36 -0700 (PDT) Received: by 10.223.74.89 with SMTP id t25mr4529838faj.65.1316411375899; Sun, 18 Sep 2011 22:49:35 -0700 (PDT) X-Forwarded-To: linaro-patchwork@canonical.com X-Forwarded-For: patch@linaro.org linaro-patchwork@canonical.com Delivered-To: patches@linaro.org Received: by 10.152.18.198 with SMTP id y6cs23932lad; Sun, 18 Sep 2011 22:49:35 -0700 (PDT) Received: by 10.216.186.205 with SMTP id w55mr2364032wem.102.1316411374880; Sun, 18 Sep 2011 22:49:34 -0700 (PDT) Received: from mail-wy0-f178.google.com (mail-wy0-f178.google.com [74.125.82.178]) by mx.google.com with ESMTPS id k4si17882618wed.128.2011.09.18.22.49.33 (version=TLSv1/SSLv3 cipher=OTHER); Sun, 18 Sep 2011 22:49:34 -0700 (PDT) Received-SPF: pass (google.com: domain of yong.zhang0@gmail.com designates 74.125.82.178 as permitted sender) client-ip=74.125.82.178; Authentication-Results: mx.google.com; spf=pass (google.com: domain of yong.zhang0@gmail.com designates 74.125.82.178 as permitted sender) smtp.mail=yong.zhang0@gmail.com; dkim=pass (test mode) header.i=@gmail.com Received: by wyf23 with SMTP id 23so6097031wyf.37 for ; Sun, 18 Sep 2011 22:49:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=hLNN0xdCcI0gyqOzVolMYQJb/q1ZH5Vr4sI8CM0bnd0=; b=tkmSUdfonK6LyoiI0r+ejHF7ReCXv8gnGVjZv//gz4FKwTh3hFj/ID+tUsdaFV445u EMSBrj+pYijrPraXs2yrELiixnz2L7BZF6q1pjuW5DwKUxZgKheyAVbyh+wikRK/LwWS eiO11J3EgeY1zlV/2TmFf13nqOF3yHBzrGEgo= MIME-Version: 1.0 Received: by 10.216.168.81 with SMTP id j59mr806470wel.7.1316411373486; Sun, 18 Sep 2011 22:49:33 -0700 (PDT) Received: by 10.216.187.137 with HTTP; Sun, 18 Sep 2011 22:49:33 -0700 (PDT) In-Reply-To: <20110919041459.GJ2333@linux.vnet.ibm.com> References: <20110906180015.GA2560@linux.vnet.ibm.com> <1315332049-2604-41-git-send-email-paulmck@linux.vnet.ibm.com> <20110918040923.GA2363@zhy> <20110919041459.GJ2333@linux.vnet.ibm.com> Date: Mon, 19 Sep 2011 13:49:33 +0800 Message-ID: Subject: Re: [PATCH tip/core/rcu 41/55] rcu: Permit rt_mutex_unlock() with irqs disabled From: Yong Zhang To: paulmck@linux.vnet.ibm.com Cc: linux-kernel@vger.kernel.org, mingo@elte.hu, laijs@cn.fujitsu.com, dipankar@in.ibm.com, akpm@linux-foundation.org, mathieu.desnoyers@polymtl.ca, josh@joshtriplett.org, niv@us.ibm.com, tglx@linutronix.de, peterz@infradead.org, rostedt@goodmis.org, Valdis.Kletnieks@vt.edu, dhowells@redhat.com, eric.dumazet@gmail.com, darren@dvhart.com, patches@linaro.org, "Paul E. McKenney" On Mon, Sep 19, 2011 at 12:14 PM, Paul E. McKenney wrote: > On Sun, Sep 18, 2011 at 12:09:23PM +0800, Yong Zhang wrote: >> > diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h >> > index d3127e8..f6c63ea 100644 >> > --- a/kernel/rcutree_plugin.h >> > +++ b/kernel/rcutree_plugin.h >> > @@ -1149,6 +1149,8 @@ static void rcu_initiate_boost_trace(struct rcu_node *rnp) >> > >> >  #endif /* #else #ifdef CONFIG_RCU_TRACE */ >> > >> > +static struct lock_class_key rcu_boost_class; >> > + >> >  /* >> >   * Carry out RCU priority boosting on the task indicated by ->exp_tasks >> >   * or ->boost_tasks, advancing the pointer to the next task in the >> > @@ -1211,10 +1213,14 @@ static int rcu_boost(struct rcu_node *rnp) >> >      */ >> >     t = container_of(tb, struct task_struct, rcu_node_entry); >> >     rt_mutex_init_proxy_locked(&mtx, t); >> > +   /* Avoid lockdep false positives.  This rt_mutex is its own thing. */ >> > +   lockdep_set_class_and_name(&mtx.wait_lock, &rcu_boost_class, >> > +                              "rcu_boost_mutex"); >> >     t->rcu_boost_mutex = &mtx; >> >>       raw_spin_unlock_irqrestore(&rnp->lock, flags);  <====A >> >> >     rt_mutex_lock(&mtx);  /* Side effect: boosts task t's priority. */ >> >     rt_mutex_unlock(&mtx);  /* Keep lockdep happy. */ >> > +   local_irq_restore(flags); >> >> Does it help here? >> irq is enabled at A. So we still call rt_mutex_lock() with irq enabled. >> >> Seems should s/raw_spin_unlock_irqrestore/raw_spin_unlock ? > > Hmmm...  The above works at least by accident, but I am clearly not > testing calling rt_mutex_lock(&mtx) and rt_mutex_unlock(&mtx) with > interrupts disabled anywhere near as heavily as I thought I was. > > I will fix this one way or the other. Forget to mention: if we want to suppress the lockdep warning on overlapping usage of rcu_read_*()/local_irq_*() like below: rcu_read_lock(); ... local_irq_disable(); ... rcu_read_unlock(); ... local_irq_enable(); 'rt_mutex_unlock(rbmp);' must also be surrounded by local_irq_irqsave()/restore(). Untested patch is attached. Thanks, Yong --- From: Yong Zhang Subject: [PATCH] rcu: Permit rt_mutex_unlock() with irqs disabled take#2 This make the below rcu usage really valid(AKA: lockdep will not warn on it): rcu_read_lock(); local_irq_disable(); rcu_read_unlock(); local_irq_enable(); Signed-off-by: Yong Zhang --- kernel/rcutree_plugin.h | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-) /* @@ -1225,7 +1228,7 @@ static int rcu_boost(struct rcu_node *rnp) lockdep_set_class_and_name(&mtx.wait_lock, &rcu_boost_class, "rcu_boost_mutex"); t->rcu_boost_mutex = &mtx; - raw_spin_unlock_irqrestore(&rnp->lock, flags); + raw_spin_unlock(&rnp->lock); rt_mutex_lock(&mtx); /* Side effect: boosts task t's priority. */ rt_mutex_unlock(&mtx); /* Keep lockdep happy. */ local_irq_restore(flags); >From 7d74d1b89a4cd4c03b30e47044b716913f68bd1d Mon Sep 17 00:00:00 2001 From: Yong Zhang Date: Mon, 19 Sep 2011 13:42:32 +0800 Subject: [PATCH] rcu: Permit rt_mutex_unlock() with irqs disabled take#2 This make the below rcu usage really valid(AKA: lockdep will not warn on it): rcu_read_lock(); local_irq_disable(); rcu_read_unlock(); local_irq_enable(); Signed-off-by: Yong Zhang --- kernel/rcutree_plugin.h | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-) diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h index e7eea74..d41a9b0 100644 --- a/kernel/rcutree_plugin.h +++ b/kernel/rcutree_plugin.h @@ -398,8 +398,11 @@ static noinline void rcu_read_unlock_special(struct task_struct *t) #ifdef CONFIG_RCU_BOOST /* Unboost if we were boosted. */ - if (rbmp) + if (rbmp) { + local_irq_save(flags); rt_mutex_unlock(rbmp); + local_irq_restore(flags); + } #endif /* #ifdef CONFIG_RCU_BOOST */ /* @@ -1225,7 +1228,7 @@ static int rcu_boost(struct rcu_node *rnp) lockdep_set_class_and_name(&mtx.wait_lock, &rcu_boost_class, "rcu_boost_mutex"); t->rcu_boost_mutex = &mtx; - raw_spin_unlock_irqrestore(&rnp->lock, flags); + raw_spin_unlock(&rnp->lock); rt_mutex_lock(&mtx); /* Side effect: boosts task t's priority. */ rt_mutex_unlock(&mtx); /* Keep lockdep happy. */ local_irq_restore(flags); -- 1.7.4.1