Date: Tue, 4 Mar 2014 12:14:44 -0500 From: John Baldwin <jhb@freebsd.org> To: "George V. Neville-Neil" <gnn@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r262727 - head/sys/net Message-ID: <201403041214.44230.jhb@freebsd.org> In-Reply-To: <201403040509.s2459lou017310@svn.freebsd.org> References: <201403040509.s2459lou017310@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <vijju.singh at gmail.com> > 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201403041214.44230.jhb>