OS-7034 ctxops should use stack ordering for save/restore

Reviewed by: John Levon <john.levon@joyent.com>
Reviewed by: Robert Mustacchi <rm@joyent.com>
Reviewed by: Jerry Jelinek <jerry.jelinek@joyent.com>
Approved by: Jerry Jelinek <jerry.jelinek@joyent.com>

5 files changed, 202 insertions, 77 deletions

diff --git a/usr/src/cmd/mdb/common/modules/genunix/ctxop.c b/usr/src/cmd/mdb/common/modules/genunix/ctxop.c
index 302ffa03c9..bf2815dfdb 100644
--- a/usr/src/cmd/mdb/common/modules/genunix/ctxop.c
+++ b/usr/src/cmd/mdb/common/modules/genunix/ctxop.c
@@ -23,56 +23,97 @@
  * Copyright (c) 2000 by Sun Microsystems, Inc.
  * All rights reserved.
  */
+/*
+ * Copyright 2018 Joyent, Inc.
+ */
 
-#pragma ident	"%Z%%M%	%I%	%E% SMI"
-
-#include <sys/mdb_modapi.h>
-#include <sys/thread.h>
+#include <mdb/mdb_modapi.h>
+#include <mdb/mdb_ctf.h>
 #include "ctxop.h"
 
