Date: Fri, 23 Dec 2011 12:13:44 -0500 From: Arnaud Lacombe <lacombar@gmail.com> To: John Baldwin <jhb@freebsd.org> Cc: Robert Watson <rwatson@freebsd.org>, net@freebsd.org Subject: Re: Transitioning if_addr_lock to an rwlock Message-ID: <CACqU3MW2OBH8UmfFqSq%2BJQBXcy3N82jwa7KcTSmHBqeiUypp_A@mail.gmail.com> In-Reply-To: <201112221130.01823.jhb@freebsd.org> References: <201112221130.01823.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi, On Thu, Dec 22, 2011 at 11:30 AM, John Baldwin <jhb@freebsd.org> wrote: > Another bit of lock contention I ran into between a device driver doing s= low > MAC filter updates and the receive path is IF_ADDR_LOCK(). =A0NIC device = drivers > typically hold this lock while iterating the list of multicast addresses = to > program their MAC filter. =A0OTOH, ip_input() uses this lock to check to = see if > an incoming packet is a broadcast packet or not. =A0So even with the pcbi= nfo > contention from my previous patch addressed, I still ran into a problem w= ith > IF_ADDR_LOCK(). =A0We already have some partial support for making this l= ock be > an rwlock (the APIs that drivers now use implies an rlock), so I finished= the > transition and checked various non-driver users to see which ones could u= se a > read lock (most uses can). =A0The current patch I have for this is on 8, = but if > folks think this is a good idea I can work on porting it to HEAD. =A0For = HEAD > the strategy I would use would be to split this up into 2 phases: > > 1) Add IF_ADDR locking macros to differentiate read locks vs write locks = along > =A0 with appropriate assertion macros. =A0Update current users of the loc= king > =A0 macros to use either read or write locks explicitly. =A0To preserve K= PI, > =A0 the existing LOCK/UNLOCK macros would map to write lock operations. = =A0In > =A0 the initial patch, the locking would still be implemented via a mtx. > > 2) Replace the mutex with an rwlock and update the locking macros as > =A0 appropriate. > out of curiosity, what do you expect from the conversion ? performance improvement ? latency improvement ? Does this particular lock show up in any significant way in lock profiling that make the change noticeable ? Thanks, - Arnaud > Phase 1 should definitely be MFC'able. =A0Phase 2 may or may not be. =A0R= obert had > the foresight to change drivers to use explicit function wrappers around > IF_ADDR_LOCK, and sizeof(struct mtx) =3D=3D sizeof(struct rwlock), so if = we > changed the lock type the KBI for existing device drivers would all be fi= ne. > Most of the remaining uses of the locking macros are in parts of the kern= el > that aren't loadable (such as inet and inet6). =A0We can look at the plac= es that > to do change and if any of them are in kld's then it would be up to re@ t= o > decide if 2) was actually safe to merge. =A0However, even if Phase 2 cann= ot be > MFC'd, having phase 1 makes it easier for downstream consumers to apply P= hase > 2 locally if they need it. > > You can find the patch for 8.x at > http://www.freebsd.org/~jhb/patches/if_addr_rwlock.patch > > -- > John Baldwin > _______________________________________________ > freebsd-net@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-net > To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CACqU3MW2OBH8UmfFqSq%2BJQBXcy3N82jwa7KcTSmHBqeiUypp_A>