From owner-svn-src-all@freebsd.org Wed Mar 2 18:46:19 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 429E9AC2E01; Wed, 2 Mar 2016 18:46:19 +0000 (UTC) (envelope-from kib@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 1D7E71EA1; Wed, 2 Mar 2016 18:46:19 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id u22IkIEL010864; Wed, 2 Mar 2016 18:46:18 GMT (envelope-from kib@FreeBSD.org) Received: (from kib@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id u22IkHWM010861; Wed, 2 Mar 2016 18:46:17 GMT (envelope-from kib@FreeBSD.org) Message-Id: <201603021846.u22IkHWM010861@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: kib set sender to kib@FreeBSD.org using -f From: Konstantin Belousov Date: Wed, 2 Mar 2016 18:46:17 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r296320 - in head/sys: kern 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.20 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, 02 Mar 2016 18:46:19 -0000 Author: kib Date: Wed Mar 2 18:46:17 2016 New Revision: 296320 URL: https://svnweb.freebsd.org/changeset/base/296320 Log: If callout_stop_safe() noted that the callout is currently executing, but next invocation is cancelled while migrating, sleepq_check_timeout() needs to be informed that the callout is stopped. Otherwise the thread switches off CPU and never become runnable, since running callout could have already raced with us, while the migrating and cancelled callout could be one which is expected to set TDP_TIMOFAIL flag for us. This contradicts with the expected behaviour of callout_stop() for other callers, which e.g. decrement references from the callout callbacks. Add a new flag CS_MIGRBLOCK requesting report of the situation as 'successfully stopped'. Reviewed by: jhb (previous version) Tested by: cognet, pho PR: 200992 Sponsored by: The FreeBSD Foundation MFC after: 2 weeks Differential revision: https://reviews.freebsd.org/D5221 Modified: head/sys/kern/kern_timeout.c head/sys/kern/subr_sleepqueue.c head/sys/sys/callout.h Modified: head/sys/kern/kern_timeout.c ============================================================================== --- head/sys/kern/kern_timeout.c Wed Mar 2 16:36:24 2016 (r296319) +++ head/sys/kern/kern_timeout.c Wed Mar 2 18:46:17 2016 (r296320) @@ -1155,14 +1155,14 @@ callout_schedule(struct callout *c, int } int -_callout_stop_safe(struct callout *c, int safe, void (*drain)(void *)) +_callout_stop_safe(struct callout *c, int flags, void (*drain)(void *)) { struct callout_cpu *cc, *old_cc; struct lock_class *class; int direct, sq_locked, use_lock; int not_on_a_list; - if (safe) + if ((flags & CS_DRAIN) != 0) WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, c->c_lock, "calling %s", __func__); @@ -1170,7 +1170,7 @@ _callout_stop_safe(struct callout *c, in * Some old subsystems don't hold Giant while running a callout_stop(), * so just discard this check for the moment. */ - if (!safe && c->c_lock != NULL) { + if ((flags & CS_DRAIN) == 0 && c->c_lock != NULL) { if (c->c_lock == &Giant.lock_object) use_lock = mtx_owned(&Giant); else { @@ -1253,7 +1253,7 @@ again: return (-1); } - if (safe) { + if ((flags & CS_DRAIN) != 0) { /* * The current callout is running (or just * about to run) and blocking is allowed, so @@ -1370,7 +1370,7 @@ again: cc_exec_drain(cc, direct) = drain; } CC_UNLOCK(cc); - return (0); + return ((flags & CS_MIGRBLOCK) != 0); } CTR3(KTR_CALLOUT, "failed to stop %p func %p arg %p", c, c->c_func, c->c_arg); Modified: head/sys/kern/subr_sleepqueue.c ============================================================================== --- head/sys/kern/subr_sleepqueue.c Wed Mar 2 16:36:24 2016 (r296319) +++ head/sys/kern/subr_sleepqueue.c Wed Mar 2 18:46:17 2016 (r296320) @@ -586,7 +586,8 @@ sleepq_check_timeout(void) * another CPU, so synchronize with it to avoid having it * accidentally wake up a subsequent sleep. */ - else if (callout_stop(&td->td_slpcallout) == 0) { + else if (_callout_stop_safe(&td->td_slpcallout, CS_MIGRBLOCK, NULL) + == 0) { td->td_flags |= TDF_TIMEOUT; TD_SET_SLEEPING(td); mi_switch(SW_INVOL | SWT_SLEEPQTIMO, NULL); Modified: head/sys/sys/callout.h ============================================================================== --- head/sys/sys/callout.h Wed Mar 2 16:36:24 2016 (r296319) +++ head/sys/sys/callout.h Wed Mar 2 18:46:17 2016 (r296320) @@ -62,6 +62,12 @@ struct callout_handle { struct callout *callout; }; +/* Flags for callout_stop_safe() */ +#define CS_DRAIN 0x0001 /* callout_drain(), wait allowed */ +#define CS_MIGRBLOCK 0x0002 /* Block migration, return value + indicates that the callout was + executing */ + #ifdef _KERNEL /* * Note the flags field is actually *two* fields. The c_flags @@ -81,7 +87,7 @@ struct callout_handle { */ #define callout_active(c) ((c)->c_flags & CALLOUT_ACTIVE) #define callout_deactivate(c) ((c)->c_flags &= ~CALLOUT_ACTIVE) -#define callout_drain(c) _callout_stop_safe(c, 1, NULL) +#define callout_drain(c) _callout_stop_safe(c, CS_DRAIN, NULL) void callout_init(struct callout *, int); void _callout_init_lock(struct callout *, struct lock_object *, int); #define callout_init_mtx(c, mtx, flags) \