[DTrace-devel] [PATCH] Move proc lock to where we actually find a USDT process
eugene.loh at oracle.com
eugene.loh at oracle.com
Mon Jan 6 15:55:25 UTC 2025
From: Eugene Loh <eugene.loh at 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 at 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
More information about the DTrace-devel
mailing list