Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 17 Sep 2007 12:20:55 -0700
From:      Julian Elischer <julian@elischer.org>
To:        Marko Zec <zec@icir.org>
Cc:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   Re: PERFORCE change 125520 for review
Message-ID:  <46EED397.3040700@elischer.org>
In-Reply-To: <200709172048.29253.zec@icir.org>
References:  <200708212351.l7LNpi6Q006480@repoman.freebsd.org> <46EEC18F.6000809@elischer.org> <200709172048.29253.zec@icir.org>

next in thread | previous in thread | raw e-mail | index | archive | help
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..)


> 
> 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?46EED397.3040700>