Linux DTrace development list
 help / color / mirror / Atom feed
* [PATCH] Move proc lock to where we actually find a USDT process
@ 2025-01-06 15:55 eugene.loh
  2025-02-26 16:53 ` [DTrace-devel] " Kris Van Hees
  0 siblings, 1 reply; 2+ messages in thread
From: eugene.loh @ 2025-01-06 15:55 UTC (permalink / raw)
  To: dtrace, dtrace-devel

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

The function dt_pid_create_usdt_probes_proc() creates USDT probes
for a specific process.  It is called in one of two ways:

    dt_proc_scan(dtp, dpr) ->
    dt_pid_create_probes_module(dtp, dpr)

        The process has been locked and we are updating its
        USDT probes.

    dt_pid_create_usdt_probes(pdp, dtp, pcb)

        Here, we look for any pids that might have USDT probes,
        calling the pid-specific function for each candidate.

One problem is that the first code path assumes the process is locked.
This means the second code path has to lock processes even before we
know if it has any USDT probes we care about.

Change dt_pid_create_usdt_probes_proc() so the caller can specify the
process in either one of two ways:

    by pid (implying the process has not been locked)

    by dpr (implying the process has been locked)

In the first case, the function will lock the process, but only if
USDT probes have been found.

Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
---
 libdtrace/dt_pid.c | 70 +++++++++++++++++++++++++++-------------------
 1 file changed, 42 insertions(+), 28 deletions(-)

diff --git a/libdtrace/dt_pid.c b/libdtrace/dt_pid.c
index 8110ccead..6db882059 100644
--- a/libdtrace/dt_pid.c
+++ b/libdtrace/dt_pid.c
@@ -782,33 +782,42 @@ validate_dof_record(const char *path, const dof_parsed_t *parsed,
 
 
 /*
- * Create underlying probes relating to the probespec passed on input.
+ * Create underlying probes relating to the probe description passed on input.
+ * Just set up probes relating to mappings found in this one process.
  *
- * dpr must be set and locked.  Just set up probes relating to mappings found
- * in this one process.
+ * Either the pid must be specified or else dpr must be set and locked.
  *
  * Return 0 on success or -1 on error.  (Failure to create specific underlying
  * probes is not an error.)
  */
 static int
-dt_pid_create_usdt_probes_proc(dtrace_hdl_t *dtp, dt_proc_t *dpr,
+dt_pid_create_usdt_probes_proc(dtrace_hdl_t *dtp, pid_t pid, dt_proc_t *dpr,
 			       dtrace_probedesc_t *pdp, dt_pcb_t *pcb)
 {
 	const dt_provider_t *pvp;
 	int ret = 0;
+	int dpr_caller;		/* dpr was set by caller */
 	char *probepath = NULL;
 	glob_t probeglob = {0};
 
-	assert(dpr != NULL && dpr->dpr_proc);
-	assert(MUTEX_HELD(&dpr->dpr_lock));
+	if (dpr == NULL) {
+		assert(pid != -1);
+		dpr_caller = 0;
+	} else {
+		assert(pid == -1);
+		assert(dpr->dpr_proc);
+		assert(MUTEX_HELD(&dpr->dpr_lock));
+		pid = dpr->dpr_pid;
+		dpr_caller = 1;
+	}
 
 	dt_dprintf("Scanning for usdt probes in %i matching %s:%s:%s\n",
-		   dpr->dpr_pid, pdp->mod, pdp->fun, pdp->prb);
+		   pid, pdp->mod, pdp->fun, pdp->prb);
 
 	pvp = dt_provider_lookup(dtp, "usdt");
 	assert(pvp != NULL);
 
-	if (Pstate(dpr->dpr_proc) == PS_DEAD)
+	if (dpr != NULL && Pstate(dpr->dpr_proc) == PS_DEAD)
 		return 0;
 
 	/*
@@ -835,7 +844,7 @@ dt_pid_create_usdt_probes_proc(dtrace_hdl_t *dtp, dt_proc_t *dpr,
 	assert(pvp->impl != NULL && pvp->impl->provide_probe != NULL);
 
 	if (asprintf(&probepath, "%s/probes/%i/%s/%s/%s/%s", dtp->dt_dofstash_path,
-		     dpr->dpr_pid, pdp->prv[0] == '\0' ? "*" : pdp->prv,
+		     pid, pdp->prv[0] == '\0' ? "*" : pdp->prv,
 		     pdp->mod[0] == '\0' ? "*" : pdp->mod,
 		     pdp->fun[0] == '\0' ? "*" : pdp->fun,
 		     pdp->prb[0] == '\0' ? "*" : pdp->prb) < 0)
@@ -858,6 +867,19 @@ dt_pid_create_usdt_probes_proc(dtrace_hdl_t *dtp, dt_proc_t *dpr,
 		return 0;
 	}
 
+	/* Set dpr and grab the process, if necessary. */
+	if (dpr_caller == 0) {
+		if (dt_proc_grab_lock(dtp, pid, DTRACE_PROC_WAITING |
+				      DTRACE_PROC_SHORTLIVED) < 0) {
+			dt_pid_error(dtp, pcb, NULL, D_PROC_GRAB,
+			    "failed to grab process %d", (int)pid);
+			return -1;
+		}
+		dpr = dt_proc_lookup(dtp, pid);
+		assert(dpr != NULL);
+	}
+
+	/* Loop over USDT probes. */
 	for (size_t i = 0; i < probeglob.gl_pathc; i++) {
 		char *dof_buf = NULL, *p;
 		struct stat s;
@@ -1051,10 +1073,16 @@ dt_pid_create_usdt_probes_proc(dtrace_hdl_t *dtp, dt_proc_t *dpr,
 		free(path);
 		free(dof_buf);
 		globfree(&probeglob);
+		if (dpr_caller == 0)
+			dt_proc_release_unlock(dtp, pid);
 		return -1;
 	}
 
 	globfree(&probeglob);
+	if (dpr_caller == 0) {
+		dt_pid_fix_mod(NULL, pdp, dtp, pid);
+		dt_proc_release_unlock(dtp, pid);
+	}
 	return ret;
 
 scan_err:
@@ -1237,7 +1265,6 @@ dt_pid_create_usdt_probes(dtrace_probedesc_t *pdp, dtrace_hdl_t *dtp, dt_pcb_t *
 			  + strlen(dtp->dt_dofstash_path)
 			  + strlen("/probes/");
 		pid_t pid;
-		dt_proc_t *dpr;
 		dtrace_probedesc_t pdptmp;
 
 		/* Pull out the pid. */
@@ -1247,28 +1274,15 @@ dt_pid_create_usdt_probes(dtrace_probedesc_t *pdp, dtrace_hdl_t *dtp, dt_pcb_t *
 		if (!Pexists(pid))
 			continue;
 
-		/* Grab the process. */
-		if (dt_proc_grab_lock(dtp, pid, DTRACE_PROC_WAITING |
-				      DTRACE_PROC_SHORTLIVED) < 0) {
-			dt_pid_error(dtp, pcb, NULL, D_PROC_GRAB,
-			    "failed to grab process %d", (int)pid);
-			err = 1;  // FIXME but do we want to set the error if we end up return 0?
-			continue;
-		}
-		dpr = dt_proc_lookup(dtp, pid);
-		assert(dpr != NULL);
-
-		/* Create USDT probes for this process. */
+		/* Construct the probe descriptor. */
 		pdptmp.prv = strchr(s, '/') + 1;
 		pdptmp.mod = pdp->mod[0] == '\0' ? "*" : pdp->mod;
 		pdptmp.fun = pdp->fun[0] == '\0' ? "*" : pdp->fun;
 		pdptmp.prb = pdp->prb[0] == '\0' ? "*" : pdp->prb;
-		if (dt_pid_create_usdt_probes_proc(dtp, dpr, &pdptmp, pcb))
-			err = 1;
-
-		dt_pid_fix_mod(NULL, &pdptmp, dtp, dpr->dpr_pid);
 
-		dt_proc_release_unlock(dtp, pid);
+		/* Create USDT probes for this process. */
+		if (dt_pid_create_usdt_probes_proc(dtp, pid, NULL, &pdptmp, pcb))
+			err = 1;
 	}
 	free(globpat);
 	globfree(&globbuf);
@@ -1341,7 +1355,7 @@ dt_pid_create_probes_module(dtrace_hdl_t *dtp, dt_proc_t *dpr)
 			 * a USDT provider.
 			 */
 			if (strcmp(provname, pdp->prv) != 0) {
-				if (dt_pid_create_usdt_probes_proc(dtp, dpr, pdp, NULL) < 0)
+				if (dt_pid_create_usdt_probes_proc(dtp, -1, dpr, pdp, NULL) < 0)
 					ret = 1;
 				else
 					dt_pid_fix_mod(NULL, pdp, dtp, dpr->dpr_pid);
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-02-26 16:53 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-06 15:55 [PATCH] Move proc lock to where we actually find a USDT process eugene.loh
2025-02-26 16:53 ` [DTrace-devel] " Kris Van Hees

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox