Rick, I have a question, what will happen if the nfs_freesillyrename() call happens when the mount is down? Will it block the taskqueue_thread?
If so, it might make more sense to make a taskqueue per mount point. If not, excuse me. :-) -Alfred * Rick Macklem <rmack...@freebsd.org> [111115 21:05] wrote: > Author: rmacklem > Date: Wed Nov 16 05:05:13 2011 > New Revision: 227549 > URL: http://svn.freebsd.org/changeset/base/227549 > > Log: > MFC: r224604 > Fix a LOR in the NFS client which could cause a deadlock. > This was reported to the mailing list freebsd-...@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. > > Modified: > stable/7/sys/nfsclient/nfs_node.c > stable/7/sys/nfsclient/nfsnode.h > Directory Properties: > stable/7/sys/ (props changed) > stable/7/sys/cddl/contrib/opensolaris/ (props changed) > stable/7/sys/contrib/dev/acpica/ (props changed) > stable/7/sys/contrib/pf/ (props changed) > > Modified: stable/7/sys/nfsclient/nfs_node.c > ============================================================================== > --- stable/7/sys/nfsclient/nfs_node.c Wed Nov 16 02:52:24 2011 > (r227548) > +++ stable/7/sys/nfsclient/nfs_node.c Wed Nov 16 05:05:13 2011 > (r227549) > @@ -45,6 +45,7 @@ __FBSDID("$FreeBSD$"); > #include <sys/proc.h> > #include <sys/socket.h> > #include <sys/sysctl.h> > +#include <sys/taskqueue.h> > #include <sys/vnode.h> > > #include <vm/uma.h> > @@ -59,6 +60,8 @@ __FBSDID("$FreeBSD$"); > > static uma_zone_t nfsnode_zone; > > +static void nfs_freesillyrename(void *arg, __unused int pending); > + > #define TRUE 1 > #define FALSE 0 > > @@ -191,6 +194,20 @@ nfs_nget(struct mount *mntp, nfsfh_t *fh > 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_NFSREQ); > +} > + > int > nfs_inactive(struct vop_inactive_args *ap) > { > @@ -213,8 +230,8 @@ nfs_inactive(struct vop_inactive_args *a > */ > (sp->s_removeit)(sp); > crfree(sp->s_cred); > - vrele(sp->s_dvp); > - FREE((caddr_t)sp, M_NFSREQ); > + TASK_INIT(&sp->s_task, 0, nfs_freesillyrename, sp); > + taskqueue_enqueue(taskqueue_thread, &sp->s_task); > } > np->n_flag &= NMODIFIED; > return (0); > > Modified: stable/7/sys/nfsclient/nfsnode.h > ============================================================================== > --- stable/7/sys/nfsclient/nfsnode.h Wed Nov 16 02:52:24 2011 > (r227548) > +++ stable/7/sys/nfsclient/nfsnode.h Wed Nov 16 05:05:13 2011 > (r227549) > @@ -36,6 +36,7 @@ > #ifndef _NFSCLIENT_NFSNODE_H_ > #define _NFSCLIENT_NFSNODE_H_ > > +#include <sys/_task.h> > #if !defined(_NFSCLIENT_NFS_H_) && !defined(_KERNEL) > #include <nfs/nfs.h> > #endif > @@ -45,6 +46,7 @@ > * can be removed by nfs_inactive() > */ > struct sillyrename { > + struct task s_task; > struct ucred *s_cred; > struct vnode *s_dvp; > int (*s_removeit)(struct sillyrename *sp); -- - Alfred Perlstein .- VMOA #5191, 03 vmax, 92 gs500, 85 ch250, 07 zx10 .- FreeBSD committer _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
