On Thu, 02 Oct 2008 16:11:01 +0200, Tom Haynes <Thomas.Haynes at Sun.COM> wrote:
>> 292 nfs4_ephemeral_tree_incr(nfs4_ephemeral_tree_t *net)
>> 293 {
>> 294 ASSERT(mutex_owned(&net->net_cnt_lock));
>> 295 net->net_refcnt++;
>> 296 ASSERT(net->net_refcnt != 0);
>>
>> 312 nfs4_ephemeral_tree_decr(nfs4_ephemeral_tree_t *net)
>> 313 {
>> 314 ASSERT(mutex_owned(&net->net_cnt_lock));
>> 315 ASSERT(net->net_refcnt != 0);
>> 316 net->net_refcnt--;
>>
>> to me, it looks lile we never ever just want to continue if
>> we encounter the "net_refcnt != 0" case, debug or not, thus
>> a VERIFY() seems to be more appropriate here.
>
> I was taught not to use VERIFYs as it caused customers problems.
Outch ! really ? man, we got a problem already ;-)
http://grok.czech.sun.com:8080/source/search?q=&defs=&refs=VERIFY&path=&hist=&project=%2Fonnv-clone
nah, honestly I'd be interested to know about that, really...
>> PS: fwiw, I usually consider code that drops locks that have
>> been acquired somewhere else 'net_tree_lock' bad practice and
>> not "safely" ;-) even more if the "go/nogo" decision is being
>> parsed thru multiple layers here with 'pmust_unlock'
>
> The alternative is to pull code from the v4 unmounting down into this code.
>
> As that code is dependent on whether it is a forced unmount or not, it makes
> it harder to abstract. But that could be done.
>
> But I didn't want to pull non-mirror mount code down into the mirror mount
> code. I.e., I didn't want to inadvertently hide it because it was put where
> not expected.
quite, wasn't a request for this fix as this was not new code that had been
added but already existing one...
cheers
frankB
