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 <sys/mutex.h>
 #include <sys/rwlock.h>
 #include <sys/refcount.h>
+#include <sys/signalvar.h>
 #include <sys/socket.h>
 #include <sys/systm.h>
 #include <sys/vnode.h>

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 <sys/kernel.h>\n" \
            "#include <sys/mount.h>\n" \
            "#include <sys/sdt.h>\n" \
+           "#include <sys/signalvar.h>\n" \
            "#include <sys/systm.h>\n" \
            "#include <sys/vnode.h>\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)
_______________________________________________
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"

Reply via email to