On Thu, 02 Oct 2008 02:56:46 +0200, Tom Haynes <Thomas.Haynes at Sun.COM> wrote:
> The webrev can be found here: > http://cr.opensolaris.org/~tdh/6751438/ looks sufficiently sane to me. comments though... 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. 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' 1706 * Common code to safely release net_cnt_lock and net_tree_lock 1707 */ 1708 void 1709 nfs4_ephemeral_umount_unlock(bool_t *pmust_unlock, 1710 bool_t *pmust_rele, nfs4_ephemeral_tree_t **pnet) 1711 { 1712 nfs4_ephemeral_tree_t *net = *pnet; 1713 1714 if (*pmust_unlock) { 1715 mutex_enter(&net->net_cnt_lock); 1716 net->net_status &= ~NFS4_EPHEMERAL_TREE_UMOUNTING; 1717 if (*pmust_rele) 1718 nfs4_ephemeral_tree_decr(net); 1719 mutex_exit(&net->net_cnt_lock); 1720 1721 mutex_exit(&net->net_tree_lock); 1722 1723 *pmust_unlock = FALSE; 1724 } 1725 } --- frankB
