Date: Thu, 22 Dec 2011 11:30:01 -0500 From: John Baldwin <jhb@freebsd.org> To: net@freebsd.org Cc: Robert Watson <rwatson@freebsd.org> Subject: Transitioning if_addr_lock to an rwlock Message-ID: <201112221130.01823.jhb@freebsd.org>
next in thread | raw e-mail | index | archive | help
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. 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201112221130.01823.jhb>