Mercurial > illumos > illumos-gate
changeset 10532:f0c47e0a91ac
6853999 bump mi_srvset_cnt in remove_mi()
6868260 panic in nfs4delegreturn_cleanup_impl() even after fix for 6721281
6874901 race between find_nfs4_server_all() and nfs4_server_rele()
author | Pavel Filipensky <Pavel.Filipensky@Sun.COM> |
---|---|
date | Tue, 15 Sep 2009 04:26:04 +0100 |
parents | 3dbb42515756 |
children | 2bb744c642f6 |
files | usr/src/uts/common/fs/nfs/nfs4_callback.c usr/src/uts/common/fs/nfs/nfs4_vfsops.c |
diffstat | 2 files changed, 54 insertions(+), 56 deletions(-) [+] |
line wrap: on
line diff
--- a/usr/src/uts/common/fs/nfs/nfs4_callback.c Tue Sep 15 10:38:49 2009 +0800 +++ b/usr/src/uts/common/fs/nfs/nfs4_callback.c Tue Sep 15 04:26:04 2009 +0100 @@ -1296,14 +1296,6 @@ boolean_t need_rele = B_FALSE; /* - * Nothing todo if OPEN_DELEGATE_NONE. Return before calling - * find_nfs4_server_all(), since we might be called from - * nfs4_recov_tread() after nfs4_server_t has been freed. - */ - if (rp->r_deleg_type == OPEN_DELEGATE_NONE) - return; - - /* * Caller must be holding mi_recovlock in read mode * to call here. This is provided by start_op. * Delegation management requires to grab s_lock @@ -1312,7 +1304,8 @@ if (np == NULL) { np = find_nfs4_server_all(mi, 1); - ASSERT(np != NULL); + if (np == NULL) + return; need_rele = B_TRUE; } else { mutex_enter(&np->s_lock); @@ -1320,6 +1313,14 @@ mutex_enter(&rp->r_statev4_lock); + if (rp->r_deleg_type == OPEN_DELEGATE_NONE) { + mutex_exit(&rp->r_statev4_lock); + mutex_exit(&np->s_lock); + if (need_rele) + nfs4_server_rele(np); + return; + } + /* * Free the cred originally held when * the delegation was granted. Caller must
--- a/usr/src/uts/common/fs/nfs/nfs4_vfsops.c Tue Sep 15 10:38:49 2009 +0800 +++ b/usr/src/uts/common/fs/nfs/nfs4_vfsops.c Tue Sep 15 04:26:04 2009 +0100 @@ -3432,6 +3432,7 @@ /* Now mark the mntinfo4's links as being removed */ mi->mi_clientid_prev = mi->mi_clientid_next = NULL; mi->mi_srv = NULL; + mi->mi_srvset_cnt++; VFS_RELE(mi->mi_vfsp); } @@ -3476,11 +3477,10 @@ } (void) nfs_rw_enter_sig(&mi->mi_recovlock, RW_READER, 0); - sp = mi->mi_srv; - if (sp != NULL) { - mutex_enter(&sp->s_lock); + if (sp = find_nfs4_server_all(mi, 1)) { nfs4_remove_mi_from_server_nolock(mi, sp); mutex_exit(&sp->s_lock); + nfs4_server_rele(sp); } nfs_rw_exit(&mi->mi_recovlock); } @@ -3778,17 +3778,31 @@ } /* - * find_nfs4_server() and find_nfs4_server_all() are used to find - * a nfs4_server_t to which the mntinfo4_t was already linked. - * this is in contrast to searching a new nfs4_server_t, by walking - * the nfs4_server_lst list, needed e.g. when doing a failover. - * * Locks the nfs4_server down if it is found and returns a reference that * must eventually be freed. - * - * Returns NULL it no match is found. This means one of two things: either - * mi is in the process of being mounted, or mi has been unmounted. - * + */ +static nfs4_server_t * +lookup_nfs4_server(nfs4_server_t *sp, int any_state) +{ + nfs4_server_t *np; + + mutex_enter(&nfs4_server_lst_lock); + for (np = nfs4_server_lst.forw; np != &nfs4_server_lst; np = np->forw) { + mutex_enter(&np->s_lock); + if (np == sp && np->s_refcnt > 0 && + (np->s_thread_exit != NFS4_THREAD_EXIT || any_state)) { + mutex_exit(&nfs4_server_lst_lock); + np->s_refcnt++; + return (np); + } + mutex_exit(&np->s_lock); + } + mutex_exit(&nfs4_server_lst_lock); + + return (NULL); +} + +/* * The caller should be holding mi->mi_recovlock, and it should continue to * hold the lock until done with the returned nfs4_server_t. Once * mi->mi_recovlock is released, there is no guarantee that the returned @@ -3797,30 +3811,37 @@ nfs4_server_t * find_nfs4_server(mntinfo4_t *mi) { - return (find_nfs4_server_all(mi, 0)); + ASSERT(nfs_rw_lock_held(&mi->mi_recovlock, RW_READER) || + nfs_rw_lock_held(&mi->mi_recovlock, RW_WRITER)); + + return (lookup_nfs4_server(mi->mi_srv, 0)); } /* - * Same as above, but takes an "all" parameter which can be + * Same as above, but takes an "any_state" parameter which can be * set to 1 if the caller wishes to find nfs4_server_t's which * have been marked for termination by the exit of the renew * thread. This should only be used by operations which are * cleaning up and will not cause an OTW op. */ nfs4_server_t * -find_nfs4_server_all(mntinfo4_t *mi, int all) +find_nfs4_server_all(mntinfo4_t *mi, int any_state) { - nfs4_server_t *np = mi->mi_srv; - ASSERT(nfs_rw_lock_held(&mi->mi_recovlock, RW_READER) || nfs_rw_lock_held(&mi->mi_recovlock, RW_WRITER)); - if (np && (np->s_thread_exit != NFS4_THREAD_EXIT || all)) { - mutex_enter(&np->s_lock); - np->s_refcnt++; - return (np); - } - return (NULL); + return (lookup_nfs4_server(mi->mi_srv, any_state)); +} + +/* + * Lock sp, but only if it's still active (in the list and hasn't been + * flagged as exiting) or 'any_state' is non-zero. + * Returns TRUE if sp got locked and adds a reference to sp. + */ +bool_t +nfs4_server_vlock(nfs4_server_t *sp, int any_state) +{ + return (lookup_nfs4_server(sp, any_state) != NULL); } /* @@ -3879,30 +3900,6 @@ } /* - * Lock sp, but only if it's still active (in the list and hasn't been - * flagged as exiting) or 'all' is non-zero. - * Returns TRUE if sp got locked and adds a reference to sp. - */ -bool_t -nfs4_server_vlock(nfs4_server_t *sp, int all) -{ - nfs4_server_t *np; - - mutex_enter(&nfs4_server_lst_lock); - for (np = nfs4_server_lst.forw; np != &nfs4_server_lst; np = np->forw) { - if (sp == np && (np->s_thread_exit != NFS4_THREAD_EXIT || - all != 0)) { - mutex_enter(&np->s_lock); - np->s_refcnt++; - mutex_exit(&nfs4_server_lst_lock); - return (TRUE); - } - } - mutex_exit(&nfs4_server_lst_lock); - return (FALSE); -} - -/* * Fork off a thread to free the data structures for a mount. */