Fix a LOR in the NFS client which could cause a deadlock.

This was reported to the mailing list freebsd-net@freebsd.org
on July 21, 2011 under the subject "LOR with nfsclient sillyrename".
The LOR occurred when nfs_inactive() called vrele(sp->s_dvp)
while holding the vnode lock on the file in s_dvp. This patch
modifies the client so that it performs the vrele(sp->s_dvp)
as a separate task to avoid the LOR. This fix was discussed
with jhb@ and kib@, who both proposed variations of it.

Tested by:	pho, jlott at averesystems.com
Submitted by:	jhb (earlier version)
Reviewed by:	kib
Approved by:	re (kib)
MFC after:	2 weeks
This commit is contained in:
rmacklem 2011-08-02 11:28:42 +00:00
parent 9a3c5b3af4
commit afa0f6e53c
2 changed files with 22 additions and 2 deletions

View File

@ -47,6 +47,7 @@ __FBSDID("$FreeBSD$");
#include <sys/proc.h> #include <sys/proc.h>
#include <sys/socket.h> #include <sys/socket.h>
#include <sys/sysctl.h> #include <sys/sysctl.h>
#include <sys/taskqueue.h>
#include <sys/vnode.h> #include <sys/vnode.h>
#include <vm/uma.h> #include <vm/uma.h>
@ -65,6 +66,8 @@ MALLOC_DECLARE(M_NEWNFSREQ);
uma_zone_t newnfsnode_zone; uma_zone_t newnfsnode_zone;
static void nfs_freesillyrename(void *arg, __unused int pending);
void void
ncl_nhinit(void) ncl_nhinit(void)
{ {
@ -186,6 +189,20 @@ ncl_nget(struct mount *mntp, u_int8_t *fhp, int fhsize, struct nfsnode **npp,
return (0); return (0);
} }
/*
* Do the vrele(sp->s_dvp) as a separate task in order to avoid a
* deadlock because of a LOR when vrele() locks the directory vnode.
*/
static void
nfs_freesillyrename(void *arg, __unused int pending)
{
struct sillyrename *sp;
sp = arg;
vrele(sp->s_dvp);
free(sp, M_NEWNFSREQ);
}
int int
ncl_inactive(struct vop_inactive_args *ap) ncl_inactive(struct vop_inactive_args *ap)
{ {
@ -220,8 +237,8 @@ ncl_inactive(struct vop_inactive_args *ap)
*/ */
ncl_removeit(sp, vp); ncl_removeit(sp, vp);
crfree(sp->s_cred); crfree(sp->s_cred);
vrele(sp->s_dvp); TASK_INIT(&sp->s_task, 0, nfs_freesillyrename, sp);
FREE((caddr_t)sp, M_NEWNFSREQ); taskqueue_enqueue(taskqueue_thread, &sp->s_task);
mtx_lock(&np->n_mtx); mtx_lock(&np->n_mtx);
} }
np->n_flag &= NMODIFIED; np->n_flag &= NMODIFIED;

View File

@ -35,11 +35,14 @@
#ifndef _NFSCLIENT_NFSNODE_H_ #ifndef _NFSCLIENT_NFSNODE_H_
#define _NFSCLIENT_NFSNODE_H_ #define _NFSCLIENT_NFSNODE_H_
#include <sys/_task.h>
/* /*
* Silly rename structure that hangs off the nfsnode until the name * Silly rename structure that hangs off the nfsnode until the name
* can be removed by nfs_inactive() * can be removed by nfs_inactive()
*/ */
struct sillyrename { struct sillyrename {
struct task s_task;
struct ucred *s_cred; struct ucred *s_cred;
struct vnode *s_dvp; struct vnode *s_dvp;
long s_namlen; long s_namlen;