Date: Tue, 18 Sep 2007 00:28:23 +0200 From: Marko Zec <zec@icir.org> To: "Kip Macy" <kip.macy@gmail.com> Cc: Perforce Change Reviews <perforce@freebsd.org>, Julian Elischer <julian@elischer.org> Subject: Re: PERFORCE change 125520 for review Message-ID: <200709180028.24481.zec@icir.org> In-Reply-To: <b1fa29170709171243y4bffc448gdd0384c452befd36@mail.gmail.com> References: <200708212351.l7LNpi6Q006480@repoman.freebsd.org> <46EED397.3040700@elischer.org> <b1fa29170709171243y4bffc448gdd0384c452befd36@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Monday 17 September 2007 21:43:50 Kip Macy wrote: > On 9/17/07, Julian Elischer <julian@elischer.org> wrote: > > Marko Zec wrote: > > > On Monday 17 September 2007 20:03:59 Julian Elischer wrote: > > >> Marko Zec wrote: > > >>> http://perforce.freebsd.org/chv.cgi?CH=125520 > > >>> > > >>> Change 125520 by zec@zec_tpx32 on 2007/08/21 23:51:39 > > >>> > > >>> Given that ng_pipe nodes can be connected into arbitrary > > >>> topologies, and therefore it is possible for ngp_rcvdata() > > >>> to be recursively called from a single thread, it is > > >>> necessary to explicitly allow for the ng_pipe_giant mutex > > >>> to be recursively acquireable. > > >> > > >> OR use a different locking scheme. > > > > > > That's right, but I'm just wondering is there anything > > > fundamentally wrong with lock recursing (both in general as well > > > as in this particular case)? > > > > we are trying as a general rule trying to keep lock recursion to an > > absolute minimum. It can make debugging other things very hard. and > > can introduce bugs that are hard to find.. > > > > Generally a bad idea. If you don't know you are recursing, how can > > you avoid the problems you don't know about? (sounds silly but..) > > Just to add my 0.02$ from recent experiences ... > Lock recursion creates the following problems: > - lock is often held much longer than it really needs to be > - it is no longer possible from code inspection to determine > where the lock is or is not held > - it makes it near impossible to mix shared and exclusive access > to a lock, acquiring an exclusive lock that you already hold shared > results in a deadlock and vice versa > - it makes lock ordering more complicated to infer and work with > > So ... from code reading experience recently recursive locks are > typically needed for code where the control flow pre-dates any > thoughts of locking. ...which precisely might often be the case with netgraph, where it is impossible to know beforehand how the interconnection topology would look like, nor can it be always determined upfront which paths through the topology (i.e. translating to a call graph) would or would not be taken. Anyhow, point taken. Though given that ng_pipe is not really a critical part of our infrastructure, but more like a toy I'm occassionaly playing with, I'll take some time before more thoroughly thinking of and implementing an alternative locking scheme. Marko > -Kip > > > > Marko > > > > > >> i.e. reference counts or something. > > >> > > >>> Affected files ... > > >>> > > >>> .. //depot/projects/vimage/src/sys/netgraph/ng_pipe.c#2 edit > > >>> > > >>> Differences ... > > >>> > > >>> ==== //depot/projects/vimage/src/sys/netgraph/ng_pipe.c#2 > > >>> (text+ko) ==== > > >>> > > >>> @@ -1028,7 +1028,7 @@ > > >>> error = EEXIST; > > >>> else { > > >>> mtx_init(&ng_pipe_giant, "ng_pipe_giant", > > >>> NULL, - MTX_DEF); > > >>> + MTX_DEF | MTX_RECURSE); > > >>> LIST_INIT(&node_head); > > >>> LIST_INIT(&hook_head); > > >>> ds_handle = timeout((timeout_t *) > > >>> &pipe_scheduler,
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200709180028.24481.zec>