From owner-freebsd-net@FreeBSD.ORG Fri Dec 23 17:42:41 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 DB2B11065688; Fri, 23 Dec 2011 17:42:41 +0000 (UTC) (envelope-from lacombar@gmail.com) Received: from mail-wi0-f182.google.com (mail-wi0-f182.google.com [209.85.212.182]) by mx1.freebsd.org (Postfix) with ESMTP id 412868FC23; Fri, 23 Dec 2011 17:42:41 +0000 (UTC) Received: by wibhr1 with SMTP id hr1so6586872wib.13 for ; Fri, 23 Dec 2011 09:42:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; bh=/ciziAOXfDTJtHhmLIOcLgYcnJeB6/w7RklKdx4rjy8=; b=NwTASkjKBiBktPVmr1RKjou+G+KZtdKwFVRGfN0OxSxl6wEJEhTjchH+k32FQxeGbf PymdeLOI0cuM7ctlRJRaikHbYu5hOuBVzMh9TFB6T7+2D1U2hUx5fRNEhWaJA8ETLDvx Hs+Mzhdb4FMBnBeo8Uuhmkgo2sNixDqQpaSms= MIME-Version: 1.0 Received: by 10.180.107.134 with SMTP id hc6mr31575617wib.21.1324660424775; Fri, 23 Dec 2011 09:13:44 -0800 (PST) Received: by 10.216.178.204 with HTTP; Fri, 23 Dec 2011 09:13:44 -0800 (PST) In-Reply-To: <201112221130.01823.jhb@freebsd.org> References: <201112221130.01823.jhb@freebsd.org> Date: Fri, 23 Dec 2011 12:13:44 -0500 Message-ID: From: Arnaud Lacombe To: John Baldwin Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: Robert Watson , net@freebsd.org Subject: Re: 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: Fri, 23 Dec 2011 17:42:41 -0000 Hi, On Thu, Dec 22, 2011 at 11:30 AM, John Baldwin 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"