From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754837AbcCXBDg (ORCPT ); Wed, 23 Mar 2016 21:03:36 -0400 Received: from mail-ig0-f180.google.com ([209.85.213.180]:35795 "EHLO mail-ig0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753243AbcCXBDf (ORCPT ); Wed, 23 Mar 2016 21:03:35 -0400 Date: Thu, 24 Mar 2016 09:02:48 +0800 From: Boqun Feng To: "Paul E. McKenney" Cc: linux-kernel@vger.kernel.org, Josh Triplett , Steven Rostedt , Mathieu Desnoyers , Lai Jiangshan , Frederic Weisbecker Subject: Re: [PATCH] rcu: Remove superfluous versions of rcu_read_lock_sched_held() Message-ID: <20160324010248.GE2203@fixme-laptop.cn.ibm.com> References: <1458745908-30431-1-git-send-email-boqun.feng@gmail.com> <20160323163846.GD4287@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="hUH5gZbnpyIv7Mn4" Content-Disposition: inline In-Reply-To: <20160323163846.GD4287@linux.vnet.ibm.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 --hUH5gZbnpyIv7Mn4 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Mar 23, 2016 at 09:38:46AM -0700, Paul E. McKenney wrote: > On Wed, Mar 23, 2016 at 11:11:48PM +0800, Boqun Feng wrote: > > Currently, we have four versions of rcu_read_lock_sched_held(), > > depending on the combined choices on PREEMPT_COUNT and DEBUG_LOCK_ALLOC. > > But we actually don't need to specialize those for PREEMPT_COUNT=3Dn > > kernel. Because: > >=20 > > 1. For the implementations in DEBUG_LOCK_ALLOC=3Dn kernel, we can use > > preemptible() to implement one rcu_read_lock_sched_held(), which > > gives us the same behavior as the current two. > >=20 > > 2. For the implementations in DEBUG_LOCK_ALLOC=3Dy kernel, even when > > PREEMPT_COUNT=3Dn, one CPU may block no grace period because of > > the same reason for the PREEMPT_COUNT=3Dy and !PREEMPT kernel. > > (e.g. dynticks or cpu hotplug) > >=20 > > So unify the implementations of rcu_read_lock_sched_held() by using > > macro preemptible() and tightening up the lock-held checking for > > PREEMPT_COUNT=3Dn kernel. And this will improve the readability, make t= he > > debug checking as expected and save several lines of code. > >=20 > > Signed-off-by: Boqun Feng >=20 > Looks like a nice consolidation! I have queued this for review and > testing, updating the commit log as follows: >=20 > Currently, we have four versions of rcu_read_lock_sched_held(), > depending on the combined choices on PREEMPT_COUNT and > DEBUG_LOCK_ALLOC. However, there is an existing function > preemptible() that already distinguishes between the > PREEMPT_COUNT=3Dy and PREEMPT_COUNT=3Dn cases, and allows these four > implementations to be consolidated down to two. > =20 > This commit therefore uses preemptible() to achieve this > consolidation. Note that there could be a small performance > regression in the case of CONFIG_DEBUG_LOCK_ALLOC=3Dy && > PREEMPT_COUNT=3Dn. However, given the overhead associated with > CONFIG_DEBUG_LOCK_ALLOC=3Dy, this should be down in the noise. >=20 > Does that capture it? >=20 Yes, thank you ;-) Regards, Boqun > Thanx, Paul >=20 > > --- > > include/linux/rcupdate.h | 17 +---------------- > > kernel/rcu/update.c | 4 ++-- > > 2 files changed, 3 insertions(+), 18 deletions(-) > >=20 > > diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h > > index b5d48bd..e3f845b 100644 > > --- a/include/linux/rcupdate.h > > +++ b/include/linux/rcupdate.h > > @@ -510,14 +510,7 @@ int rcu_read_lock_bh_held(void); > > * CONFIG_DEBUG_LOCK_ALLOC, this assumes we are in an RCU-sched read-s= ide > > * critical section unless it can prove otherwise. > > */ > > -#ifdef CONFIG_PREEMPT_COUNT > > int rcu_read_lock_sched_held(void); > > -#else /* #ifdef CONFIG_PREEMPT_COUNT */ > > -static inline int rcu_read_lock_sched_held(void) > > -{ > > - return 1; > > -} > > -#endif /* #else #ifdef CONFIG_PREEMPT_COUNT */ > >=20 > > #else /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */ > >=20 > > @@ -534,18 +527,10 @@ static inline int rcu_read_lock_bh_held(void) > > return 1; > > } > >=20 > > -#ifdef CONFIG_PREEMPT_COUNT > > static inline int rcu_read_lock_sched_held(void) > > { > > - return preempt_count() !=3D 0 || irqs_disabled(); > > + return !preemptible(); > > } > > -#else /* #ifdef CONFIG_PREEMPT_COUNT */ > > -static inline int rcu_read_lock_sched_held(void) > > -{ > > - return 1; > > -} > > -#endif /* #else #ifdef CONFIG_PREEMPT_COUNT */ > > - > > #endif /* #else #ifdef CONFIG_DEBUG_LOCK_ALLOC */ > >=20 > > #ifdef CONFIG_PROVE_RCU > > diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c > > index ca828b4..3ccdc8e 100644 > > --- a/kernel/rcu/update.c > > +++ b/kernel/rcu/update.c > > @@ -67,7 +67,7 @@ static int rcu_normal_after_boot; > > module_param(rcu_normal_after_boot, int, 0); > > #endif /* #ifndef CONFIG_TINY_RCU */ > >=20 > > -#if defined(CONFIG_DEBUG_LOCK_ALLOC) && defined(CONFIG_PREEMPT_COUNT) > > +#ifdef CONFIG_DEBUG_LOCK_ALLOC > > /** > > * rcu_read_lock_sched_held() - might we be in RCU-sched read-side cri= tical section? > > * > > @@ -111,7 +111,7 @@ int rcu_read_lock_sched_held(void) > > return 0; > > if (debug_locks) > > lockdep_opinion =3D lock_is_held(&rcu_sched_lock_map); > > - return lockdep_opinion || preempt_count() !=3D 0 || irqs_disabled(); > > + return lockdep_opinion || !preemptible(); > > } > > EXPORT_SYMBOL(rcu_read_lock_sched_held); > > #endif > > --=20 > > 2.7.3 > >=20 >=20 --hUH5gZbnpyIv7Mn4 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAABCAAGBQJW8zyzAAoJEEl56MO1B/q4ZesH/jVpqp74vVYesVVFq6QQXOlG rHJTSrFP1DU6huujvZVt0Zu/wyyUEyzIO9cTBcR5F7gwburBlQuRM0udMHh5R12x OTsEEWrkP3TVZbRlNHy3NEw8+qq4JMOS3srjTxaNE1JdFTqBly3VpndFLX15UImp SUwRq5BqaW30uH8RzTX8NU5PAF5rRt4QlOuIDl0wOgqEuYYYXApCcj2XtdavllTA 1ayHX+FrDarDjoMBL2xhnua8ubS02gvuIRI8KE5NMMVa6iGjYN6JaN4aXI1BO1kb hxf9QoqOibbMHogVQODEU4qAG9kmLxuopvQEieNLrAvCOXRMILT7VTvSBZpncno= =dvU9 -----END PGP SIGNATURE----- --hUH5gZbnpyIv7Mn4--