Date: Mon, 17 Sep 2007 12:43:50 -0700 From: "Kip Macy" <kip.macy@gmail.com> To: "Julian Elischer" <julian@elischer.org> Cc: Marko Zec <zec@icir.org>, Perforce Change Reviews <perforce@freebsd.org> Subject: Re: PERFORCE change 125520 for review Message-ID: <b1fa29170709171243y4bffc448gdd0384c452befd36@mail.gmail.com> In-Reply-To: <46EED397.3040700@elischer.org> References: <200708212351.l7LNpi6Q006480@repoman.freebsd.org> <46EEC18F.6000809@elischer.org> <200709172048.29253.zec@icir.org> <46EED397.3040700@elischer.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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. -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?b1fa29170709171243y4bffc448gdd0384c452befd36>