Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 28 Aug 2018 20:21:36 +0000 (UTC)
From:      Mark Johnston <markj@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r338359 - in head/sys/cddl/dev/fbt: . aarch64 arm mips powerpc riscv x86
Message-ID:  <201808282021.w7SKLabG060006@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: markj
Date: Tue Aug 28 20:21:36 2018
New Revision: 338359
URL: https://svnweb.freebsd.org/changeset/base/338359

Log:
  Allow multiple FBT probes to share a tracepoint.
  
  With GNU ifuncs, multiple FBT probes may correspond to the same
  instruction.  fbt_invop() assumed that this could not happen and
  would return after the first probe found in the global FBT hash
  table, which might not be the one that's enabled.  Fix the problem
  on x86 by linking probes that share a tracepoint and having each
  linked probe fire when the tracepoint is hit.
  
  PR:		230846
  Approved by:	re (gjb)
  Sponsored by:	The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D16921

Modified:
  head/sys/cddl/dev/fbt/aarch64/fbt_isa.c
  head/sys/cddl/dev/fbt/arm/fbt_isa.c
  head/sys/cddl/dev/fbt/fbt.c
  head/sys/cddl/dev/fbt/fbt.h
  head/sys/cddl/dev/fbt/mips/fbt_isa.c
  head/sys/cddl/dev/fbt/powerpc/fbt_isa.c
  head/sys/cddl/dev/fbt/riscv/fbt_isa.c
  head/sys/cddl/dev/fbt/x86/fbt_isa.c

Modified: head/sys/cddl/dev/fbt/aarch64/fbt_isa.c
==============================================================================
--- head/sys/cddl/dev/fbt/aarch64/fbt_isa.c	Tue Aug 28 18:50:34 2018	(r338358)
+++ head/sys/cddl/dev/fbt/aarch64/fbt_isa.c	Tue Aug 28 20:21:36 2018	(r338359)
@@ -152,7 +152,7 @@ again:
 		fbt->fbtp_id = dtrace_probe_create(fbt_id, modname,
 		    name, FBT_RETURN, 3, fbt);
 	} else {
-		retfbt->fbtp_next = fbt;
+		retfbt->fbtp_probenext = fbt;
 		fbt->fbtp_id = retfbt->fbtp_id;
 	}
 	retfbt = fbt;

Modified: head/sys/cddl/dev/fbt/arm/fbt_isa.c
==============================================================================
--- head/sys/cddl/dev/fbt/arm/fbt_isa.c	Tue Aug 28 18:50:34 2018	(r338358)
+++ head/sys/cddl/dev/fbt/arm/fbt_isa.c	Tue Aug 28 20:21:36 2018	(r338359)
@@ -165,7 +165,7 @@ again:
 		fbt->fbtp_id = dtrace_probe_create(fbt_id, modname,
 		    name, FBT_RETURN, 2, fbt);
 	} else {
-		retfbt->fbtp_next = fbt;
+		retfbt->fbtp_probenext = fbt;
 		fbt->fbtp_id = retfbt->fbtp_id;
 	}
 	retfbt = fbt;

Modified: head/sys/cddl/dev/fbt/fbt.c
==============================================================================
--- head/sys/cddl/dev/fbt/fbt.c	Tue Aug 28 18:50:34 2018	(r338358)
+++ head/sys/cddl/dev/fbt/fbt.c	Tue Aug 28 20:21:36 2018	(r338359)
@@ -156,7 +156,7 @@ fbt_doubletrap(void)
 	for (i = 0; i < fbt_probetab_size; i++) {
 		fbt = fbt_probetab[i];
 
-		for (; fbt != NULL; fbt = fbt->fbtp_next)
+		for (; fbt != NULL; fbt = fbt->fbtp_probenext)
 			fbt_patch_tracepoint(fbt, fbt->fbtp_savedval);
 	}
 }
@@ -205,39 +205,52 @@ fbt_provide_module(void *arg, modctl_t *lf)
 }
 
 static void
