Date: Sun, 3 Jul 2016 18:19:48 +0000 (UTC) From: Konstantin Belousov <kib@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r302328 - in head/sys: kern sys Message-ID: <201607031819.u63IJm79075708@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: kib Date: Sun Jul 3 18:19:48 2016 New Revision: 302328 URL: https://svnweb.freebsd.org/changeset/base/302328 Log: Provide helper macros to detect 'non-silent SBDRY' state and to calculate appropriate return value for stops. Simplify the code by using them. Fix typo in sig_suspend_threads(). The thread which sleep must be aborted is td2. (*) In issignal(), when handling stopping signal for thread in TD_SBDRY_INTR state, do not stop, this is wrong and fires assert. This is yet another place where execution should be forced out of SBDRY-protected region. For such case, return -1 from issignal() and translate it to corresponding error code in sleepq_catch_signals(). Assert that other consumers of cursig() are not affected by the new return value. (*) Micro-optimize, mostly VFS and VOP methods, by avoiding calling the functions when SIGDEFERSTOP_NOP non-change is requested. (**) Reported and tested by: pho (*) Requested by: bde (**) Sponsored by: The FreeBSD Foundation MFC after: 2 weeks Approved by: re (gjb) Modified: head/sys/kern/kern_sig.c head/sys/kern/kern_thread.c head/sys/kern/subr_sleepqueue.c head/sys/sys/proc.h head/sys/sys/signalvar.h Modified: head/sys/kern/kern_sig.c ============================================================================== --- head/sys/kern/kern_sig.c Sun Jul 3 17:52:21 2016 (r302327) +++ head/sys/kern/kern_sig.c Sun Jul 3 18:19:48 2016 (r302328) @@ -1251,6 +1251,7 @@ kern_sigtimedwait(struct thread *td, sig mtx_lock(&ps->ps_mtx); sig = cursig(td); mtx_unlock(&ps->ps_mtx); + KASSERT(sig >= 0, ("sig %d", sig)); if (sig != 0 && SIGISMEMBER(waitset, sig)) { if (sigqueue_get(&td->td_sigqueue, sig, ksi) != 0 || sigqueue_get(&p->p_sigqueue, sig, ksi) != 0) { @@ -1512,8 +1513,10 @@ kern_sigsuspend(struct thread *td, sigse /* void */; thread_suspend_check(0); mtx_lock(&p->p_sigacts->ps_mtx); - while ((sig = cursig(td)) != 0) + while ((sig = cursig(td)) != 0) { + KASSERT(sig >= 0, ("sig %d", sig)); has_sig += postsig(sig); + } mtx_unlock(&p->p_sigacts->ps_mtx); } PROC_UNLOCK(p); @@ -2476,11 +2479,9 @@ sig_suspend_threads(struct thread *td, s */ KASSERT(!TD_IS_SUSPENDED(td2), ("thread with deferred stops suspended")); - if ((td2->td_flags & (TDF_SERESTART | - TDF_SEINTR)) != 0 && sending) { - wakeup_swapper |= sleepq_abort(td, - (td2->td_flags & TDF_SERESTART) - != 0 ? ERESTART : EINTR); + if (TD_SBDRY_INTR(td2) && sending) { + wakeup_swapper |= sleepq_abort(td2, + TD_SBDRY_ERRNO(td2)); } } else if (!TD_IS_SUSPENDED(td2)) { thread_suspend_one(td2); @@ -2628,7 +2629,7 @@ sigdeferstop_curr_flags(int cflags) * accesses below. */ int -sigdeferstop(int mode) +sigdeferstop_impl(int mode) { struct thread *td; int cflags, nflags; @@ -2655,11 +2656,11 @@ sigdeferstop(int mode) panic("sigdeferstop: invalid mode %x", mode); break; } - if (cflags != nflags) { - thread_lock(td); - td->td_flags = (td->td_flags & ~cflags) | nflags; - thread_unlock(td); - } + if (cflags == nflags) + return (SIGDEFERSTOP_VAL_NCHG); + thread_lock(td); + td->td_flags = (td->td_flags & ~cflags) | nflags; + thread_unlock(td); return (cflags); } @@ -2670,11 +2671,12 @@ sigdeferstop(int mode) * either via ast() or a subsequent interruptible sleep. */ void -sigallowstop(int prev) +sigallowstop_impl(int prev) { struct thread *td; int cflags; + KASSERT(prev != SIGDEFERSTOP_VAL_NCHG, ("failed sigallowstop")); KASSERT((prev & ~(TDF_SBDRY | TDF_SEINTR | TDF_SERESTART)) == 0, ("sigallowstop: incorrect previous mode %x", prev)); td = curthread; @@ -2835,6 +2837,11 @@ issignal(struct thread *td) (p->p_pgrp->pg_jobc == 0 && prop & SA_TTYSTOP)) break; /* == ignore */ + if (TD_SBDRY_INTR(td)) { + KASSERT((td->td_flags & TDF_SBDRY) != 0, + ("lost TDF_SBDRY")); + return (-1); + } mtx_unlock(&ps->ps_mtx); WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, &p->p_mtx.lock_object, "Catching SIGSTOP"); Modified: head/sys/kern/kern_thread.c ============================================================================== --- head/sys/kern/kern_thread.c Sun Jul 3 17:52:21 2016 (r302327) +++ head/sys/kern/kern_thread.c Sun Jul 3 18:19:48 2016 (r302328) @@ -894,7 +894,7 @@ thread_suspend_check(int return_instead) { struct thread *td; struct proc *p; - int wakeup_swapper, r; + int wakeup_swapper; td = curthread; p = td->td_proc; @@ -927,21 +927,10 @@ thread_suspend_check(int return_instead) if ((td->td_flags & TDF_SBDRY) != 0) { KASSERT(return_instead, ("TDF_SBDRY set for unsafe thread_suspend_check")); - switch (td->td_flags & (TDF_SEINTR | TDF_SERESTART)) { - case 0: - r = 0; - break; - case TDF_SEINTR: - r = EINTR; - break; - case TDF_SERESTART: - r = ERESTART; - break; - default: - panic("both TDF_SEINTR and TDF_SERESTART"); - break; - } - return (r); + KASSERT((td->td_flags & (TDF_SEINTR | TDF_SERESTART)) != + (TDF_SEINTR | TDF_SERESTART), + ("both TDF_SEINTR and TDF_SERESTART")); + return (TD_SBDRY_INTR(td) ? TD_SBDRY_ERRNO(td) : 0); } /* Modified: head/sys/kern/subr_sleepqueue.c ============================================================================== --- head/sys/kern/subr_sleepqueue.c Sun Jul 3 17:52:21 2016 (r302327) +++ head/sys/kern/subr_sleepqueue.c Sun Jul 3 18:19:48 2016 (r302328) @@ -453,7 +453,16 @@ sleepq_catch_signals(void *wchan, int pr ps = p->p_sigacts; mtx_lock(&ps->ps_mtx); sig = cursig(td); - if (sig == 0) { + if (sig == -1) { + mtx_unlock(&ps->ps_mtx); + KASSERT((td->td_flags & TDF_SBDRY) != 0, ("lost TDF_SBDRY")); + KASSERT(TD_SBDRY_INTR(td), + ("lost TDF_SERESTART of TDF_SEINTR")); + KASSERT((td->td_flags & (TDF_SEINTR | TDF_SERESTART)) != + (TDF_SEINTR | TDF_SERESTART), + ("both TDF_SEINTR and TDF_SERESTART")); + ret = TD_SBDRY_ERRNO(td); + } else if (sig == 0) { mtx_unlock(&ps->ps_mtx); ret = thread_suspend_check(1); MPASS(ret == 0 || ret == EINTR || ret == ERESTART); Modified: head/sys/sys/proc.h ============================================================================== --- head/sys/sys/proc.h Sun Jul 3 17:52:21 2016 (r302327) +++ head/sys/sys/proc.h Sun Jul 3 18:19:48 2016 (r302328) @@ -511,6 +511,11 @@ do { \ #define TD_SET_RUNQ(td) (td)->td_state = TDS_RUNQ #define TD_SET_CAN_RUN(td) (td)->td_state = TDS_CAN_RUN +#define TD_SBDRY_INTR(td) \ + (((td)->td_flags & (TDF_SEINTR | TDF_SERESTART)) != 0) +#define TD_SBDRY_ERRNO(td) \ + (((td)->td_flags & TDF_SEINTR) != 0 ? EINTR : ERESTART) + /* * Process structure. */ Modified: head/sys/sys/signalvar.h ============================================================================== --- head/sys/sys/signalvar.h Sun Jul 3 17:52:21 2016 (r302327) +++ head/sys/sys/signalvar.h Sun Jul 3 18:19:48 2016 (r302328) @@ -337,9 +337,29 @@ extern struct mtx sigio_lock; #define SIGDEFERSTOP_EINTR 3 /* ignore STOPs, return EINTR */ #define SIGDEFERSTOP_ERESTART 4 /* ignore STOPs, return ERESTART */ +#define SIGDEFERSTOP_VAL_NCHG (-1) /* placeholder indicating no state change */ +int sigdeferstop_impl(int mode); +void sigallowstop_impl(int prev); + +static inline int +sigdeferstop(int mode) +{ + + if (mode == SIGDEFERSTOP_NOP) + return (SIGDEFERSTOP_VAL_NCHG); + return (sigdeferstop_impl(mode)); +} + +static inline void +sigallowstop(int prev) +{ + + if (prev == SIGDEFERSTOP_VAL_NCHG) + return; + sigallowstop_impl(prev); +} + int cursig(struct thread *td); -int sigdeferstop(int mode); -void sigallowstop(int prev); void execsigs(struct proc *p); void gsignal(int pgid, int sig, ksiginfo_t *ksi); void killproc(struct proc *p, char *why);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201607031819.u63IJm79075708>