svn commit: r227549 - stable/7/sys/nfsclient
Alfred Perlstein
alfred at freebsd.org
Thu Nov 17 04:16:58 UTC 2011
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 <rmacklem at 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-net at 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
More information about the svn-src-stable-7
mailing list