From owner-svn-src-head@freebsd.org Tue Jul 19 18:31:20 2016 Return-Path: Delivered-To: svn-src-head@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 B16BAB9DCF2; Tue, 19 Jul 2016 18:31:20 +0000 (UTC) (envelope-from rrs@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 8A3E915DD; Tue, 19 Jul 2016 18:31:20 +0000 (UTC) (envelope-from rrs@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id u6JIVJc0007543; Tue, 19 Jul 2016 18:31:19 GMT (envelope-from rrs@FreeBSD.org) Received: (from rrs@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id u6JIVJWq007542; Tue, 19 Jul 2016 18:31:19 GMT (envelope-from rrs@FreeBSD.org) Message-Id: <201607191831.u6JIVJWq007542@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: rrs set sender to rrs@FreeBSD.org using -f From: Randall Stewart Date: Tue, 19 Jul 2016 18:31:19 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r303037 - head/sys/kern X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 19 Jul 2016 18:31:20 -0000 Author: rrs Date: Tue Jul 19 18:31:19 2016 New Revision: 303037 URL: https://svnweb.freebsd.org/changeset/base/303037 Log: This reverts out Gleb's changes and adds three small fixes that I think closes up the races Gleb was looking for. This is running quite nicely in Netflix and now no longer causes TCP-tcb leaks. Differential Revision: 7135 Modified: head/sys/kern/kern_timeout.c Modified: head/sys/kern/kern_timeout.c ============================================================================== --- head/sys/kern/kern_timeout.c Tue Jul 19 18:15:22 2016 (r303036) +++ head/sys/kern/kern_timeout.c Tue Jul 19 18:31:19 2016 (r303037) @@ -1050,7 +1050,7 @@ callout_reset_sbt_on(struct callout *c, */ if (c->c_lock != NULL && !cc_exec_cancel(cc, direct)) cancelled = cc_exec_cancel(cc, direct) = true; - if (cc_exec_waiting(cc, direct)) { + if (cc_exec_waiting(cc, direct) || cc_exec_drain(cc, direct)) { /* * Someone has called callout_drain to kill this * callout. Don't reschedule. @@ -1166,7 +1166,7 @@ _callout_stop_safe(struct callout *c, in struct callout_cpu *cc, *old_cc; struct lock_class *class; int direct, sq_locked, use_lock; - int cancelled, not_on_a_list; + int not_on_a_list; if ((flags & CS_DRAIN) != 0) WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, c->c_lock, @@ -1234,17 +1234,47 @@ again: panic("migration should not happen"); #endif } - + if ((drain != NULL) && (c->c_iflags & CALLOUT_PENDING) && + (cc_exec_curr(cc, direct) != c)) { + /* + * This callout is executing and we are draining. + * The only way this can happen is if its also + * been rescheduled to run on one thread *and* asked to drain + * on this thread (at the same time it is waiting to execute). + */ + if ((c->c_iflags & CALLOUT_PROCESSED) == 0) { + if (cc_exec_next(cc) == c) + cc_exec_next(cc) = LIST_NEXT(c, c_links.le); + LIST_REMOVE(c, c_links.le); + } else { + TAILQ_REMOVE(&cc->cc_expireq, c, c_links.tqe); + } + c->c_iflags &= ~CALLOUT_PENDING; + c->c_flags &= ~CALLOUT_ACTIVE; + } /* - * If the callout is running, try to stop it or drain it. + * If the callout isn't pending, it's not on the queue, so + * don't attempt to remove it from the queue. We can try to + * stop it by other means however. */ - if (cc_exec_curr(cc, direct) == c) { + if (!(c->c_iflags & CALLOUT_PENDING)) { /* - * Succeed we to stop it or not, we must clear the - * active flag - this is what API users expect. + * If it wasn't on the queue and it isn't the current + * callout, then we can't stop it, so just bail. + * It probably has already been run (if locking + * is properly done). You could get here if the caller + * calls stop twice in a row for example. The second + * call would fall here without CALLOUT_ACTIVE set. */ c->c_flags &= ~CALLOUT_ACTIVE; - + if (cc_exec_curr(cc, direct) != c) { + CTR3(KTR_CALLOUT, "failed to stop %p func %p arg %p", + c, c->c_func, c->c_arg); + CC_UNLOCK(cc); + if (sq_locked) + sleepq_release(&cc_exec_waiting(cc, direct)); + return (-1); + } if ((flags & CS_DRAIN) != 0) { /* * The current callout is running (or just @@ -1278,7 +1308,6 @@ again: old_cc = cc; goto again; } - /* * Migration could be cancelled here, but * as long as it is still not sure when it @@ -1362,6 +1391,8 @@ again: cc_exec_drain(cc, direct) = drain; } CC_UNLOCK(cc); + if (drain) + return(0); return ((flags & CS_EXECUTING) != 0); } CTR3(KTR_CALLOUT, "failed to stop %p func %p arg %p", @@ -1369,20 +1400,12 @@ again: if (drain) { cc_exec_drain(cc, direct) = drain; } - KASSERT(!sq_locked, ("sleepqueue chain still locked")); - cancelled = ((flags & CS_EXECUTING) != 0); - } else - cancelled = 1; - - if (sq_locked) - sleepq_release(&cc_exec_waiting(cc, direct)); - - if ((c->c_iflags & CALLOUT_PENDING) == 0) { - CTR3(KTR_CALLOUT, "failed to stop %p func %p arg %p", - c, c->c_func, c->c_arg); CC_UNLOCK(cc); + KASSERT(!sq_locked, ("sleepqueue chain still locked")); return (0); } + if (sq_locked) + sleepq_release(&cc_exec_waiting(cc, direct)); c->c_iflags &= ~CALLOUT_PENDING; c->c_flags &= ~CALLOUT_ACTIVE; @@ -1400,7 +1423,7 @@ again: } callout_cc_del(c, cc); CC_UNLOCK(cc); - return (cancelled); + return (1); } void @@ -1615,7 +1638,6 @@ SYSCTL_PROC(_kern, OID_AUTO, callout_sta CTLTYPE_INT | CTLFLAG_RW | CTLFLAG_MPSAFE, 0, 0, sysctl_kern_callout_stat, "I", "Dump immediate statistic snapshot of the scheduled callouts"); - #ifdef DDB static void _show_callout(struct callout *c)