[PATCH 01/19] Change probes from having lists of clauses to lists of stmts

* [PATCH 01/19] Change probes from having lists of clauses to lists of stmts
@ 2024-08-29  5:25 eugene.loh
  2024-08-29  5:25 ` [PATCH 02/19] Add a hook for a provider-specific "update" function eugene.loh
                   ` (18 more replies)
  0 siblings, 19 replies; 44+ messages in thread
From: eugene.loh @ 2024-08-29  5:25 UTC (permalink / raw)
  To: dtrace, dtrace-devel

From: Eugene Loh <eugene.loh@oracle.com>

Each stmt has a clause and a probe description.  Traditionally,
we have added clauses to probes.  Further, we have generated an
enabled probe ID for each probe/clause combination, and the
consumer has use the EPID to determine the PRID as well as the
data descriptor for the clause.

In an up-coming patch, we will move to a new scheme, in which a
widened EPID will have the PRID in its upper 32 bits and a stmt
ID in the lower 32 bits.  The stmt ID will be used to identify
clauses to call as well as the data descriptor for the clause.

In this patch, change from probes having clauses associated
with them to having stmts.

We move the definition of dt_probe_clause_t in dt_probe.c to
be a definition of dt_probe_stmt_t in dt_impl.h so that the
type will become available for providers that will need it.

Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
---
 libdtrace/dt_cg.c      | 17 +++++----
 libdtrace/dt_impl.h    |  5 +++
 libdtrace/dt_probe.c   | 79 +++++++++++++++++++++++-------------------
 libdtrace/dt_probe.h   | 12 +++----
 libdtrace/dt_program.c |  8 ++---
 5 files changed, 66 insertions(+), 55 deletions(-)

diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index 4fab8eed..a7861829 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -844,9 +844,10 @@ typedef struct {
 } dt_clause_arg_t;
 
 static int