+struct ctxop_walk_state {
+	uintptr_t	cws_head;
+	uint_t		cws_next_offset;
+};
+
 int
 ctxop_walk_init(mdb_walk_state_t *wsp)
 {
-	kthread_t thread, *tp = &thread;
+	struct ctxop_walk_state *priv;
+	int offset;
+	uintptr_t addr;
 
 	if (wsp->walk_addr == NULL) {
 		mdb_warn("must specify thread for ctxop walk\n");
 		return (WALK_ERR);
 	}
-	if (mdb_vread(tp, sizeof (*tp), wsp->walk_addr) == -1) {
-		mdb_warn("failed to read thread at %p", wsp->walk_addr);
+
+	offset = mdb_ctf_offsetof_by_name("kthread_t", "t_ctx");
+	if (offset == -1)
 		return (WALK_ERR);
+
+	if (mdb_vread(&addr, sizeof (addr),
+	    wsp->walk_addr + offset) != sizeof (addr)) {
+		mdb_warn("failed to read thread %p", wsp->walk_addr);
+		return (WALK_ERR);
+	}
+
+	/* No further work for threads with a NULL t_ctx */
+	if (addr == 0) {
+		wsp->walk_data = NULL;
+		return (WALK_DONE);
 	}
 
-	wsp->walk_data = mdb_alloc(sizeof (ctxop_t), UM_SLEEP);
-	wsp->walk_addr = (uintptr_t)tp->t_ctx;
+	/* rely on CTF for the offset of the 'next' pointer */
+	offset = mdb_ctf_offsetof_by_name("struct ctxop", "next");
+	if (offset == -1)
+		return (WALK_ERR);
+
+	priv = mdb_alloc(sizeof (*priv), UM_SLEEP);
+	priv->cws_head = addr;
+	priv->cws_next_offset = (uint_t)offset;
 
+	wsp->walk_data = priv;
+	wsp->walk_addr = addr;
 	return (WALK_NEXT);
 }
 
 int
 ctxop_walk_step(mdb_walk_state_t *wsp)
 {
+	struct ctxop_walk_state *priv = wsp->walk_data;
+	uintptr_t next;
 	int status;
 
-	if (wsp->walk_addr == NULL)
-		return (WALK_DONE);
-
-	if (mdb_vread(wsp->walk_data,
-	    sizeof (ctxop_t), wsp->walk_addr) == -1) {
-		mdb_warn("failed to read ctxop at %p", wsp->walk_addr);
+	if (mdb_vread(&next, sizeof (next),
+	    wsp->walk_addr + priv->cws_next_offset) == -1) {
+		mdb_warn("failed to read ctxop`next at %p",
+		    wsp->walk_addr + priv->cws_next_offset);
 		return (WALK_DONE);
 	}
 
-	status = wsp->walk_callback(wsp->walk_addr, wsp->walk_data,
-	    wsp->walk_cbdata);
+	status = wsp->walk_callback(wsp->walk_addr, NULL, wsp->walk_cbdata);
 
-	wsp->walk_addr = (uintptr_t)(((ctxop_t *)wsp->walk_data)->next);
+	if (status == WALK_NEXT) {
+		/*
+		 * If a NULL terminator or a loop back to the head element is
+		 * encountered, the walk is done.
+		 */
+		if (next == 0 || next == priv->cws_head) {
+			status = WALK_DONE;
+		}
+	}
+
+	wsp->walk_addr = next;
 	return (status);
 }
 
 void
 ctxop_walk_fini(mdb_walk_state_t *wsp)
 {
-	mdb_free(wsp->walk_data, sizeof (ctxop_t));
+	struct ctxop_walk_state *priv = wsp->walk_data;
+
+	if (priv != NULL) {
+		mdb_free(priv, sizeof (*priv));
+	}
 }
diff --git a/usr/src/cmd/mdb/common/modules/genunix/ctxop.h b/usr/src/cmd/mdb/common/modules/genunix/ctxop.h
index 529db3844c..0d0af0b931 100644
--- a/usr/src/cmd/mdb/common/modules/genunix/ctxop.h
+++ b/usr/src/cmd/mdb/common/modules/genunix/ctxop.h
@@ -27,8 +27,6 @@
 #ifndef	_MDB_CTXOP_H
 #define	_MDB_CTXOP_H
 
-#pragma ident	"%Z%%M%	%I%	%E% SMI"
-
 #include <mdb/mdb_modapi.h>
 
 #ifdef	__cplusplus
diff --git a/usr/src/uts/common/disp/thread.c b/usr/src/uts/common/disp/thread.c
index 0312cc0c8c..2cd11a0bce 100644
--- a/usr/src/uts/common/disp/thread.c
+++ b/usr/src/uts/common/disp/thread.c
@@ -21,7 +21,7 @@
 
 /*
  * Copyright (c) 1991, 2010, Oracle and/or its affiliates. All rights reserved.
- * Copyright (c) 2017, Joyent, Inc.  All rights reserved.
+ * Copyright (c) 2018 Joyent, Inc.
  */
 
 #include <sys/types.h>
@@ -1049,9 +1049,37 @@ installctx(
 	ctx->exit_op = exit;
 	ctx->free_op = free;
 	ctx->arg = arg;
-	ctx->next = t->t_ctx;
 	ctx->save_ts = 0;
 	ctx->restore_ts = 0;
+
+	/*
+	 * Keep ctxops in a doubly-linked list to allow traversal in both
+	 * directions.  Using only the newest-to-oldest ordering was adequate
+	 * previously, but reversing the order for restore_op actions is
+	 * necessary if later-added ctxops depends on earlier ones.
+	 *
+	 * One example of such a dependency:  Hypervisor software handling the
+	 * guest FPU expects that it save FPU state prior to host FPU handling
+	 * and consequently handle the guest logic _after_ the host FPU has
+	 * been restored.
+	 *
+	 * The t_ctx member points to the most recently added ctxop or is NULL
+	 * if no ctxops are associated with the thread.  The 'next' pointers
+	 * form a loop of the ctxops in newest-to-oldest order.  The 'prev'
+	 * pointers form a loop in the reverse direction, where t_ctx->prev is
+	 * the oldest entry associated with the thread.
+	 */
+	if (t->t_ctx == NULL) {
+		ctx->next = ctx;
+		ctx->prev = ctx;
+	} else {
+		struct ctxop *head = t->t_ctx, *tail = t->t_ctx->prev;
+
+		ctx->next = head;
+		ctx->prev = tail;
+		head->prev = ctx;
+		tail->next = ctx;
+	}
 	t->t_ctx = ctx;
 }
 
@@ -1069,7 +1097,7 @@ removectx(
 	void	(*exit)(void *),
 	void	(*free)(void *, int))
 {
-	struct ctxop *ctx, *prev_ctx;
+	struct ctxop *ctx, *head;
 
 	/*
 	 * The incoming kthread_t (which is the thread for which the
@@ -1094,17 +1122,31 @@ removectx(
 	 * and the target thread from racing with each other during lwp exit.
 	 */
 	mutex_enter(&t->t_ctx_lock);
-	prev_ctx = NULL;
 	kpreempt_disable();
-	for (ctx = t->t_ctx; ctx != NULL; ctx = ctx->next) {
+
+	if (t->t_ctx == NULL) {
+		mutex_exit(&t->t_ctx_lock);
+		kpreempt_enable();
+		return (0);
+	}
+
+	ctx = head = t->t_ctx;
+	do {
 		if (ctx->save_op == save && ctx->restore_op == restore &&
 		    ctx->fork_op == fork && ctx->lwp_create_op == lwp_create &&
 		    ctx->exit_op == exit && ctx->free_op == free &&
 		    ctx->arg == arg) {
-			if (prev_ctx)
-				prev_ctx->next = ctx->next;
-			else
+			ctx->prev->next = ctx->next;
+			ctx->next->prev = ctx->prev;
+			if (ctx->next == ctx) {
+				/* last remaining item */
+				t->t_ctx = NULL;
+			} else if (ctx == t->t_ctx) {
+				/* fix up head of list */
 				t->t_ctx = ctx->next;
+			}
+			ctx->next = ctx->prev = NULL;
+
 			mutex_exit(&t->t_ctx_lock);
 			if (ctx->free_op != NULL)
 				(ctx->free_op)(ctx->arg, 0);
@@ -1112,50 +1154,70 @@ removectx(
 			kpreempt_enable();
 			return (1);
 		}
-		prev_ctx = ctx;
-	}
+
+		ctx = ctx->next;
+	} while (ctx != head);
+
 	mutex_exit(&t->t_ctx_lock);
 	kpreempt_enable();
-
 	return (0);
 }
 
 void
 savectx(kthread_t *t)
 {
-	struct ctxop *ctx;
-
 	ASSERT(t == curthread);
-	for (ctx = t->t_ctx; ctx != 0; ctx = ctx->next) {
-		if (ctx->save_op != NULL) {
-			ctx->save_ts = gethrtime_unscaled();
-			(ctx->save_op)(ctx->arg);
-		}
+
+	if (t->t_ctx != NULL) {
+		struct ctxop *ctx, *head;
+
+		/* Forward traversal */
+		ctx = head = t->t_ctx;
+		do {
+			if (ctx->save_op != NULL) {
+				ctx->save_ts = gethrtime_unscaled();
+				(ctx->save_op)(ctx->arg);
+			}
+			ctx = ctx->next;
+		} while (ctx != head);
 	}
 }
 
 void
 restorectx(kthread_t *t)
 {
-	struct ctxop *ctx;
-
 	ASSERT(t == curthread);
-	for (ctx = t->t_ctx; ctx != 0; ctx = ctx->next) {
-		if (ctx->restore_op != NULL) {
-			ctx->restore_ts = gethrtime_unscaled();
-			(ctx->restore_op)(ctx->arg);
-		}
+
+	if (t->t_ctx != NULL) {
+		struct ctxop *ctx, *tail;
+
+		/* Backward traversal (starting at the tail) */
+		ctx = tail = t->t_ctx->prev;
+		do {
+			if (ctx->restore_op != NULL) {
+				ctx->restore_ts = gethrtime_unscaled();
+				(ctx->restore_op)(ctx->arg);
+			}
+			ctx = ctx->prev;
+		} while (ctx != tail);
 	}
 }
 
 void
 forkctx(kthread_t *t, kthread_t *ct)
 {
-	struct ctxop *ctx;
-
-	for (ctx = t->t_ctx; ctx != NULL; ctx = ctx->next)
-		if (ctx->fork_op != NULL)
-			(ctx->fork_op)(t, ct);
+	if (t->t_ctx != NULL) {
+		struct ctxop *ctx, *head;
+
+		/* Forward traversal */
+		ctx = head = t->t_ctx;
+		do {
+			if (ctx->fork_op != NULL) {
+				(ctx->fork_op)(t, ct);
+			}
+			ctx = ctx->next;
+		} while (ctx != head);
+	}
 }
 
 /*
@@ -1166,11 +1228,18 @@ forkctx(kthread_t *t, kthread_t *ct)
 void
 lwp_createctx(kthread_t *t, kthread_t *ct)
 {
-	struct ctxop *ctx;
-
-	for (ctx = t->t_ctx; ctx != NULL; ctx = ctx->next)
-		if (ctx->lwp_create_op != NULL)
-			(ctx->lwp_create_op)(t, ct);
+	if (t->t_ctx != NULL) {
+		struct ctxop *ctx, *head;
+
+		/* Forward traversal */
+		ctx = head = t->t_ctx;
+		do {
+			if (ctx->lwp_create_op != NULL) {
+				(ctx->lwp_create_op)(t, ct);
+			}
+			ctx = ctx->next;
+		} while (ctx != head);
+	}
 }
 
 /*
@@ -1183,11 +1252,18 @@ lwp_createctx(kthread_t *t, kthread_t *ct)
 void
 exitctx(kthread_t *t)
 {
-	struct ctxop *ctx;
-
-	for (ctx = t->t_ctx; ctx != NULL; ctx = ctx->next)
-		if (ctx->exit_op != NULL)
-			(ctx->exit_op)(t);
+	if (t->t_ctx != NULL) {
+		struct ctxop *ctx, *head;
+
+		/* Forward traversal */
+		ctx = head = t->t_ctx;
+		do {
+			if (ctx->exit_op != NULL) {
+				(ctx->exit_op)(t);
+			}
+			ctx = ctx->next;
+		} while (ctx != head);
+	}
 }
 
 /*
@@ -1197,14 +1273,21 @@ exitctx(kthread_t *t)
 void
 freectx(kthread_t *t, int isexec)
 {
-	struct ctxop *ctx;
-
 	kpreempt_disable();
-	while ((ctx = t->t_ctx) != NULL) {
-		t->t_ctx = ctx->next;
-		if (ctx->free_op != NULL)
-			(ctx->free_op)(ctx->arg, isexec);
-		kmem_free(ctx, sizeof (struct ctxop));
+	if (t->t_ctx != NULL) {
+		struct ctxop *ctx, *head;
+
+		ctx = head = t->t_ctx;
+		t->t_ctx = NULL;
+		do {
+			struct ctxop *next = ctx->next;
+
+			if (ctx->free_op != NULL) {
+				(ctx->free_op)(ctx->arg, isexec);
+			}
+			kmem_free(ctx, sizeof (struct ctxop));
+			ctx = next;
+		} while (ctx != head);
 	}
 	kpreempt_enable();
 }
@@ -1219,17 +1302,22 @@ freectx(kthread_t *t, int isexec)
 void
 freectx_ctx(struct ctxop *ctx)
 {
-	struct ctxop *nctx;
+	struct ctxop *head = ctx;
 
 	ASSERT(ctx != NULL);
 
 	kpreempt_disable();
+
+	head = ctx;
 	do {
-		nctx = ctx->next;
-		if (ctx->free_op != NULL)
+		struct ctxop *next = ctx->next;
+
+		if (ctx->free_op != NULL) {
 			(ctx->free_op)(ctx->arg, 0);
+		}
 		kmem_free(ctx, sizeof (struct ctxop));
-	} while ((ctx = nctx) != NULL);
+		ctx = next;
+	} while (ctx != head);
 	kpreempt_enable();
 }
 
diff --git a/usr/src/uts/common/sys/thread.h b/usr/src/uts/common/sys/thread.h
index 63db0fc9e9..af9fcb75cf 100644
--- a/usr/src/uts/common/sys/thread.h
+++ b/usr/src/uts/common/sys/thread.h
@@ -25,7 +25,7 @@
  */
 
 /*
- * Copyright 2017, Joyent, Inc.
+ * Copyright 2018 Joyent, Inc.
  */
 
 #ifndef	_SYS_THREAD_H
@@ -71,7 +71,8 @@ typedef struct ctxop {
 	void	(*exit_op)(void *);	/* invoked during {thread,lwp}_exit() */
 	void	(*free_op)(void *, int); /* function which frees the context */
 	void	*arg;		/* argument to above functions, ctx pointer */
-	struct ctxop *next;	/* next context ops */
+	struct ctxop *next;		/* next context ops */
+	struct ctxop *prev;		/* previous context ops */
 	hrtime_t save_ts;		/* timestamp of last save */
 	hrtime_t restore_ts;		/* timestamp of last restore */
 } ctxop_t;
diff --git a/usr/src/uts/i86pc/ml/offsets.in b/usr/src/uts/i86pc/ml/offsets.in
index 8390ba0fc9..8dc6f0f572 100644
--- a/usr/src/uts/i86pc/ml/offsets.in
+++ b/usr/src/uts/i86pc/ml/offsets.in
@@ -128,9 +128,6 @@ _kthread	THREAD_SIZE
 	t_useracc
 #endif
 
-ctxop
-	save_op			CTXOP_SAVE
-
 as
 	a_hat