+fbt_destroy_one(fbt_probe_t *fbt)
+{
+	fbt_probe_t *hash, *hashprev, *next;
+	int ndx;
+
+	ndx = FBT_ADDR2NDX(fbt->fbtp_patchpoint);
+	for (hash = fbt_probetab[ndx], hashprev = NULL; hash != NULL;
+	    hash = hash->fbtp_hashnext, hashprev = hash) {
+		if (hash == fbt) {
+			if ((next = fbt->fbtp_tracenext) != NULL)
+				next->fbtp_hashnext = hash->fbtp_hashnext;
+			else
+				next = hash->fbtp_hashnext;
+			if (hashprev != NULL)
+				hashprev->fbtp_hashnext = next;
+			else
+				fbt_probetab[ndx] = next;
+			goto free;
+		} else if (hash->fbtp_patchpoint == fbt->fbtp_patchpoint) {
+			for (next = hash; next->fbtp_tracenext != NULL;
+			    next = next->fbtp_tracenext) {
+				if (fbt == next->fbtp_tracenext) {
+					next->fbtp_tracenext =
+					    fbt->fbtp_tracenext;
+					goto free;
+				}
+			}
+		}
+	}
+	panic("probe %p not found in hash table", fbt);
+free:
+	free(fbt, M_FBT);
+}
+
+static void
 fbt_destroy(void *arg, dtrace_id_t id, void *parg)
 {
-	fbt_probe_t *fbt = parg, *next, *hash, *last;
+	fbt_probe_t *fbt = parg, *next;
 	modctl_t *ctl;
-	int ndx;
 
 	do {
 		ctl = fbt->fbtp_ctl;
-
 		ctl->fbt_nentries--;
 
-		/*
-		 * Now we need to remove this probe from the fbt_probetab.
-		 */
-		ndx = FBT_ADDR2NDX(fbt->fbtp_patchpoint);
-		last = NULL;
-		hash = fbt_probetab[ndx];
-
-		while (hash != fbt) {
-			ASSERT(hash != NULL);
-			last = hash;
-			hash = hash->fbtp_hashnext;
-		}
-
-		if (last != NULL) {
-			last->fbtp_hashnext = fbt->fbtp_hashnext;
-		} else {
-			fbt_probetab[ndx] = fbt->fbtp_hashnext;
-		}
-
-		next = fbt->fbtp_next;
-		free(fbt, M_FBT);
-
+		next = fbt->fbtp_probenext;
+		fbt_destroy_one(fbt);
 		fbt = next;
 	} while (fbt != NULL);
 }
@@ -265,14 +278,16 @@ fbt_enable(void *arg, dtrace_id_t id, void *parg)
 		return;
 	}
 
-	for (; fbt != NULL; fbt = fbt->fbtp_next)
+	for (; fbt != NULL; fbt = fbt->fbtp_probenext) {
 		fbt_patch_tracepoint(fbt, fbt->fbtp_patchval);
+		fbt->fbtp_enabled++;
+	}
 }
 
 static void
 fbt_disable(void *arg, dtrace_id_t id, void *parg)
 {
-	fbt_probe_t *fbt = parg;
+	fbt_probe_t *fbt = parg, *hash;
 	modctl_t *ctl = fbt->fbtp_ctl;
 
 	ASSERT(ctl->nenabled > 0);
@@ -281,8 +296,21 @@ fbt_disable(void *arg, dtrace_id_t id, void *parg)
 	if ((ctl->loadcnt != fbt->fbtp_loadcnt))
 		return;
 
-	for (; fbt != NULL; fbt = fbt->fbtp_next)
-		fbt_patch_tracepoint(fbt, fbt->fbtp_savedval);
+	for (; fbt != NULL; fbt = fbt->fbtp_probenext) {
+		fbt->fbtp_enabled--;
+
+		for (hash = fbt_probetab[FBT_ADDR2NDX(fbt->fbtp_patchpoint)];
+		    hash != NULL; hash = hash->fbtp_hashnext) {
+			if (hash->fbtp_patchpoint == fbt->fbtp_patchpoint) {
+				for (; hash != NULL; hash = hash->fbtp_tracenext)
+					if (hash->fbtp_enabled > 0)
+						break;
+				break;
+			}
+		}
+		if (hash == NULL)
+			fbt_patch_tracepoint(fbt, fbt->fbtp_savedval);
+	}
 }
 
 static void
@@ -296,7 +324,7 @@ fbt_suspend(void *arg, dtrace_id_t id, void *parg)
 	if ((ctl->loadcnt != fbt->fbtp_loadcnt))
 		return;
 
