From owner-svn-src-all@FreeBSD.ORG Tue Mar 4 18:38:31 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 778A078B; Tue, 4 Mar 2014 18:38:31 +0000 (UTC) Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) (using TLSv1 with cipher ADH-CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 4FC518A6; Tue, 4 Mar 2014 18:38:31 +0000 (UTC) Received: from jhbbsd.localnet (unknown [209.249.190.124]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 21C6DB95B; Tue, 4 Mar 2014 13:38:30 -0500 (EST) From: John Baldwin To: "George V. Neville-Neil" Subject: Re: svn commit: r262727 - head/sys/net Date: Tue, 4 Mar 2014 12:14:44 -0500 User-Agent: KMail/1.13.5 (FreeBSD/8.4-CBSD-20130906; KDE/4.5.5; amd64; ; ) References: <201403040509.s2459lou017310@svn.freebsd.org> In-Reply-To: <201403040509.s2459lou017310@svn.freebsd.org> MIME-Version: 1.0 Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <201403041214.44230.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Tue, 04 Mar 2014 13:38:30 -0500 (EST) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 04 Mar 2014 18:38:31 -0000 On Tuesday, March 04, 2014 12:09:47 am George V. Neville-Neil wrote: > Author: gnn > Date: Tue Mar 4 05:09:46 2014 > New Revision: 262727 > URL: http://svnweb.freebsd.org/changeset/base/262727 > > Log: > Naming consistency fix. The routing code defines > RADIX_NODE_HEAD_LOCK as grabbing the write lock, > but RADIX_NODE_HEAD_LOCK_ASSERT as checking the read lock. Actually, that isn't what RA_LOCKED means. RA_LOCKED means that it is either read- or write-locked. Note that you have now made RADIX_NODE_HEAD_LOCK_ASSERT() a redundant copy of RADIX_NODE_HEAD_WLOCK_ASSERT(). You should revert that part in some way (either remove HEAD_LOCK_ASSERT() entirely leaving just RLOCK_ASSERT() and WLOCK_ASSERT(), or restore HEAD_LOCK_ASSERT() to using RA_LOCKED if there are places that want to assert that the lock is held, but don't care if it is read or write). > Submitted by: Vijay Singh > MFC after: 1 month > > Modified: > head/sys/net/radix.h > head/sys/net/route.c > > Modified: head/sys/net/radix.h > ============================================================================== > --- head/sys/net/radix.h Tue Mar 4 03:19:36 2014 (r262726) > +++ head/sys/net/radix.h Tue Mar 4 05:09:46 2014 (r262727) > @@ -149,7 +149,8 @@ struct radix_node_head { > > > #define RADIX_NODE_HEAD_DESTROY(rnh) rw_destroy(&(rnh)->rnh_lock) > -#define RADIX_NODE_HEAD_LOCK_ASSERT(rnh) rw_assert(&(rnh)->rnh_lock, RA_LOCKED) > +#define RADIX_NODE_HEAD_LOCK_ASSERT(rnh) rw_assert(&(rnh)->rnh_lock, RA_WLOCKED) > +#define RADIX_NODE_HEAD_RLOCK_ASSERT(rnh) rw_assert(&(rnh)->rnh_lock, RA_RLOCKED) > #define RADIX_NODE_HEAD_WLOCK_ASSERT(rnh) rw_assert(&(rnh)->rnh_lock, RA_WLOCKED) > #endif /* _KERNEL */ > > > Modified: head/sys/net/route.c > ============================================================================== > --- head/sys/net/route.c Tue Mar 4 03:19:36 2014 (r262726) > +++ head/sys/net/route.c Tue Mar 4 05:09:46 2014 (r262727) > @@ -381,7 +381,7 @@ rtalloc1_fib(struct sockaddr *dst, int r > RADIX_NODE_HEAD_RLOCK(rnh); > #ifdef INVARIANTS > else > - RADIX_NODE_HEAD_LOCK_ASSERT(rnh); > + RADIX_NODE_HEAD_RLOCK_ASSERT(rnh); > #endif This was fine before if it is ok for a caller to hold a write lock when calling this function. If that is not allowed, then your change here is correct. > rn = rnh->rnh_matchaddr(dst, rnh); > if (rn && ((rn->rn_flags & RNF_ROOT) == 0)) { > @@ -1000,9 +1000,10 @@ rn_mpath_update(int req, struct rt_addri > * a matching RTAX_GATEWAY. > */ > struct rtentry *rt, *rto = NULL; > - register struct radix_node *rn; > + struct radix_node *rn; > int error = 0; > > + RADIX_NODE_HEAD_LOCK_ASSERT(rnh); If a write lock is required, please use WLOCK_ASSERT() instead. > rn = rnh->rnh_lookup(dst, netmask, rnh); > if (rn == NULL) > return (ESRCH); > -- John Baldwin