[DTrace-devel] [PATCH v3 02/19] Add a dt_provider_discover() function
Kris Van Hees
kris.van.hees at oracle.com
Wed Oct 23 22:22:23 UTC 2024
On Wed, Oct 16, 2024 at 12:01:36PM -0400, eugene.loh at oracle.com wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
>
> It is possible for new providers and probes to appear after dtrace_go().
> Add a function that can be called for such discovery, on a per-provider
> basis. Each provider that supports such discovery needs to provide:
>
> *) a function discover() that performs discovery on behalf of the provider
>
> *) a function add_probe() that adds a particular (discovered) probe
>
> To support looping over providers, move dt_providers[] from
> dt_open.c to dt_provider.c and extern it in dt_provider.h.
>
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
> libdtrace/dt_bpf.c | 6 +++++
> libdtrace/dt_open.c | 22 +---------------
> libdtrace/dt_provider.c | 57 +++++++++++++++++++++++++++++++++++++++++
> libdtrace/dt_provider.h | 6 +++++
> libdtrace/dt_work.c | 3 +++
> 5 files changed, 73 insertions(+), 21 deletions(-)
>
> diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
> index ad11d178e..7a36b6187 100644
> --- a/libdtrace/dt_bpf.c
> +++ b/libdtrace/dt_bpf.c
> @@ -1275,6 +1275,12 @@ dt_bpf_load_progs(dtrace_hdl_t *dtp, uint_t cflags)
> int fd;
> int rc = -1;
>
> + if (prp->prov->impl->prog_type == BPF_PROG_TYPE_UNSPEC) {
> + if (prp->prov->impl->add_probe)
> + prp->prov->impl->add_probe(dtp, prp);
> + continue;
This could benefit from a comment perhaps? It took me a bit to figure out
why we would want this here, since this is the case where the probes are
already known to exist (they were discovered during program compilation).
So, needing a call to add_probe seems out-of-place here.
I do understand (after digging through the code a bit) that it is needed for
USDT because you add probe name data to the usdt_names BPF map and probe data
to the usdt_prids BPF map from the add_probe() callback. But thatis far from
obvious... But try to keep the comment non-USDT specific if you can. Perhaps
just mention this is needed to handle probe types that are discoverable at a
later time, because they may need additional processing.
I think that testing for prog_type == UNSPEC is not appropriate here. That
seems to be a case of leaking USDT-specific implementation detail into this
generic function. One way to deal with this might be to add a test in
add_probe_uprobe() on dtp->dt_active. If dt_active == 0, then
add_probe_uprobe() should immediately return (because the probe already exists
in the system). If dt_active == 1, then it is a runtime discovered probe.
> + }
> +
> dp = prp->difo;
> if (dp == NULL)
> continue;
> diff --git a/libdtrace/dt_open.c b/libdtrace/dt_open.c
> index 848141ddc..1f586fc4f 100644
> --- a/libdtrace/dt_open.c
> +++ b/libdtrace/dt_open.c
> @@ -61,26 +61,6 @@ const dt_version_t _dtrace_versions[] = {
> 0
> };
>
> -/*
> - * List of provider modules that register providers and probes. A single
> - * provider module may create multiple providers.
> - */
> -static const dt_provimpl_t *dt_providers[] = {
> - &dt_dtrace, /* list dt_dtrace first */
> - &dt_cpc,
> - &dt_fbt_fprobe,
> - &dt_io,
> - &dt_ip,
> - &dt_lockstat,
> - &dt_proc,
> - &dt_profile,
> - &dt_rawtp,
> - &dt_sched,
> - &dt_sdt,
> - &dt_syscall,
> - &dt_uprobe,
> -};
> -
> /*
> * Table of global identifiers. This is used to populate the global identifier
> * hash when a new dtrace client open occurs. For more info see dt_ident.h.
> @@ -1210,7 +1190,7 @@ dtrace_init(dtrace_hdl_t *dtp)
> * known providers.
> */
> dt_probe_init(dtp);
> - for (i = 0; i < ARRAY_SIZE(dt_providers); i++) {
> + for (i = 0; dt_providers[i]; i++) {
> int n;
>
> n = dt_providers[i]->populate(dtp);
> diff --git a/libdtrace/dt_provider.c b/libdtrace/dt_provider.c
> index 8cfef0ba2..5f9766183 100644
> --- a/libdtrace/dt_provider.c
> +++ b/libdtrace/dt_provider.c
> @@ -18,10 +18,32 @@
> #include <port.h>
>
> #include <dt_provider.h>
> +#include <dt_probe.h>
> #include <dt_module.h>
> #include <dt_string.h>
> #include <dt_list.h>
>
> +/*
> + * List of provider modules that register providers and probes. A single
> + * provider module may create multiple providers.
> + */
> +const dt_provimpl_t *dt_providers[] = {
> + &dt_dtrace, /* list dt_dtrace first */
> + &dt_cpc,
> + &dt_fbt_fprobe,
> + &dt_io,
> + &dt_ip,
> + &dt_lockstat,
> + &dt_proc,
> + &dt_profile,
> + &dt_rawtp,
> + &dt_sched,
> + &dt_sdt,
> + &dt_syscall,
> + &dt_uprobe,
> + NULL
> +};
> +
> static uint32_t
> dt_provider_hval(const dt_provider_t *pvp)
> {
> @@ -149,3 +171,38 @@ dt_provider_xref(dtrace_hdl_t *dtp, dt_provider_t *pvp, id_t id)
> BT_SET(pvp->pv_xrefs, id);
> return 0;
> }
> +
> +int
> +dt_provider_discover(dtrace_hdl_t *dtp)
> +{
> + int i, prid = dtp->dt_probe_id;
> +
> + /* Discover new probes. */
> + for (i = 0; dt_providers[i]; i++) {
> + int n;
> +
> + if (dt_providers[i]->discover == NULL)
> + continue;
> +
> + n = dt_providers[i]->discover(dtp);
> + if (n < 0)
> + return -1; /* errno is already set */
> + }
> +
> + /* Add them. */
> + for ( ; prid < dtp->dt_probe_id; prid++) {
> + dt_probe_t *prp = dtp->dt_probes[prid];
> + int rc;
> +
> + dt_probe_enable(dtp, prp);
> +
> + if (prp->prov->impl->add_probe == NULL)
> + continue;
> +
> + rc = prp->prov->impl->add_probe(dtp, prp);
> + if (rc < 0)
> + return rc;
> + }
> +
> + return 0;
> +}
> diff --git a/libdtrace/dt_provider.h b/libdtrace/dt_provider.h
> index 17b1844cb..384d4fd33 100644
> --- a/libdtrace/dt_provider.h
> +++ b/libdtrace/dt_provider.h
> @@ -62,6 +62,9 @@ typedef struct dt_provimpl {
> int (*probe_info)(dtrace_hdl_t *dtp, /* get probe info */
> const struct dt_probe *prp,
> int *argcp, dt_argdesc_t **argvp);
> + int (*discover)(dtrace_hdl_t *dtp); /* discover new probes */
> + int (*add_probe)(dtrace_hdl_t *dtp, /* add a new probe */
> + struct dt_probe *prp);
> void (*detach)(dtrace_hdl_t *dtp, /* probe cleanup */
> const struct dt_probe *prb);
> void (*probe_destroy)(dtrace_hdl_t *dtp, /* free probe data */
> @@ -86,6 +89,8 @@ extern dt_provimpl_t dt_sdt;
> extern dt_provimpl_t dt_syscall;
> extern dt_provimpl_t dt_uprobe;
>
> +extern const dt_provimpl_t *dt_providers[];
> +
> typedef struct dt_provider {
> dt_list_t pv_list; /* list forward/back pointers */
> struct dt_hentry he; /* htab links */
> @@ -110,6 +115,7 @@ extern dt_provider_t *dt_provider_create(dtrace_hdl_t *, const char *,
> const dt_provimpl_t *,
> const dtrace_pattr_t *, void *);
> extern int dt_provider_xref(dtrace_hdl_t *, dt_provider_t *, id_t);
> +extern int dt_provider_discover(dtrace_hdl_t *dtp);
>
> #ifdef __cplusplus
> }
> diff --git a/libdtrace/dt_work.c b/libdtrace/dt_work.c
> index 11335345a..5318cdb7e 100644
> --- a/libdtrace/dt_work.c
> +++ b/libdtrace/dt_work.c
> @@ -362,6 +362,9 @@ dtrace_work(dtrace_hdl_t *dtp, FILE *fp, dtrace_consume_probe_f *pfunc,
> dtrace_workstatus_t rval;
> int gen;
>
> + if (dt_provider_discover(dtp) < 0)
> + return DTRACE_WORKSTATUS_ERROR;
> +
> switch (dtrace_status(dtp)) {
> case DTRACE_STATUS_EXITED:
> case DTRACE_STATUS_STOPPED:
> --
> 2.43.5
>
More information about the DTrace-devel
mailing list