-dt_cg_call_clause(dtrace_hdl_t *dtp, dt_ident_t *idp, dt_clause_arg_t *arg)
+dt_cg_call_clause(dtrace_hdl_t *dtp, dtrace_stmtdesc_t *sdp, dt_clause_arg_t *arg)
 {
 	dt_irlist_t	*dlp = arg->dlp;
+	dt_ident_t	*idp = sdp->dtsd_clause;
 
 	/*
 	 *	if (*dctx.act != act)	// ldw %r0, [%r9 + DCTX_ACT]
@@ -874,14 +875,12 @@ dt_cg_call_clause(dtrace_hdl_t *dtp, dt_ident_t *idp, dt_clause_arg_t *arg)
 }
 
 void
-dt_cg_tramp_call_clauses(dt_pcb_t *pcb, const dt_probe_t *prp,
-			 dt_activity_t act)
+dt_cg_tramp_call_clauses(dt_pcb_t *pcb, const dt_probe_t *prp, dt_activity_t act)
 {
 	dt_irlist_t	*dlp = &pcb->pcb_ir;
 	dt_clause_arg_t	arg = { dlp, act, pcb->pcb_exitlbl };
 
-	dt_probe_clause_iter(pcb->pcb_hdl, prp,
-			     (dt_clause_f *)dt_cg_call_clause, &arg);
+	dt_probe_stmt_iter(pcb->pcb_hdl, prp, (dt_stmt_f *)dt_cg_call_clause, &arg);
 }
 
 static int
@@ -990,9 +989,10 @@ dt_cg_tramp_epilogue_advance(dt_pcb_t *pcb, dt_activity_t act)
 }
 
 static int
-dt_cg_tramp_error_call_clause(dtrace_hdl_t *dtp, dt_ident_t *idp,
-			      dt_irlist_t *dlp)
+dt_cg_tramp_error_call_clause(dtrace_hdl_t *dtp, dtrace_stmtdesc_t *sdp, dt_irlist_t *dlp)
 {
+	dt_ident_t *idp = sdp->dtsd_clause;
+
 	/*
 	 *	dt_error_#(dctx);	// mov %r1, %r9
 	 *				// call dt_error_#
@@ -1029,8 +1029,7 @@ dt_cg_tramp_error(dt_pcb_t *pcb)
 	TRACE_REGSET("Trampoline: Begin");
 	emit(dlp, BPF_MOV_REG(BPF_REG_9, BPF_REG_1));
 
-	dt_probe_clause_iter(dtp, dtp->dt_error,
-			     (dt_clause_f *)dt_cg_tramp_error_call_clause, dlp);
+	dt_probe_stmt_iter(dtp, dtp->dt_error, (dt_stmt_f *)dt_cg_tramp_error_call_clause, dlp);
 
 	emit(dlp, BPF_MOV_IMM(BPF_REG_0, 0));
 	emit(dlp, BPF_RETURN());
diff --git a/libdtrace/dt_impl.h b/libdtrace/dt_impl.h
index 44dd1415..98fddc23 100644
--- a/libdtrace/dt_impl.h
+++ b/libdtrace/dt_impl.h
@@ -258,6 +258,11 @@ typedef struct dt_percpu_drops {
 
 typedef uint32_t dt_version_t;		/* encoded version (see below) */
 
+typedef struct dt_probe_stmt {
+	dt_list_t		list;
+	dtrace_stmtdesc_t	*stmt;
+} dt_probe_stmt_t;
+
 struct dtrace_hdl {
 	const dtrace_vector_t *dt_vector; /* library vector, if vectored open */
 	void *dt_varg;	/* vector argument, if vectored open */
diff --git a/libdtrace/dt_probe.c b/libdtrace/dt_probe.c
index ab90d2ed..bb28bbed 100644
--- a/libdtrace/dt_probe.c
+++ b/libdtrace/dt_probe.c
@@ -24,11 +24,6 @@
 #include <dt_list.h>
 #include <dt_bpf.h>
 
-typedef struct dt_probe_clause {
-	dt_list_t	list;
-	dt_ident_t	*clause;
-} dt_probe_clause_t;
-
 typedef struct dt_probe_dependent {
 	dt_list_t	list;
 	dt_probe_t	*probe;
@@ -467,7 +462,7 @@ dt_probe_enable(dtrace_hdl_t *dtp, dt_probe_t *prp)
 void
 dt_probe_destroy(dt_probe_t *prp)
 {
-	dt_probe_clause_t	*pcp, *pcp_next;
+	dt_probe_stmt_t		*psp, *psp_next;
 	dt_probe_instance_t	*pip, *pip_next;
 	dt_probe_dependent_t	*dep, *dep_next;
 	dtrace_hdl_t		*dtp;
@@ -499,9 +494,18 @@ dt_probe_destroy(dt_probe_t *prp)
 	dt_free(dtp, prp->nargv);
 	dt_free(dtp, prp->xargv);
 
-	for (pcp = dt_list_next(&prp->clauses); pcp != NULL; pcp = pcp_next) {
-		pcp_next = dt_list_next(pcp);
-		dt_free(dtp, pcp);
+	for (psp = dt_list_next(&prp->stmts); psp != NULL; psp = psp_next) {
+		psp_next = dt_list_next(psp);
+
+		/*
+		 * FIXME?  Is there nothing inside we also need to free?
+		 *
+		 * If psp->stmt was created with dt_probe_error_stmt(), maybe:
+		 *     dt_ident_t *idp = psp->stmt->dtsd_clause;
+		 *     dt_iddtor_difo(idp);
+		 *     dt_ident_destroy(idp);
+		 */
+		dt_free(dtp, psp);
 	}
 
 	for (dep = dt_list_next(&prp->dependents); dep != NULL; dep = dep_next) {
@@ -1283,24 +1287,24 @@ dtrace_probe_iter(dtrace_hdl_t *dtp, const dtrace_probedesc_t *pdp,
 }
 
 /*
- * Create an ERROR-probe specific copy of a given clause.
+ * Create an ERROR-probe specific copy of a given stmt.
  *
- * A modified copy of the clause is necessary because the ERROR probe may share
- * some clauses with other probes, and yet it needs to be handled differently.
+ * A modified copy of the stmt is necessary because the ERROR probe may share
+ * some stmts with other probes, and yet it needs to be handled differently.
  *
- * The following modifications are made in the copy of the clause:
- *
- *	- it is named dt_error_N where N is taken from the original clause
- *	  dt_clause_N (which also guarantees uniqueness)
+ * In the copy of the stmt, the clause is named dt_error_N, where N is taken
+ * from the original stmt's dt_clause_N (which also guarantees uniqueness).
  */
-dt_ident_t *
-dt_probe_error_clause(dtrace_hdl_t *dtp, dt_ident_t *idp)
+static dtrace_stmtdesc_t *
+dt_probe_error_stmt(dtrace_hdl_t *dtp, dtrace_stmtdesc_t *sdp)
 {
 	char		*name;
 	int		len;
+	dt_ident_t	*idp = sdp->dtsd_clause;
 	dtrace_difo_t	*dp = dt_dlib_get_func_difo(dtp, idp);
 	dt_ident_t	*nidp = NULL;
 	dtrace_difo_t	*ndp;
+	dtrace_stmtdesc_t *nsdp = NULL;
 
 	/*
 	 * Copy the DIFO.
@@ -1316,7 +1320,6 @@ dt_probe_error_clause(dtrace_hdl_t *dtp, dt_ident_t *idp)
 	name = dt_alloc(dtp, len);
 	if (name == NULL)
 		goto no_mem;
-
 	snprintf(name, len, "dt_error_%s", idp->di_name + strlen("dt_clause_"));
 
 	/*
@@ -1330,7 +1333,12 @@ dt_probe_error_clause(dtrace_hdl_t *dtp, dt_ident_t *idp)
 
 	dt_ident_set_data(nidp, ndp);
 
-	return nidp;
+	nsdp = dt_alloc(dtp, sizeof(dtrace_stmtdesc_t));
+	if (nsdp == NULL)
+		goto no_mem;
+	nsdp->dtsd_clause = nidp;
+
+	return nsdp;
 
 no_mem:
 	if (ndp != NULL)
@@ -1343,40 +1351,39 @@ no_mem:
 }
 
 int
-dt_probe_add_clause(dtrace_hdl_t *dtp, dt_probe_t *prp, dt_ident_t *idp)
+dt_probe_add_stmt(dtrace_hdl_t *dtp, dt_probe_t *prp, dtrace_stmtdesc_t *sdp)
 {
-	dt_probe_clause_t	*pcp;
+	dt_probe_stmt_t	*psp;
 
-	pcp = dt_zalloc(dtp, sizeof(dt_probe_clause_t));
-	if (pcp == NULL)
+	psp = dt_zalloc(dtp, sizeof(dt_probe_stmt_t));
+	if (psp == NULL)
 		return dt_set_errno(dtp, EDT_NOMEM);
 
 	if (prp == dtp->dt_error) {
-		pcp->clause = dt_probe_error_clause(dtp, idp);
-		if (pcp->clause == NULL) {
-			dt_free(dtp, pcp);
+		psp->stmt = dt_probe_error_stmt(dtp, sdp);
+		if (psp->stmt == NULL) {
+			dt_free(dtp, psp);
 			return 0;
 		}
 	} else
-		pcp->clause = idp;
+		psp->stmt = sdp;
 
-	dt_list_append(&prp->clauses, pcp);
+	dt_list_append(&prp->stmts, psp);
 
 	return 0;
 }
 
 int
-dt_probe_clause_iter(dtrace_hdl_t *dtp, const dt_probe_t *prp,
-		     dt_clause_f *func, void *arg)
+dt_probe_stmt_iter(dtrace_hdl_t *dtp, const dt_probe_t *prp, dt_stmt_f *func, void *arg)
 {
-	dt_probe_clause_t	*pcp;
-	int			rc;
+	dt_probe_stmt_t	*psp;
+	int		rc;
 
 	assert(func != NULL);
 
-	for (pcp = dt_list_next(&prp->clauses); pcp != NULL;
-	     pcp = dt_list_next(pcp)) {
-		rc = func(dtp, pcp->clause, arg);
+	for (psp = dt_list_next(&prp->stmts); psp != NULL;
+	     psp = dt_list_next(psp)) {
+		rc = func(dtp, psp->stmt, arg);
 
 		if (rc != 0)
 			return rc;
diff --git a/libdtrace/dt_probe.h b/libdtrace/dt_probe.h
index b4c1a3e4..2a78cb9c 100644
--- a/libdtrace/dt_probe.h
+++ b/libdtrace/dt_probe.h
@@ -32,7 +32,7 @@ typedef struct dt_probe_instance {
 
 typedef struct dt_probe {
 	dt_list_t list;			/* prev/next in enablings chain */
-	dt_list_t clauses;		/* clauses to attach */
+	dt_list_t stmts;		/* stmts */
 	dt_list_t dependents;		/* dependenct probes to attach */
 	const dtrace_probedesc_t *desc;	/* probe description (id, name) */
 	dt_provider_t *prov;		/* pointer to containing provider */
@@ -86,11 +86,11 @@ typedef int dt_probe_f(dtrace_hdl_t *dtp, dt_probe_t *prp, void *arg);
 extern int dt_probe_iter(dtrace_hdl_t *dtp, const dtrace_probedesc_t *pdp,
 			 dt_probe_f *pfunc, dtrace_probe_f *dfunc, void *arg);
 
-extern int dt_probe_add_clause(dtrace_hdl_t *dtp, dt_probe_t *prp,
-			       dt_ident_t *idp);
-typedef int dt_clause_f(dtrace_hdl_t *dtp, dt_ident_t *idp, void *arg);
-extern int dt_probe_clause_iter(dtrace_hdl_t *dtp, const dt_probe_t *prp,
-				dt_clause_f *func, void *arg);
+extern int dt_probe_add_stmt(dtrace_hdl_t *dtp, dt_probe_t *prp,
+			     dtrace_stmtdesc_t *sdp);
+typedef int dt_stmt_f(dtrace_hdl_t *dtp, dtrace_stmtdesc_t *sdp, void *arg);
+extern int dt_probe_stmt_iter(dtrace_hdl_t *dtp, const dt_probe_t *prp,
+			      dt_stmt_f *func, void *arg);
 
 extern int dt_probe_add_dependent(dtrace_hdl_t *dtp, dt_probe_t *prp,
 				  dt_probe_t *idprp);
diff --git a/libdtrace/dt_program.c b/libdtrace/dt_program.c
index bdb434e0..afbf7265 100644
--- a/libdtrace/dt_program.c
+++ b/libdtrace/dt_program.c
@@ -139,8 +139,8 @@ dtrace_program_info(dtrace_hdl_t *dtp, dtrace_prog_t *pgp,
 }
 
 typedef struct pi_state {
-	int		*cnt;
-	dt_ident_t	*idp;
+	int			*cnt;
+	dtrace_stmtdesc_t	*sdp;
 } pi_state_t;
 
 static int
@@ -151,7 +151,7 @@ dt_stmt_probe(dtrace_hdl_t *dtp, dt_probe_t *prp, pi_state_t *st)
 	dt_probe_info(dtp, prp->desc, &p);
 	dt_probe_enable(dtp, prp);
 
-	dt_probe_add_clause(dtp, prp, st->idp);
+	dt_probe_add_stmt(dtp, prp, st->sdp);
 	(*st->cnt)++;
 
 	return 0;
@@ -166,7 +166,7 @@ dt_prog_stmt(dtrace_hdl_t *dtp, dtrace_prog_t *pgp, dtrace_stmtdesc_t *sdp,
 	int			rc;
 
 	st.cnt = cnt;
-	st.idp = sdp->dtsd_clause;
+	st.sdp = sdp;
 	rc = dt_probe_iter(dtp, pdp, (dt_probe_f *)dt_stmt_probe, NULL, &st);
 
 	/*
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 44+ messages in thread