From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S936562AbcLOPOb (ORCPT ); Thu, 15 Dec 2016 10:14:31 -0500 Received: from mail-pf0-f193.google.com ([209.85.192.193]:35982 "EHLO mail-pf0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757719AbcLOPO3 (ORCPT ); Thu, 15 Dec 2016 10:14:29 -0500 Date: Thu, 15 Dec 2016 23:14:16 +0800 From: Boqun Feng To: Mark Rutland Cc: linux-kernel@vger.kernel.org, "Paul E . McKenney " , Josh Triplett , Steven Rostedt , Mathieu Desnoyers , Lai Jiangshan , Colin King Subject: Re: [RFC v2 1/5] rcu: Introduce for_each_leaf_node_cpu() Message-ID: <20161215151416.GO9728@tardis.cn.ibm.com> References: <20161215024204.28620-1-boqun.feng@gmail.com> <20161215024204.28620-2-boqun.feng@gmail.com> <20161215114351.GA21758@leverpostej> <20161215143858.GM9728@tardis.cn.ibm.com> <20161215151015.GG21758@leverpostej> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="p/1JFEOz/hVXxMAZ" Content-Disposition: inline In-Reply-To: <20161215151015.GG21758@leverpostej> User-Agent: Mutt/1.7.1 (2016-10-04) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --p/1JFEOz/hVXxMAZ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Dec 15, 2016 at 03:10:15PM +0000, Mark Rutland wrote: > On Thu, Dec 15, 2016 at 10:38:58PM +0800, Boqun Feng wrote: > > On Thu, Dec 15, 2016 at 11:43:52AM +0000, Mark Rutland wrote: > > > On Thu, Dec 15, 2016 at 10:42:00AM +0800, Boqun Feng wrote: > > > > +#define MASK_BITS(mask) (BITS_PER_BYTE * sizeof(mask)) > > > > +/* > > > > + * Iterate over all CPUs a leaf RCU node which are still masked in > > > > + * @mask. > > > > + * > > > > + * Note @rnp has to be a leaf node and @mask has to belong to @rnp. > > >=20 > > > Not a big deal, but perhaps it's worth enforcing this? If we took just > > > the name of the mask here, (e.g. qsmask rather than rnp->qsmask), we > > > could have the macro always use (rnp)->(mask). That would also make t= he > > > invocations shorter. > >=20 > > I thought about this approach, but there may be some cases it seems > > inappropriate, see patch #5, passing "qsmaskinitnext" directly to the > > for_each_leaf_node_cpu() might be OK, but it just break another > > abstraction layer which rcu_rnp_online_cpus() provides. >=20 > I had missed that. Given that, not enforcingi t makes sense to me. >=20 > > > > And we > > > > + * assume that no CPU is masked in @mask but not set in cpu_possib= le_mask. IOW, > > > > + * masks of a leaf node never set a bit for an "impossible" CPU. > > > > + */ > > > > +#define for_each_leaf_node_cpu(rnp, mask, cpu) \ > > > > + for ((cpu) =3D (rnp)->grplo + find_first_bit(&(mask), MASK_BITS(m= ask)); \ > > > > + (cpu) <=3D (rnp)->grphi && !WARN_ON_ONCE(!cpu_possible(cpu))= ; \ > > >=20 > > > If this happens, we'll exit the loop. If there are any reamining > > > possible CPUs, we'll skip them, which would be less than ideal. > > >=20 > > > I guess this shouldn't happen anyway, but it might be worth continuin= g. > > >=20 > >=20 > > I chose to break if we met impossible only because I wanted to avoid > > using that "if(...) else" trick in an iteration macro ;-) >=20 > Understandable. ;) >=20 > > I don't know whether this is the first time something like this is > > brought into kernel, so I'm kinda hesitating to bring this in. But seems > > I got you as one supporter ;-) > >=20 > > Certainly, skip is better than stop. >=20 > From a quick look around, I found at least a few instances of the pattern= =2E e.g. >=20 > include/linux/cpufreq.h: >=20 > #define cpufreq_for_each_valid_entry(pos, table) \ > for (pos =3D table; pos->frequency !=3D CPUFREQ_TABLE_END; pos++)= \ > if (pos->frequency =3D=3D CPUFREQ_ENTRY_INVALID) = \ > continue; \ > else >=20 > tools/perf/util/build-id.c: > #define dsos__for_each_with_build_id(pos, head) \ > list_for_each_entry(pos, head, node) \ > if (!pos->has_build_id) \ > continue; \ > else >=20 > Some drivers, like drivers/net/ethernet/broadcom/bnx2x/bnx2x.h really lov= e it! >=20 Thank you! So no reason we don't use it, let me send an updated patch ;-) Regards, Boqun > Thanks, > Mark. --p/1JFEOz/hVXxMAZ Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEcBAABCAAGBQJYUrNBAAoJEEl56MO1B/q4wAoH/j97tdy1VcXVXxqTtRJ0QOmT ItCFu1egMIRuHFVyTVKHFRE+HKUzywJRPxyJVKMVCJLYm7A3ZymNSfbN7JOUAWTq aGmk3qG4cQX+zhtWTWNOZgwYQmpAlRXGgYRicXOIJP1O910thptYOrYzIgaiq4Ve j11MMKNcioWIp3sXJConOCgJQTtH8fQbu03tVy66ZFK3VgESVoI+Lxry574q/mok Smc+zu+4f9Oj9vv2g338L5PV6BGcl0FlaUxhN7Qfe2yL2qkLQi9JyVxqGMR5YdrX lHjuARcoE+I4Vir1XCGCm5OmBqtea4g623LCATJ5n7Pt8lPnv7sEkJpPd7heKd4= =HMBc -----END PGP SIGNATURE----- --p/1JFEOz/hVXxMAZ--