From owner-svn-src-head@FreeBSD.ORG Thu Feb 21 19:02:53 2013 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 33AC9FC0; Thu, 21 Feb 2013 19:02:53 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) by mx1.freebsd.org (Postfix) with ESMTP id 227B31DC; Thu, 21 Feb 2013 19:02:53 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.5/8.14.5) with ESMTP id r1LJ2rpk033722; Thu, 21 Feb 2013 19:02:53 GMT (envelope-from jhb@svn.freebsd.org) Received: (from jhb@localhost) by svn.freebsd.org (8.14.5/8.14.5/Submit) id r1LJ2o5T033708; Thu, 21 Feb 2013 19:02:50 GMT (envelope-from jhb@svn.freebsd.org) Message-Id: <201302211902.r1LJ2o5T033708@svn.freebsd.org> From: John Baldwin Date: Thu, 21 Feb 2013 19:02:50 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r247116 - in head/sys: fs/nfs fs/nfsclient kern nfsclient sys tools X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 21 Feb 2013 19:02:53 -0000 Author: jhb Date: Thu Feb 21 19:02:50 2013 New Revision: 247116 URL: http://svnweb.freebsd.org/changeset/base/247116 Log: Further refine the handling of stop signals in the NFS client. The changes in r246417 were incomplete as they did not add explicit calls to sigdeferstop() around all the places that previously passed SBDRY to _sleep(). In addition, nfs_getcacheblk() could trigger a write RPC from getblk() resulting in sigdeferstop() recursing. Rather than manually deferring stop signals in specific places, change the VFS_*() and VOP_*() methods to defer stop signals for filesystems which request this behavior via a new VFCF_SBDRY flag. Note that this has to be a VFC flag rather than a MNTK flag so that it works properly with VFS_MOUNT() when the mount is not yet fully constructed. For now, only the NFS clients are set this new flag in VFS_SET(). A few other related changes: - Add an assertion to ensure that TDF_SBDRY doesn't leak to userland. - When a lookup request uses VOP_READLINK() to follow a symlink, mark the request as being on behalf of the thread performing the lookup (cnp_thread) rather than using a NULL thread pointer. This causes NFS to properly handle signals during this VOP on an interruptible mount. PR: kern/176179 Reported by: Russell Cattelan (sigdeferstop() recursion) Reviewed by: kib MFC after: 1 month Modified: head/sys/fs/nfs/nfs_commonkrpc.c head/sys/fs/nfsclient/nfs_clvfsops.c head/sys/kern/kern_sig.c head/sys/kern/subr_trap.c head/sys/kern/vfs_export.c head/sys/kern/vfs_lookup.c head/sys/nfsclient/nfs_krpc.c head/sys/nfsclient/nfs_vfsops.c head/sys/sys/mount.h head/sys/sys/signalvar.h head/sys/tools/vnode_if.awk Modified: head/sys/fs/nfs/nfs_commonkrpc.c ============================================================================== --- head/sys/fs/nfs/nfs_commonkrpc.c Thu Feb 21 19:02:29 2013 (r247115) +++ head/sys/fs/nfs/nfs_commonkrpc.c Thu Feb 21 19:02:50 2013 (r247116) @@ -1080,7 +1080,6 @@ newnfs_set_sigmask(struct thread *td, si SIGDELSET(newset, newnfs_sig_set[i]); } mtx_unlock(&p->p_sigacts->ps_mtx); - sigdeferstop(td); kern_sigprocmask(td, SIG_SETMASK, &newset, oldset, SIGPROCMASK_PROC_LOCKED); PROC_UNLOCK(p); @@ -1092,7 +1091,6 @@ newnfs_restore_sigmask(struct thread *td if (td == NULL) td = curthread; /* XXX */ kern_sigprocmask(td, SIG_SETMASK, set, NULL, 0); - sigallowstop(td); } /* Modified: head/sys/fs/nfsclient/nfs_clvfsops.c ============================================================================== --- head/sys/fs/nfsclient/nfs_clvfsops.c Thu Feb 21 19:02:29 2013 (r247115) +++ head/sys/fs/nfsclient/nfs_clvfsops.c Thu Feb 21 19:02:50 2013 (r247116) @@ -132,7 +132,7 @@ static struct vfsops nfs_vfsops = { .vfs_unmount = nfs_unmount, .vfs_sysctl = nfs_sysctl, }; -VFS_SET(nfs_vfsops, nfs, VFCF_NETWORK); +VFS_SET(nfs_vfsops, nfs, VFCF_NETWORK | VFCF_SBDRY); /* So that loader and kldload(2) can find us, wherever we are.. */ MODULE_VERSION(nfs, 1); Modified: head/sys/kern/kern_sig.c ============================================================================== --- head/sys/kern/kern_sig.c Thu Feb 21 19:02:29 2013 (r247115) +++ head/sys/kern/kern_sig.c Thu Feb 21 19:02:50 2013 (r247116) @@ -2537,16 +2537,22 @@ tdsigcleanup(struct thread *td) } -/* Defer the delivery of SIGSTOP for the current thread. */ -void -sigdeferstop(struct thread *td) +/* + * Defer the delivery of SIGSTOP for the current thread. Returns true + * if stops were deferred and false if they were already deferred. + */ +int +sigdeferstop(void) { + struct thread *td; - KASSERT(!(td->td_flags & TDF_SBDRY), - ("attempt to set TDF_SBDRY recursively")); + td = curthread; + if (td->td_flags & TDF_SBDRY) + return (0); thread_lock(td); td->td_flags |= TDF_SBDRY; thread_unlock(td); + return (1); } /* @@ -2555,11 +2561,11 @@ sigdeferstop(struct thread *td) * will suspend either via ast() or a subsequent interruptible sleep. */ void -sigallowstop(struct thread *td) +sigallowstop() { + struct thread *td; - KASSERT(td->td_flags & TDF_SBDRY, - ("attempt to clear already-cleared TDF_SBDRY")); + td = curthread; thread_lock(td); td->td_flags &= ~TDF_SBDRY; thread_unlock(td); Modified: head/sys/kern/subr_trap.c ============================================================================== --- head/sys/kern/subr_trap.c Thu Feb 21 19:02:29 2013 (r247115) +++ head/sys/kern/subr_trap.c Thu Feb 21 19:02:50 2013 (r247116) @@ -164,6 +164,8 @@ userret(struct thread *td, struct trapfr ("userret: Returning with with pinned thread")); KASSERT(td->td_vp_reserv == 0, ("userret: Returning while holding vnode reservation")); + KASSERT((td->td_flags & TDF_SBDRY) == 0, + ("userret: Returning with stop signals deferred")); #ifdef VIMAGE /* Unfortunately td_vnet_lpush needs VNET_DEBUG. */ VNET_ASSERT(curvnet == NULL, Modified: head/sys/kern/vfs_export.c ============================================================================== --- head/sys/kern/vfs_export.c Thu Feb 21 19:02:29 2013 (r247115) +++ head/sys/kern/vfs_export.c Thu Feb 21 19:02:50 2013 (r247116) @@ -49,6 +49,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include #include Modified: head/sys/kern/vfs_lookup.c ============================================================================== --- head/sys/kern/vfs_lookup.c Thu Feb 21 19:02:29 2013 (r247115) +++ head/sys/kern/vfs_lookup.c Thu Feb 21 19:02:50 2013 (r247116) @@ -339,7 +339,7 @@ namei(struct nameidata *ndp) auio.uio_offset = 0; auio.uio_rw = UIO_READ; auio.uio_segflg = UIO_SYSSPACE; - auio.uio_td = (struct thread *)0; + auio.uio_td = td; auio.uio_resid = MAXPATHLEN; error = VOP_READLINK(ndp->ni_vp, &auio, cnp->cn_cred); if (error) { Modified: head/sys/nfsclient/nfs_krpc.c ============================================================================== --- head/sys/nfsclient/nfs_krpc.c Thu Feb 21 19:02:29 2013 (r247115) +++ head/sys/nfsclient/nfs_krpc.c Thu Feb 21 19:02:50 2013 (r247116) @@ -748,7 +748,6 @@ nfs_set_sigmask(struct thread *td, sigse SIGDELSET(newset, nfs_sig_set[i]); } mtx_unlock(&p->p_sigacts->ps_mtx); - sigdeferstop(td); kern_sigprocmask(td, SIG_SETMASK, &newset, oldset, SIGPROCMASK_PROC_LOCKED); PROC_UNLOCK(p); @@ -760,7 +759,6 @@ nfs_restore_sigmask(struct thread *td, s if (td == NULL) td = curthread; /* XXX */ kern_sigprocmask(td, SIG_SETMASK, set, NULL, 0); - sigallowstop(td); } /* Modified: head/sys/nfsclient/nfs_vfsops.c ============================================================================== --- head/sys/nfsclient/nfs_vfsops.c Thu Feb 21 19:02:29 2013 (r247115) +++ head/sys/nfsclient/nfs_vfsops.c Thu Feb 21 19:02:50 2013 (r247116) @@ -146,7 +146,7 @@ static struct vfsops nfs_vfsops = { .vfs_unmount = nfs_unmount, .vfs_sysctl = nfs_sysctl, }; -VFS_SET(nfs_vfsops, oldnfs, VFCF_NETWORK); +VFS_SET(nfs_vfsops, oldnfs, VFCF_NETWORK | VFCF_SBDRY); /* So that loader and kldload(2) can find us, wherever we are.. */ MODULE_VERSION(oldnfs, 1); Modified: head/sys/sys/mount.h ============================================================================== --- head/sys/sys/mount.h Thu Feb 21 19:02:29 2013 (r247115) +++ head/sys/sys/mount.h Thu Feb 21 19:02:50 2013 (r247116) @@ -493,6 +493,7 @@ struct ovfsconf { #define VFCF_UNICODE 0x00200000 /* stores file names as Unicode */ #define VFCF_JAIL 0x00400000 /* can be mounted from within a jail */ #define VFCF_DELEGADMIN 0x00800000 /* supports delegated administration */ +#define VFCF_SBDRY 0x01000000 /* defer stop requests */ typedef uint32_t fsctlop_t; @@ -629,30 +630,121 @@ struct vfsops { vfs_statfs_t __vfs_statfs; -#define VFS_MOUNT(MP) (*(MP)->mnt_op->vfs_mount)(MP) -#define VFS_UNMOUNT(MP, FORCE) (*(MP)->mnt_op->vfs_unmount)(MP, FORCE) -#define VFS_ROOT(MP, FLAGS, VPP) \ - (*(MP)->mnt_op->vfs_root)(MP, FLAGS, VPP) -#define VFS_QUOTACTL(MP, C, U, A) \ - (*(MP)->mnt_op->vfs_quotactl)(MP, C, U, A) -#define VFS_STATFS(MP, SBP) __vfs_statfs((MP), (SBP)) -#define VFS_SYNC(MP, WAIT) (*(MP)->mnt_op->vfs_sync)(MP, WAIT) -#define VFS_VGET(MP, INO, FLAGS, VPP) \ - (*(MP)->mnt_op->vfs_vget)(MP, INO, FLAGS, VPP) -#define VFS_FHTOVP(MP, FIDP, FLAGS, VPP) \ - (*(MP)->mnt_op->vfs_fhtovp)(MP, FIDP, FLAGS, VPP) -#define VFS_CHECKEXP(MP, NAM, EXFLG, CRED, NUMSEC, SEC) \ - (*(MP)->mnt_op->vfs_checkexp)(MP, NAM, EXFLG, CRED, NUMSEC, SEC) -#define VFS_EXTATTRCTL(MP, C, FN, NS, N) \ - (*(MP)->mnt_op->vfs_extattrctl)(MP, C, FN, NS, N) -#define VFS_SYSCTL(MP, OP, REQ) \ - (*(MP)->mnt_op->vfs_sysctl)(MP, OP, REQ) -#define VFS_SUSP_CLEAN(MP) \ - ({if (*(MP)->mnt_op->vfs_susp_clean != NULL) \ - (*(MP)->mnt_op->vfs_susp_clean)(MP); }) -#define VFS_RECLAIM_LOWERVP(MP, VP) \ - ({if (*(MP)->mnt_op->vfs_reclaim_lowervp != NULL) \ - (*(MP)->mnt_op->vfs_reclaim_lowervp)((MP), (VP)); }) +#define VFS_PROLOGUE(MP) do { \ + int _enable_stops; \ + \ + _enable_stops = ((MP) != NULL && \ + ((MP)->mnt_vfc->vfc_flags & VFCF_SBDRY) && sigdeferstop()) + +#define VFS_EPILOGUE(MP) \ + if (_enable_stops) \ + sigallowstop(); \ +} while (0) + +#define VFS_MOUNT(MP) ({ \ + int _rc; \ + \ + VFS_PROLOGUE(MP); \ + _rc = (*(MP)->mnt_op->vfs_mount)(MP); \ + VFS_EPILOGUE(MP); \ + _rc; }) + +#define VFS_UNMOUNT(MP, FORCE) ({ \ + int _rc; \ + \ + VFS_PROLOGUE(MP); \ + _rc = (*(MP)->mnt_op->vfs_unmount)(MP, FORCE); \ + VFS_EPILOGUE(MP); \ + _rc; }) + +#define VFS_ROOT(MP, FLAGS, VPP) ({ \ + int _rc; \ + \ + VFS_PROLOGUE(MP); \ + _rc = (*(MP)->mnt_op->vfs_root)(MP, FLAGS, VPP); \ + VFS_EPILOGUE(MP); \ + _rc; }) + +#define VFS_QUOTACTL(MP, C, U, A) ({ \ + int _rc; \ + \ + VFS_PROLOGUE(MP); \ + _rc = (*(MP)->mnt_op->vfs_quotactl)(MP, C, U, A); \ + VFS_EPILOGUE(MP); \ + _rc; }) + +#define VFS_STATFS(MP, SBP) ({ \ + int _rc; \ + \ + VFS_PROLOGUE(MP); \ + _rc = __vfs_statfs((MP), (SBP)); \ + VFS_EPILOGUE(MP); \ + _rc; }) + +#define VFS_SYNC(MP, WAIT) ({ \ + int _rc; \ + \ + VFS_PROLOGUE(MP); \ + _rc = (*(MP)->mnt_op->vfs_sync)(MP, WAIT); \ + VFS_EPILOGUE(MP); \ + _rc; }) + +#define VFS_VGET(MP, INO, FLAGS, VPP) ({ \ + int _rc; \ + \ + VFS_PROLOGUE(MP); \ + _rc = (*(MP)->mnt_op->vfs_vget)(MP, INO, FLAGS, VPP); \ + VFS_EPILOGUE(MP); \ + _rc; }) + +#define VFS_FHTOVP(MP, FIDP, FLAGS, VPP) ({ \ + int _rc; \ + \ + VFS_PROLOGUE(MP); \ + _rc = (*(MP)->mnt_op->vfs_fhtovp)(MP, FIDP, FLAGS, VPP); \ + VFS_EPILOGUE(MP); \ + _rc; }) + +#define VFS_CHECKEXP(MP, NAM, EXFLG, CRED, NUMSEC, SEC) ({ \ + int _rc; \ + \ + VFS_PROLOGUE(MP); \ + _rc = (*(MP)->mnt_op->vfs_checkexp)(MP, NAM, EXFLG, CRED, NUMSEC,\ + SEC); \ + VFS_EPILOGUE(MP); \ + _rc; }) + +#define VFS_EXTATTRCTL(MP, C, FN, NS, N) ({ \ + int _rc; \ + \ + VFS_PROLOGUE(MP); \ + _rc = (*(MP)->mnt_op->vfs_extattrctl)(MP, C, FN, NS, N); \ + VFS_EPILOGUE(MP); \ + _rc; }) + +#define VFS_SYSCTL(MP, OP, REQ) ({ \ + int _rc; \ + \ + VFS_PROLOGUE(MP); \ + _rc = (*(MP)->mnt_op->vfs_sysctl)(MP, OP, REQ); \ + VFS_EPILOGUE(MP); \ + _rc; }) + +#define VFS_SUSP_CLEAN(MP) do { \ + if (*(MP)->mnt_op->vfs_susp_clean != NULL) { \ + VFS_PROLOGUE(MP); \ + (*(MP)->mnt_op->vfs_susp_clean)(MP); \ + VFS_EPILOGUE(MP); \ + } \ +} while (0) + +#define VFS_RECLAIM_LOWERVP(MP, VP) do { \ + if (*(MP)->mnt_op->vfs_reclaim_lowervp != NULL) { \ + VFS_PROLOGUE(MP); \ + (*(MP)->mnt_op->vfs_reclaim_lowervp)((MP), (VP)); \ + VFS_EPILOGUE(MP); \ + } \ +} while (0) #define VFS_KNOTE_LOCKED(vp, hint) do \ { \ Modified: head/sys/sys/signalvar.h ============================================================================== --- head/sys/sys/signalvar.h Thu Feb 21 19:02:29 2013 (r247115) +++ head/sys/sys/signalvar.h Thu Feb 21 19:02:50 2013 (r247116) @@ -328,8 +328,8 @@ extern struct mtx sigio_lock; #define SIGPROCMASK_PS_LOCKED 0x0004 int cursig(struct thread *td, int stop_allowed); -void sigdeferstop(struct thread *td); -void sigallowstop(struct thread *td); +int sigdeferstop(void); +void sigallowstop(void); void execsigs(struct proc *p); void gsignal(int pgid, int sig, ksiginfo_t *ksi); void killproc(struct proc *p, char *why); Modified: head/sys/tools/vnode_if.awk ============================================================================== --- head/sys/tools/vnode_if.awk Thu Feb 21 19:02:29 2013 (r247115) +++ head/sys/tools/vnode_if.awk Thu Feb 21 19:02:50 2013 (r247116) @@ -172,6 +172,7 @@ if (cfile) { "#include \n" \ "#include \n" \ "#include \n" \ + "#include \n" \ "#include \n" \ "#include \n" \ "\n" \ @@ -365,10 +366,12 @@ while ((getline < srcfile) > 0) { add_debug_code(name, args[i], "Entry", "\t"); printc("\tKTR_START" ctrstr); add_pre(name); + printc("\tVFS_PROLOGUE(a->a_" args[0]"->v_mount);") printc("\tif (vop->"name" != NULL)") printc("\t\trc = vop->"name"(a);") printc("\telse") printc("\t\trc = vop->vop_bypass(&a->a_gen);") + printc("\tVFS_EPILOGUE(a->a_" args[0]"->v_mount);") printc("\tSDT_PROBE(vfs, vop, " name ", return, a->a_" args[0] ", a, rc, 0, 0);\n"); printc("\tif (rc == 0) {"); for (i = 0; i < numargs; ++i)