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 slow > MAC filter updates and the receive path is IF_ADDR_LOCK(). NIC device drivers > typically hold this lock while iterating the list of multicast addresses to > program their MAC filter. OTOH, ip_input() uses this lock to check to see if > an incoming packet is a broadcast packet or not. So even with the pcbinfo > contention from my previous patch addressed, I still ran into a problem with > IF_ADDR_LOCK(). We already have some partial support for making this lock 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 use a > read lock (most uses can). The 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. For 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 > with appropriate assertion macros. Update current users of the locking > macros to use either read or write locks explicitly. To preserve KPI, > the existing LOCK/UNLOCK macros would map to write lock operations. In > 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 > 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. Phase 2 may or may not be. Robert had > the foresight to change drivers to use explicit function wrappers around > IF_ADDR_LOCK, and sizeof(struct mtx) == sizeof(struct rwlock), so if we > changed the lock type the KBI for existing device drivers would all be fine. > Most of the remaining uses of the locking macros are in parts of the kernel > that aren't loadable (such as inet and inet6). We can look at the places that > to do change and if any of them are in kld's then it would be up to re@ to > decide if 2) was actually safe to merge. However, even if Phase 2 cannot be > MFC'd, having phase 1 makes it easier for downstream consumers to apply Phase > 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>