From owner-svn-src-all@FreeBSD.ORG Wed Feb 6 17:06:53 2013 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 4870A7DB; Wed, 6 Feb 2013 17:06: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 2C28AAEA; Wed, 6 Feb 2013 17:06: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 r16H6row088895; Wed, 6 Feb 2013 17:06:53 GMT (envelope-from jhb@svn.freebsd.org) Received: (from jhb@localhost) by svn.freebsd.org (8.14.5/8.14.5/Submit) id r16H6qMo088889; Wed, 6 Feb 2013 17:06:52 GMT (envelope-from jhb@svn.freebsd.org) Message-Id: <201302061706.r16H6qMo088889@svn.freebsd.org> From: John Baldwin Date: Wed, 6 Feb 2013 17:06:52 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r246417 - in head/sys: fs/nfs kern nfsclient sys X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Feb 2013 17:06:53 -0000 Author: jhb Date: Wed Feb 6 17:06:51 2013 New Revision: 246417 URL: http://svnweb.freebsd.org/changeset/base/246417 Log: Rework the handling of stop signals in the NFS client. The changes in 195702, 195703, and 195821 prevented a thread from suspending while holding locks inside of NFS by forcing the thread to fail sleeps with EINTR or ERESTART but defer the thread suspension to the user boundary. However, this had the effect that stopping a process during an NFS request could abort the request and trigger EINTR errors that were visible to userland processes (previously the thread would have suspended and completed the request once it was resumed). This change instead effectively masks stop signals while in the NFS client. It uses the existing TDF_SBDRY flag to effect this since SIGSTOP cannot be masked directly. Also, instead of setting PBDRY on individual sleeps, the NFS client now sets the TDF_SBDRY flag around each NFS request and stop signals are masked for all sleeps during that region (the previous change missed sleeps in lockmgr locks). The end result is that stop signals sent to threads performing an NFS request are completely ignored until after the NFS request has finished processing and the thread prepares to return to userland. This restores the behavior of stop signals being transparent to userland processes while still preventing threads from suspending while holding NFS locks. Reviewed by: kib MFC after: 1 month Modified: head/sys/fs/nfs/nfs_commonkrpc.c head/sys/kern/kern_sig.c head/sys/kern/subr_sleepqueue.c head/sys/nfsclient/nfs_krpc.c head/sys/sys/signalvar.h Modified: head/sys/fs/nfs/nfs_commonkrpc.c ============================================================================== --- head/sys/fs/nfs/nfs_commonkrpc.c Wed Feb 6 16:43:35 2013 (r246416) +++ head/sys/fs/nfs/nfs_commonkrpc.c Wed Feb 6 17:06:51 2013 (r246417) @@ -1031,7 +1031,6 @@ int newnfs_sig_set[] = { SIGTERM, SIGHUP, SIGKILL, - SIGSTOP, SIGQUIT }; @@ -1052,7 +1051,7 @@ nfs_sig_pending(sigset_t set) /* * The set/restore sigmask functions are used to (temporarily) overwrite - * the process p_sigmask during an RPC call (for example). These are also + * the thread td_sigmask during an RPC call (for example). These are also * used in other places in the NFS client that might tsleep(). */ void @@ -1081,8 +1080,10 @@ 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); - kern_sigprocmask(td, SIG_SETMASK, &newset, oldset, 0); } void @@ -1091,6 +1092,7 @@ 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/kern/kern_sig.c ============================================================================== --- head/sys/kern/kern_sig.c Wed Feb 6 16:43:35 2013 (r246416) +++ head/sys/kern/kern_sig.c Wed Feb 6 17:06:51 2013 (r246417) @@ -2364,6 +2364,13 @@ tdsigwakeup(struct thread *td, int sig, } /* + * Don't awaken a sleeping thread for SIGSTOP if the + * STOP signal is deferred. + */ + if ((prop & SA_STOP) && (td->td_flags & TDF_SBDRY)) + goto out; + + /* * Give low priority threads a better chance to run. */ if (td->td_priority > PUSER) @@ -2404,12 +2411,13 @@ sig_suspend_threads(struct thread *td, s if ((TD_IS_SLEEPING(td2) || TD_IS_SWAPPED(td2)) && (td2->td_flags & TDF_SINTR)) { if (td2->td_flags & TDF_SBDRY) { - if (TD_IS_SUSPENDED(td2)) - wakeup_swapper |= - thread_unsuspend_one(td2); - if (TD_ON_SLEEPQ(td2)) - wakeup_swapper |= - sleepq_abort(td2, ERESTART); + /* + * Once a thread is asleep with + * TDF_SBDRY set, it should never + * become suspended due to this check. + */ + KASSERT(!TD_IS_SUSPENDED(td2), + ("thread with deferred stops suspended")); } else if (!TD_IS_SUSPENDED(td2)) { thread_suspend_one(td2); } @@ -2529,6 +2537,34 @@ tdsigcleanup(struct thread *td) } +/* Defer the delivery of SIGSTOP for the current thread. */ +void +sigdeferstop(struct thread *td) +{ + + KASSERT(!(td->td_flags & TDF_SBDRY), + ("attempt to set TDF_SBDRY recursively")); + thread_lock(td); + td->td_flags |= TDF_SBDRY; + thread_unlock(td); +} + +/* + * Permit the delivery of SIGSTOP for the current thread. This does + * not immediately suspend if a stop was posted. Instead, the thread + * will suspend either via ast() or a subsequent interruptible sleep. + */ +void +sigallowstop(struct thread *td) +{ + + KASSERT(td->td_flags & TDF_SBDRY, + ("attempt to clear already-cleared TDF_SBDRY")); + thread_lock(td); + td->td_flags &= ~TDF_SBDRY; + thread_unlock(td); +} + /* * If the current process has received a signal (should be caught or cause * termination, should interrupt current syscall), return the signal number. @@ -2561,7 +2597,7 @@ issignal(struct thread *td, int stop_all SIGSETOR(sigpending, p->p_sigqueue.sq_signals); SIGSETNAND(sigpending, td->td_sigmask); - if (p->p_flag & P_PPWAIT) + if (p->p_flag & P_PPWAIT || td->td_flags & TDF_SBDRY) SIG_STOPSIGMASK(sigpending); if (SIGISEMPTY(sigpending)) /* no signal to send */ return (0); @@ -2677,10 +2713,6 @@ issignal(struct thread *td, int stop_all (p->p_pgrp->pg_jobc == 0 && prop & SA_TTYSTOP)) break; /* == ignore */ - - /* Ignore, but do not drop the stop signal. */ - if (stop_allowed != SIG_STOP_ALLOWED) - return (sig); mtx_unlock(&ps->ps_mtx); WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, &p->p_mtx.lock_object, "Catching SIGSTOP"); Modified: head/sys/kern/subr_sleepqueue.c ============================================================================== --- head/sys/kern/subr_sleepqueue.c Wed Feb 6 16:43:35 2013 (r246416) +++ head/sys/kern/subr_sleepqueue.c Wed Feb 6 17:06:51 2013 (r246417) @@ -352,8 +352,6 @@ sleepq_add(void *wchan, struct lock_obje if (flags & SLEEPQ_INTERRUPTIBLE) { td->td_flags |= TDF_SINTR; td->td_flags &= ~TDF_SLEEPABORT; - if (flags & SLEEPQ_STOP_ON_BDRY) - td->td_flags |= TDF_SBDRY; } thread_unlock(td); } @@ -600,7 +598,7 @@ sleepq_check_signals(void) /* We are no longer in an interruptible sleep. */ if (td->td_flags & TDF_SINTR) - td->td_flags &= ~(TDF_SINTR | TDF_SBDRY); + td->td_flags &= ~TDF_SINTR; if (td->td_flags & TDF_SLEEPABORT) { td->td_flags &= ~TDF_SLEEPABORT; @@ -747,7 +745,7 @@ sleepq_resume_thread(struct sleepqueue * td->td_wmesg = NULL; td->td_wchan = NULL; - td->td_flags &= ~(TDF_SINTR | TDF_SBDRY); + td->td_flags &= ~TDF_SINTR; CTR3(KTR_PROC, "sleepq_wakeup: thread %p (pid %ld, %s)", (void *)td, (long)td->td_proc->p_pid, td->td_name); Modified: head/sys/nfsclient/nfs_krpc.c ============================================================================== --- head/sys/nfsclient/nfs_krpc.c Wed Feb 6 16:43:35 2013 (r246416) +++ head/sys/nfsclient/nfs_krpc.c Wed Feb 6 17:06:51 2013 (r246417) @@ -699,7 +699,6 @@ int nfs_sig_set[] = { SIGTERM, SIGHUP, SIGKILL, - SIGSTOP, SIGQUIT }; @@ -720,7 +719,7 @@ nfs_sig_pending(sigset_t set) /* * The set/restore sigmask functions are used to (temporarily) overwrite - * the process p_sigmask during an RPC call (for example). These are also + * the thread td_sigmask during an RPC call (for example). These are also * used in other places in the NFS client that might tsleep(). */ void @@ -749,8 +748,10 @@ 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); - kern_sigprocmask(td, SIG_SETMASK, &newset, oldset, 0); } void @@ -759,6 +760,7 @@ 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/sys/signalvar.h ============================================================================== --- head/sys/sys/signalvar.h Wed Feb 6 16:43:35 2013 (r246416) +++ head/sys/sys/signalvar.h Wed Feb 6 17:06:51 2013 (r246417) @@ -328,6 +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); void execsigs(struct proc *p); void gsignal(int pgid, int sig, ksiginfo_t *ksi); void killproc(struct proc *p, char *why);