From owner-freebsd-net@FreeBSD.ORG Thu Dec 22 16:30:03 2011 Return-Path: Delivered-To: net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 4512E106566C; Thu, 22 Dec 2011 16:30:03 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 192EC8FC08; Thu, 22 Dec 2011 16:30:03 +0000 (UTC) Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [96.47.65.170]) by cyrus.watson.org (Postfix) with ESMTPSA id BD4EF46B0A; Thu, 22 Dec 2011 11:30:02 -0500 (EST) Received: from jhbbsd.localnet (unknown [209.249.190.124]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 49B3FB914; Thu, 22 Dec 2011 11:30:02 -0500 (EST) From: John Baldwin To: net@freebsd.org Date: Thu, 22 Dec 2011 11:30:01 -0500 User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p8; KDE/4.5.5; amd64; ; ) MIME-Version: 1.0 Content-Type: Text/Plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Message-Id: <201112221130.01823.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Thu, 22 Dec 2011 11:30:02 -0500 (EST) Cc: Robert Watson Subject: Transitioning if_addr_lock to an rwlock X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Dec 2011 16:30:03 -0000 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