-	for (; fbt != NULL; fbt = fbt->fbtp_next)
+	for (; fbt != NULL; fbt = fbt->fbtp_probenext)
 		fbt_patch_tracepoint(fbt, fbt->fbtp_savedval);
 }
 
@@ -311,7 +339,7 @@ fbt_resume(void *arg, dtrace_id_t id, void *parg)
 	if ((ctl->loadcnt != fbt->fbtp_loadcnt))
 		return;
 
-	for (; fbt != NULL; fbt = fbt->fbtp_next)
+	for (; fbt != NULL; fbt = fbt->fbtp_probenext)
 		fbt_patch_tracepoint(fbt, fbt->fbtp_patchval);
 }
 

Modified: head/sys/cddl/dev/fbt/fbt.h
==============================================================================
--- head/sys/cddl/dev/fbt/fbt.h	Tue Aug 28 18:50:34 2018	(r338358)
+++ head/sys/cddl/dev/fbt/fbt.h	Tue Aug 28 20:21:36 2018	(r338359)
@@ -34,9 +34,18 @@
 
 #include "fbt_isa.h"
 
+/*
+ * fbt_probe is a bit of a misnomer.  One of these structures is created for
+ * each trace point of an FBT probe.  A probe might have multiple trace points
+ * (e.g., a function with multiple return instructions), and different probes
+ * might have a trace point at the same address (e.g., GNU ifuncs).
+ */
 typedef struct fbt_probe {
-	struct fbt_probe *fbtp_hashnext;
-	fbt_patchval_t	*fbtp_patchpoint;
+	struct fbt_probe *fbtp_hashnext;	/* global hash table linkage */
+	struct fbt_probe *fbtp_tracenext;	/* next probe for tracepoint */
+	struct fbt_probe *fbtp_probenext;	/* next tracepoint for probe */
+	int		fbtp_enabled;
+	fbt_patchval_t  *fbtp_patchpoint;
 	int8_t		fbtp_rval;
 	fbt_patchval_t	fbtp_patchval;
 	fbt_patchval_t	fbtp_savedval;
@@ -46,7 +55,6 @@ typedef struct fbt_probe {
 	modctl_t	*fbtp_ctl;
 	int		fbtp_loadcnt;
 	int		fbtp_symindx;
-	struct fbt_probe *fbtp_next;
 } fbt_probe_t;
 
 struct linker_file;

Modified: head/sys/cddl/dev/fbt/mips/fbt_isa.c
==============================================================================
--- head/sys/cddl/dev/fbt/mips/fbt_isa.c	Tue Aug 28 18:50:34 2018	(r338358)
+++ head/sys/cddl/dev/fbt/mips/fbt_isa.c	Tue Aug 28 20:21:36 2018	(r338359)
@@ -142,7 +142,7 @@ again:
 		fbt->fbtp_id = dtrace_probe_create(fbt_id, modname,
 		    name, FBT_RETURN, 3, fbt);
 	} else {
-		retfbt->fbtp_next = fbt;
+		retfbt->fbtp_probenext = fbt;
 		fbt->fbtp_id = retfbt->fbtp_id;
 	}
 	retfbt = fbt;

Modified: head/sys/cddl/dev/fbt/powerpc/fbt_isa.c
==============================================================================
--- head/sys/cddl/dev/fbt/powerpc/fbt_isa.c	Tue Aug 28 18:50:34 2018	(r338358)
+++ head/sys/cddl/dev/fbt/powerpc/fbt_isa.c	Tue Aug 28 20:21:36 2018	(r338359)
@@ -208,7 +208,7 @@ again:
 		fbt->fbtp_id = dtrace_probe_create(fbt_id, modname,
 		    name, FBT_RETURN, FBT_AFRAMES, fbt);
 	} else {
-		retfbt->fbtp_next = fbt;
+		retfbt->fbtp_probenext = fbt;
 		fbt->fbtp_id = retfbt->fbtp_id;
 	}
 

