From owner-svn-src-all@freebsd.org Mon Jun 27 23:46:36 2016 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 10A38B81817; Mon, 27 Jun 2016 23:46:36 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id AF8C8202F; Mon, 27 Jun 2016 23:46:35 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kib@localhost [127.0.0.1]) by kib.kiev.ua (8.15.2/8.15.2) with ESMTPS id u5RNkO5A032930 (version=TLSv1 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Tue, 28 Jun 2016 02:46:24 +0300 (EEST) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua u5RNkO5A032930 Received: (from kostik@localhost) by tom.home (8.15.2/8.15.2/Submit) id u5RNkO8j032929; Tue, 28 Jun 2016 02:46:24 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Tue, 28 Jun 2016 02:46:24 +0300 From: Konstantin Belousov To: Ngie Cooper Cc: jilles@freebsd.org, "src-committers@freebsd.org" , "svn-src-all@freebsd.org" , "svn-src-head@freebsd.org" Subject: Re: svn commit: r302216 - in head/sys: kern nlm Message-ID: <20160627234624.GV38613@kib.kiev.ua> References: <201606262008.u5QK8gTx042729@repo.freebsd.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="Wfe1KbQWcwuymTys" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.6.1 (2016-04-27) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.1 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on tom.home X-Content-Filtered-By: Mailman/MimeDel 2.1.22 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.22 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: Mon, 27 Jun 2016 23:46:36 -0000 --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 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--