When callout_reset_on() cannot immediately migrate a callout since it

is running on other cpu, the CALLOUT_PENDING flag is temporarily
cleared. Then, callout_stop() on this, in fact active, callout fails
because CALLOUT_PENDING is not set, and callout_stop() returns 0.

Now, in sleepq_check_timeout(), the failed callout_stop() causes the
sleepq code to execute mi_switch() without even setting the wmesg,
since the switch-out is supposed to be transient. In fact, the thread
is put off the CPU for full timeout interval, instead of being put on
runq immediately.  Until timeout fires, the process is unkillable for
obvious reasons.

Fix this by marking the migrating callouts with CALLOUT_DFRMIGRATION
flag. The flag is cleared by callout_stop_safe() when the function
detects a migration, besides returning the success. The softclock()
rechecks the flag for migrating callout and cancels its execution if
the flag was cleared meantime.

PR:	 misc/166340
Reported, debugging traces provided and tested by:
	Christian Esken <christian.esken trivago com>
Reviewed by:	 avg, jhb
MFC after:	 1 week

1 files changed, 36 insertions, 0 deletions

diff --git a/sys/kern/kern_timeout.c b/sys/kern/kern_timeout.c
index 5e1d4f3..3abb1ed 100644
--- a/sys/kern/kern_timeout.c
+++ b/sys/kern/kern_timeout.c
@@ -645,6 +645,32 @@ softclock(void *arg)
 					cc_cme_cleanup(cc);
 
 					/*
+					 * Handle deferred callout stops
+					 */
+					if ((c->c_flags & CALLOUT_DFRMIGRATION)
+					    == 0) {
+						CTR3(KTR_CALLOUT,
+					"deferred cancelled %p func %p arg %p",
+						    c, new_func, new_arg);
+						if (cc->cc_next == c) {
+							cc->cc_next =
+							    TAILQ_NEXT(c,
+							    c_links.tqe);
+						}
+						if (c->c_flags &
+						    CALLOUT_LOCAL_ALLOC) {
+							c->c_func = NULL;
+							SLIST_INSERT_HEAD(
+							    &cc->cc_callfree, c,
+							    c_links.sle);
+						}
+						goto nextc;
+					} else {
+						c->c_flags &= ~
+						    CALLOUT_DFRMIGRATION;
+					}
+
+					/*
 					 * It should be assert here that the
 					 * callout is not destroyed but that
 					 * is not easy.
@@ -659,6 +685,9 @@ softclock(void *arg)
 					panic("migration should not happen");
 #endif
 				}
+#ifdef SMP
+nextc:
+#endif
 				steps = 0;
 				c = cc->cc_next;
 			}
@@ -814,6 +843,7 @@ callout_reset_on(struct callout *c, int to_ticks, void (*ftn)(void *),
 			cc->cc_migration_ticks = to_ticks;
 			cc->cc_migration_func = ftn;
 			cc->cc_migration_arg = arg;
+			c->c_flags |= CALLOUT_DFRMIGRATION;
 			CTR5(KTR_CALLOUT,
 		    "migration of %p func %p arg %p in %d to %u deferred",
 			    c, c->c_func, c->c_arg, to_ticks, cpu);
@@ -984,6 +1014,12 @@ again:
 			CC_UNLOCK(cc);
 			KASSERT(!sq_locked, ("sleepqueue chain locked"));
 			return (1);
+		} else if ((c->c_flags & CALLOUT_DFRMIGRATION) != 0) {
+			c->c_flags &= ~CALLOUT_DFRMIGRATION;
+			CTR3(KTR_CALLOUT, "postponing stop %p func %p arg %p",
+			    c, c->c_func, c->c_arg);
+			CC_UNLOCK(cc);
+			return (1);
 		}
 		CTR3(KTR_CALLOUT, "failed to stop %p func %p arg %p",
 		    c, c->c_func, c->c_arg);