Modified: head/sys/cddl/dev/fbt/riscv/fbt_isa.c
==============================================================================
--- head/sys/cddl/dev/fbt/riscv/fbt_isa.c	Tue Aug 28 18:50:34 2018	(r338358)
+++ head/sys/cddl/dev/fbt/riscv/fbt_isa.c	Tue Aug 28 20:21:36 2018	(r338359)
@@ -141,7 +141,7 @@ again:
 		fbt->fbtp_id = dtrace_probe_create(fbt_id, modname,
 		    name, FBT_RETURN, 3, fbt);
 	} else {
-		retfbt->fbtp_next = fbt;
+		retfbt->fbtp_probenext = fbt;
 		fbt->fbtp_id = retfbt->fbtp_id;
 	}
 	retfbt = fbt;

Modified: head/sys/cddl/dev/fbt/x86/fbt_isa.c
==============================================================================
--- head/sys/cddl/dev/fbt/x86/fbt_isa.c	Tue Aug 28 18:50:34 2018	(r338358)
+++ head/sys/cddl/dev/fbt/x86/fbt_isa.c	Tue Aug 28 20:21:36 2018	(r338359)
@@ -67,6 +67,7 @@ fbt_invop(uintptr_t addr, struct trapframe *frame, uin
 	uintptr_t *stack;
 	uintptr_t arg0, arg1, arg2, arg3, arg4;
 	fbt_probe_t *fbt;
+	int8_t fbtrval;
 
 #ifdef __amd64__
 	stack = (uintptr_t *)frame->tf_rsp;
@@ -78,7 +79,11 @@ fbt_invop(uintptr_t addr, struct trapframe *frame, uin
 	cpu = &solaris_cpu[curcpu];
 	fbt = fbt_probetab[FBT_ADDR2NDX(addr)];
 	for (; fbt != NULL; fbt = fbt->fbtp_hashnext) {
-		if ((uintptr_t)fbt->fbtp_patchpoint == addr) {
+		if ((uintptr_t)fbt->fbtp_patchpoint != addr)
+			continue;
+		fbtrval = fbt->fbtp_rval;
+		for (; fbt != NULL; fbt = fbt->fbtp_tracenext) {
+			ASSERT(fbt->fbtp_rval == fbtrval);
 			if (fbt->fbtp_roffset == 0) {
 #ifdef __amd64__
 				/* fbt->fbtp_rval == DTRACE_INVOP_PUSHQ_RBP */
@@ -135,9 +140,8 @@ fbt_invop(uintptr_t addr, struct trapframe *frame, uin
 				    rval, 0, 0, 0);
 				cpu->cpu_dtrace_caller = 0;
 			}
-
-			return (fbt->fbtp_rval);
 		}
+		return (fbtrval);
 	}
 
 	return (0);
@@ -162,7 +166,7 @@ fbt_provide_module_function(linker_file_t lf, int symi
 {
 	char *modname = opaque;
 	const char *name = symval->name;
-	fbt_probe_t *fbt, *retfbt;
+	fbt_probe_t *fbt, *hash, *retfbt;
 	int j;
 	int size;
 	uint8_t *instr, *limit;
@@ -224,8 +228,18 @@ fbt_provide_module_function(linker_file_t lf, int symi
 	fbt->fbtp_patchval = FBT_PATCHVAL;
 	fbt->fbtp_symindx = symindx;
 
-	fbt->fbtp_hashnext = fbt_probetab[FBT_ADDR2NDX(instr)];
-	fbt_probetab[FBT_ADDR2NDX(instr)] = fbt;
+	for (hash = fbt_probetab[FBT_ADDR2NDX(instr)]; hash != NULL;
+	    hash = hash->fbtp_hashnext) {
+		if (hash->fbtp_patchpoint == fbt->fbtp_patchpoint) {
+			fbt->fbtp_tracenext = hash->fbtp_tracenext;
+			hash->fbtp_tracenext = fbt;
+			break;
+		}
+	}
+	if (hash == NULL) {
+		fbt->fbtp_hashnext = fbt_probetab[FBT_ADDR2NDX(instr)];
+		fbt_probetab[FBT_ADDR2NDX(instr)] = fbt;
+	}
 
 	lf->fbt_nentries++;
 
@@ -301,7 +315,7 @@ again:
 		fbt->fbtp_id = dtrace_probe_create(fbt_id, modname,
 		    name, FBT_RETURN, 3, fbt);
 	} else {
-		retfbt->fbtp_next = fbt;
+		retfbt->fbtp_probenext = fbt;
 		fbt->fbtp_id = retfbt->fbtp_id;
 	}
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201808282021.w7SKLabG060006>