From owner-svn-src-all@FreeBSD.ORG Wed Jan 22 21:23:18 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 779B4180; Wed, 22 Jan 2014 21:23:18 +0000 (UTC) Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) (using TLSv1 with cipher ADH-CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 4A4251551; Wed, 22 Jan 2014 21:23:18 +0000 (UTC) Received: from jhbbsd.localnet (unknown [209.249.190.124]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 4AF53B948; Wed, 22 Jan 2014 16:23:17 -0500 (EST) From: John Baldwin To: Alfred Perlstein Subject: Re: svn commit: r260898 - head/sys/kern Date: Wed, 22 Jan 2014 16:22:42 -0500 User-Agent: KMail/1.13.5 (FreeBSD/8.4-CBSD-20130906; KDE/4.5.5; amd64; ; ) References: <201401200159.s0K1xa5X012123@svn.freebsd.org> <201401221527.12779.jhb@freebsd.org> <52E03139.2020902@freebsd.org> In-Reply-To: <52E03139.2020902@freebsd.org> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201401221622.42789.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Wed, 22 Jan 2014 16:23:17 -0500 (EST) Cc: src-committers@freebsd.org, Scott Long , Neel Natu , John-Mark Gurney , svn-src-all@freebsd.org, Rui Paulo , svn-src-head@freebsd.org, Alexander Kabaev X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 22 Jan 2014 21:23:18 -0000 On Wednesday, January 22, 2014 3:59:37 pm Alfred Perlstein wrote: > > On 1/22/14, 12:27 PM, John Baldwin wrote: > > On Wednesday, January 22, 2014 2:06:39 pm Alfred Perlstein wrote: > >> Hmm, what if locks had a pointer to a 2 element char * array, the first > >> being the name, the second the type. That would keep the size of the > >> lock down and most locks could share a common tuple of name/type in each > >> subsystem. This would allow us to get rid of the pending static list. > >> > >> effectively: > >> struct lock_object { > >> char *lo_name; /* Individual lock name. */ > >> u_int lo_flags; > >> u_int lo_data; /* General class specific data. */ > >> struct witness *lo_witness; /* Data for witness. */ > >> }; > >> > >> would change to: > >> struct lock_object { > >> char **lo_name_type; /* Individual lock > >> name[0]/type[1]. */ > >> u_int lo_flags; > >> u_int lo_data; /* General class specific data. */ > >> struct witness *lo_witness; /* Data for witness. */ > >> }; > >> > >> This may be somewhat disruptive, I haven't played with how it would > >> actually change driver/etc/code. > > Where would the memory for the char* array come from? > > > That is a good question. I suspect it would be up to the subsystem to > allocate it. > > Wouldn't it be trivial for *most* of the subsystems to simply have this > either as a static global or static function variable: > > static char *mutex_typename = { "kqueue", "foo" }; > > Under kern I see this: > grep mtx_init * | grep -v NULL > ... > kern_rmlock.c: mtx_init(&rm->rm_lock_mtx, name, "rmlock_mtx", > MTX_NOWITNESS); > subr_bus.c: mtx_init(&devsoftc.mtx, "dev mtx", "devd", MTX_DEF); > > Those are solved with statics. > > Another example: > > sys/dev/ae/if_ae.c > mtx_init(&sc->mtx, device_get_nameunit(dev), MTX_NETWORK_LOCK, > MTX_DEF); > > I think the array could be in the softc here? sc->mutex_name_type[0] = > device_get_nameunit(dev); sc->mutex_name_type[1] = MTX_NETWORK_LOCK; > > Do we want to do that? It moves "wasting space" to another variable. > > I'm not sure where there isn't the possibility of using either static > (for a global mutex) or space inside the equiv of the softc (or proc or > whatever) for this? > > I'm not sure this is a good idea, just an idea. Are there places where > it's not as simple as doing this? To be honest, the whole name vs type thing isn't widely used, and it makes the mtx_init() function kind of fugly. I think what I would actually prefer is to just kill it, changing the various places that pass a separate name to just pass the type instead. Note that none of the other lock APIs even allow setting a separate type. This would let us remove the static pending list array as well. (And yes, I added the name vs type thing, but at this point I think it did not turn out nearly as useful as I had thought it would be.) The original issue of picking useful-to-witness lock names (i.e. not just using device_get_nameunit()) still remains of course. -- John Baldwin