Date: Tue, 28 Jun 2016 02:46:24 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: Ngie Cooper <yaneurabeya@gmail.com> Cc: jilles@freebsd.org, "src-committers@freebsd.org" <src-committers@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "svn-src-head@freebsd.org" <svn-src-head@freebsd.org> Subject: Re: svn commit: r302216 - in head/sys: kern nlm Message-ID: <20160627234624.GV38613@kib.kiev.ua> In-Reply-To: <CAGHfRMDsOFHNHnMyT5jQo5SX9_9rEYyOubCLYb%2BcYp8BG0hAFQ@mail.gmail.com> References: <201606262008.u5QK8gTx042729@repo.freebsd.org> <CAGHfRMDsOFHNHnMyT5jQo5SX9_9rEYyOubCLYb%2BcYp8BG0hAFQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--Wfe1KbQWcwuymTys Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Mon, Jun 27, 2016 at 01:18:48PM -0700, Ngie Cooper wrote: > On Sun, Jun 26, 2016 at 1:08 PM, Konstantin Belousov <kib@freebsd.org> wrote: > > Author: kib > > Date: Sun Jun 26 20:08:42 2016 > > New Revision: 302216 > > URL: https://svnweb.freebsd.org/changeset/base/302216 > > > > Log: > > When sleeping waiting for either local or remote advisory lock, > > interrupt sleeps with the ERESTART on the suspension attempts. > > Otherwise, single-threading requests are deferred until the locks are > > granted for NFS files, which causes hangs. > > > > When retrying local registration of the remotely-granted adv lock, > > allow full suspension and check for suspension, for usual reasons. > > > > Reported by: markj, pho > > Reviewed by: jilles > > Tested by: pho > > Sponsored by: The FreeBSD Foundation > > MFC after: 2 weeks > > Approved by: re (gjb) > > One of the NetBSD tests seems to be failing now: > https://jenkins.freebsd.org/job/FreeBSD_HEAD/334/testReport/junit/sys.kern/lockf_test/randlock/ > . Indeed. The reason is that TDF_SBDRY + TDF_SERESTART (or TDF_SEINTR) combination should be treated almost as if TDF_SBDRY is not set, at least for the purposes of filtering out stops. It must not allow the real stop to occur at a protected sleep point, though. The committed revision lacks updates to kern_sig.c to kick the sleeping thread in TDF_SERESTART condition to make it runnable to reach boundary. I attached the distilled case from the NetBSD test below. Another good test case to illustrate that is to do in one terminal echo 123 >/tmp/1 lockf /tmp/1 sleep 100000 and in another lockf /tmp/1 date ^Z This should have failed in similar way on NFS only, before the changes. The patch attached fixed both cases for me. --Wfe1KbQWcwuymTys Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="1.patch" diff --git a/sys/kern/kern_sig.c b/sys/kern/kern_sig.c index 424f316..059103dc 100644 --- a/sys/kern/kern_sig.c +++ b/sys/kern/kern_sig.c @@ -107,7 +107,7 @@ static int killpg1(struct thread *td, int sig, int pgid, int all, static int issignal(struct thread *td); static int sigprop(int sig); static void tdsigwakeup(struct thread *, int, sig_t, int); -static void sig_suspend_threads(struct thread *, struct proc *, int); +static int sig_suspend_threads(struct thread *, struct proc *, int); static int filt_sigattach(struct knote *kn); static void filt_sigdetach(struct knote *kn); static int filt_signal(struct knote *kn, long hint); @@ -2327,7 +2327,7 @@ tdsendsignal(struct proc *p, struct thread *td, int sig, ksiginfo_t *ksi) p->p_flag |= P_STOPPED_SIG; p->p_xsig = sig; PROC_SLOCK(p); - sig_suspend_threads(td, p, 1); + wakeup_swapper = sig_suspend_threads(td, p, 1); if (p->p_numthreads == p->p_suspcount) { /* * only thread sending signal to another @@ -2341,6 +2341,8 @@ tdsendsignal(struct proc *p, struct thread *td, int sig, ksiginfo_t *ksi) sigqueue_delete_proc(p, p->p_xsig); } else PROC_SUNLOCK(p); + if (wakeup_swapper) + kick_proc0(); goto out; } } else { @@ -2421,7 +2423,8 @@ tdsigwakeup(struct thread *td, int sig, sig_t action, int intrval) * Don't awaken a sleeping thread for SIGSTOP if the * STOP signal is deferred. */ - if ((prop & SA_STOP) && (td->td_flags & TDF_SBDRY)) + if ((prop & SA_STOP) != 0 && (td->td_flags & (TDF_SBDRY | + TDF_SERESTART | TDF_SEINTR)) == TDF_SBDRY) goto out; /* @@ -2449,14 +2452,16 @@ out: kick_proc0(); } -static void +static int sig_suspend_threads(struct thread *td, struct proc *p, int sending) { struct thread *td2; + int wakeup_swapper; PROC_LOCK_ASSERT(p, MA_OWNED); PROC_SLOCK_ASSERT(p, MA_OWNED); + wakeup_swapper = 0; FOREACH_THREAD_IN_PROC(p, td2) { thread_lock(td2); td2->td_flags |= TDF_ASTPENDING | TDF_NEEDSUSPCHK; @@ -2465,11 +2470,18 @@ sig_suspend_threads(struct thread *td, struct proc *p, int sending) if (td2->td_flags & TDF_SBDRY) { /* * Once a thread is asleep with - * TDF_SBDRY set, it should never + * TDF_SBDRY and without TDF_SERESTART + * or TDF_SEINTR set, it should never * become suspended due to this check. */ 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); + } } else if (!TD_IS_SUSPENDED(td2)) { thread_suspend_one(td2); } @@ -2483,6 +2495,7 @@ sig_suspend_threads(struct thread *td, struct proc *p, int sending) } thread_unlock(td2); } + return (wakeup_swapper); } int @@ -2705,7 +2718,8 @@ issignal(struct thread *td) SIGSETOR(sigpending, p->p_sigqueue.sq_signals); SIGSETNAND(sigpending, td->td_sigmask); - if (p->p_flag & P_PPWAIT || td->td_flags & TDF_SBDRY) + if ((p->p_flag & P_PPWAIT) != 0 || (td->td_flags & + (TDF_SBDRY | TDF_SERESTART | TDF_SEINTR)) == TDF_SBDRY) SIG_STOPSIGMASK(sigpending); if (SIGISEMPTY(sigpending)) /* no signal to send */ return (0); --Wfe1KbQWcwuymTys--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20160627234624.GV38613>