[DTrace-devel] [PATCH] provider: support (opaque) private data for providers
Kris Van Hees
kris.van.hees at oracle.com
Tue May 16 19:25:47 UTC 2023
There will be a v2 coming that has cleanup support for the pv_data member,
since otherwise we'll be leaking memory.
On Tue, May 16, 2023 at 01:39:38PM -0400, Kris Van Hees via DTrace-devel wrote:
> Commit 2ba7c3bd ("Add a CPC provider") added an opaque private data
> pointer to the provider implementation struct. In retrospect, that
> does not seem to be the best place because the implementation struct
> is really meant for callback function pointers. The provider struct
> itself is a better place for this. SDT providers will also make use
> of the ability to store private data for providers.
>
> Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> ---
> libdtrace/dt_parser.c | 3 ++-
> libdtrace/dt_prov_cpc.c | 45 +++++++++++++++++++++---------------
> libdtrace/dt_prov_dtrace.c | 2 +-
> libdtrace/dt_prov_fbt.c | 2 +-
> libdtrace/dt_prov_lockstat.c | 2 +-
> libdtrace/dt_prov_proc.c | 2 +-
> libdtrace/dt_prov_profile.c | 2 +-
> libdtrace/dt_prov_rawtp.c | 2 +-
> libdtrace/dt_prov_sched.c | 2 +-
> libdtrace/dt_prov_sdt.c | 2 +-
> libdtrace/dt_prov_syscall.c | 2 +-
> libdtrace/dt_prov_uprobe.c | 10 ++++----
> libdtrace/dt_provider.c | 4 +++-
> libdtrace/dt_provider.h | 4 ++--
> 14 files changed, 48 insertions(+), 36 deletions(-)
>
> diff --git a/libdtrace/dt_parser.c b/libdtrace/dt_parser.c
> index ea9ff15b..7cf24886 100644
> --- a/libdtrace/dt_parser.c
> +++ b/libdtrace/dt_parser.c
> @@ -2644,7 +2644,8 @@ dt_node_provider(char *name, dt_node_t *probes)
> if ((dnp->dn_provider = dt_provider_lookup(dtp, name)) != NULL)
> dnp->dn_provred = B_TRUE;
> else if (!(dnp->dn_provider = dt_provider_create(dtp, name, NULL,
> - &_dtrace_prvdesc)))
> + &_dtrace_prvdesc,
> + NULL)))
> longjmp(yypcb->pcb_jmpbuf, EDT_NOMEM);
> else
> dnp->dn_provider->pv_flags |= DT_PROVIDER_INTF;
> diff --git a/libdtrace/dt_prov_cpc.c b/libdtrace/dt_prov_cpc.c
> index aec4367d..feb6d1e3 100644
> --- a/libdtrace/dt_prov_cpc.c
> +++ b/libdtrace/dt_prov_cpc.c
> @@ -50,16 +50,12 @@ typedef struct cpc_probe_map {
> char *pfmname;
> } cpc_probe_map_t;
>
> -static dt_probe_t *cpc_probe_insert(dtrace_hdl_t *dtp, const char *prb)
> +static dt_probe_t *cpc_probe_insert(dtrace_hdl_t *dtp, dt_provider_t *prv,
> + const char *prb)
> {
> - dt_provider_t *prv;
> cpc_probe_t *datap;
> int i, cnt = dtp->dt_conf.num_online_cpus;
>
> - prv = dt_provider_lookup(dtp, prvname);
> - if (!prv)
> - return 0;
> -
> datap = dt_zalloc(dtp, sizeof(cpc_probe_t));
> if (datap == NULL)
> return NULL;
> @@ -82,9 +78,13 @@ err:
> static int populate(dtrace_hdl_t *dtp)
> {
> int n = 0;
> + dt_list_t *listp = dt_zalloc(dtp, sizeof(dt_list_t));
> + dt_provider_t *prv;
>
> - dt_provider_create(dtp, prvname, &dt_cpc, &pattr);
> - dt_cpc.prv_data = dt_zalloc(dtp, sizeof(dt_list_t));
> + if (listp == NULL)
> + return 0;
> +
> + prv = dt_provider_create(dtp, prvname, &dt_cpc, &pattr, listp);
>
> /* incidentally, pfm_strerror(pfm_initialize()) describes the error */
> if (pfm_initialize() != PFM_SUCCESS)
> @@ -215,7 +215,7 @@ static int populate(dtrace_hdl_t *dtp)
> continue;
> for (unsigned char *p = next_probe_map->Dname; *p; p++)
> *p = (*p == '-') ? '_' : tolower(*p);
> - dt_list_append(dt_cpc.prv_data, next_probe_map);
> + dt_list_append(listp, next_probe_map);
>
> /*
> * Compose a CPC probe name by adding mode "all" and a sample period
> @@ -234,7 +234,8 @@ static int populate(dtrace_hdl_t *dtp)
> pd.mod = modname;
> pd.fun = funname;
> pd.prb = s;
> - if (dt_probe_lookup(dtp, &pd) == NULL && cpc_probe_insert(dtp, s))
> + if (dt_probe_lookup(dtp, &pd) == NULL &&
> + cpc_probe_insert(dtp, prv, s))
> n++;
>
> dt_free(dtp, s);
> @@ -244,15 +245,17 @@ static int populate(dtrace_hdl_t *dtp)
> return n;
> }
>
> -static int decode_event(struct perf_event_attr *ap, const char *name) {
> +static int decode_event(struct perf_event_attr *ap, dt_provider_t *prv,
> + const char *name) {
> cpc_probe_map_t *probe_map;
> pfm_perf_encode_arg_t encoding;
>
> /* find the probe name mapping for this D name */
> - for (probe_map = dt_list_next(dt_cpc.prv_data);
> - probe_map; probe_map = dt_list_next(probe_map))
> + for (probe_map = dt_list_next(prv->pv_data); probe_map;
> + probe_map = dt_list_next(probe_map))
> if (strcmp(name, probe_map->Dname) == 0)
> break;
> +
> if (probe_map == NULL)
> return -1;
>
> @@ -293,7 +296,8 @@ static int decode_attributes(struct perf_event_attr *ap, const char *name) {
> return -1;
> }
>
> -static int decode_probename(struct perf_event_attr *ap, const char *name) {
> +static int decode_probename(struct perf_event_attr *ap, dt_provider_t *prv,
> + const char *name) {
> char buf[DTRACE_NAMELEN];
> char *pend;
>
> @@ -307,7 +311,7 @@ static int decode_probename(struct perf_event_attr *ap, const char *name) {
> return -1;
> *pend = '\0';
> pend++;
> - if (decode_event(ap, name) < 0)
> + if (decode_event(ap, prv, name) < 0)
> return -1;
>
> /* "mode" substring */
> @@ -342,8 +346,13 @@ static int decode_probename(struct perf_event_attr *ap, const char *name) {
>
> static int provide(dtrace_hdl_t *dtp, const dtrace_probedesc_t *pdp)
> {
> + dt_provider_t *prv;
> struct perf_event_attr attr;
>
> + prv = dt_provider_lookup(dtp, prvname);
> + if (!prv)
> + return 0;
> +
> /* make sure we have IDNONE and a legal name */
> if (pdp->id != DTRACE_IDNONE || strcmp(pdp->prv, prvname) ||
> strcmp(pdp->mod, modname) || strcmp(pdp->fun, funname))
> @@ -354,11 +363,11 @@ static int provide(dtrace_hdl_t *dtp, const dtrace_probedesc_t *pdp)
> return 0;
>
> /* check if the probe name can be decoded */
> - if (decode_probename(&attr, pdp->prb) == -1)
> + if (decode_probename(&attr, prv, pdp->prb) == -1)
> return 0;
>
> /* try to add this probe */
> - if (cpc_probe_insert(dtp, pdp->prb) == NULL)
> + if (cpc_probe_insert(dtp, prv, pdp->prb) == NULL)
> return 0;
>
> return 1;
> @@ -419,7 +428,7 @@ static int attach(dtrace_hdl_t *dtp, const dt_probe_t *prp, int bpf_fd)
> char *name = datap->name; /* same as prp->desc->prb */
>
> memset(&attr, 0, sizeof(attr));
> - if (decode_probename(&attr, name) < 0)
> + if (decode_probename(&attr, prp->prov, name) < 0)
> return -1;
> attr.wakeup_events = 1;
>
> diff --git a/libdtrace/dt_prov_dtrace.c b/libdtrace/dt_prov_dtrace.c
> index 939ab1cc..0c3bb135 100644
> --- a/libdtrace/dt_prov_dtrace.c
> +++ b/libdtrace/dt_prov_dtrace.c
> @@ -40,7 +40,7 @@ static int populate(dtrace_hdl_t *dtp)
> dt_probe_t *prp;
> int n = 0;
>
> - prv = dt_provider_create(dtp, prvname, &dt_dtrace, &pattr);
> + prv = dt_provider_create(dtp, prvname, &dt_dtrace, &pattr, NULL);
> if (prv == NULL)
> return 0;
>
> diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c
> index 19917560..d67fc7ba 100644
> --- a/libdtrace/dt_prov_fbt.c
> +++ b/libdtrace/dt_prov_fbt.c
> @@ -69,7 +69,7 @@ static int populate(dtrace_hdl_t *dtp)
> dtrace_syminfo_t sip;
> dtrace_probedesc_t pd;
>
> - prv = dt_provider_create(dtp, prvname, &dt_fbt, &pattr);
> + prv = dt_provider_create(dtp, prvname, &dt_fbt, &pattr, NULL);
> if (prv == NULL)
> return 0;
>
> diff --git a/libdtrace/dt_prov_lockstat.c b/libdtrace/dt_prov_lockstat.c
> index 2cfb7915..6bc524c5 100644
> --- a/libdtrace/dt_prov_lockstat.c
> +++ b/libdtrace/dt_prov_lockstat.c
> @@ -169,7 +169,7 @@ static int populate(dtrace_hdl_t *dtp)
> int i;
> int n = 0;
>
> - prv = dt_provider_create(dtp, prvname, &dt_lockstat, &pattr);
> + prv = dt_provider_create(dtp, prvname, &dt_lockstat, &pattr, NULL);
> if (prv == NULL)
> return 0;
>
> diff --git a/libdtrace/dt_prov_proc.c b/libdtrace/dt_prov_proc.c
> index e7fbdca2..a5f3c8c0 100644
> --- a/libdtrace/dt_prov_proc.c
> +++ b/libdtrace/dt_prov_proc.c
> @@ -121,7 +121,7 @@ static int populate(dtrace_hdl_t *dtp)
> int i;
> int n = 0;
>
> - prv = dt_provider_create(dtp, prvname, &dt_proc, &pattr);
> + prv = dt_provider_create(dtp, prvname, &dt_proc, &pattr, NULL);
> if (prv == NULL)
> return 0;
>
> diff --git a/libdtrace/dt_prov_profile.c b/libdtrace/dt_prov_profile.c
> index 22dcca5c..476dea7c 100644
> --- a/libdtrace/dt_prov_profile.c
> +++ b/libdtrace/dt_prov_profile.c
> @@ -76,7 +76,7 @@ static int populate(dtrace_hdl_t *dtp)
> int profile_n[] = { 97, 199, 499, 997, 1999, 4001, 4999 };
> int tick_n[] = { 1, 10, 100, 500, 1000, 5000 };
>
> - prv = dt_provider_create(dtp, prvname, &dt_profile, &pattr);
> + prv = dt_provider_create(dtp, prvname, &dt_profile, &pattr, NULL);
> if (prv == NULL)
> return 0;
>
> diff --git a/libdtrace/dt_prov_rawtp.c b/libdtrace/dt_prov_rawtp.c
> index 9148e715..1ff0a311 100644
> --- a/libdtrace/dt_prov_rawtp.c
> +++ b/libdtrace/dt_prov_rawtp.c
> @@ -71,7 +71,7 @@ static int populate(dtrace_hdl_t *dtp)
> char *p;
> size_t n;
>
> - prv = dt_provider_create(dtp, prvname, &dt_rawtp, &pattr);
> + prv = dt_provider_create(dtp, prvname, &dt_rawtp, &pattr, NULL);
> if (prv == NULL)
> return 0;
>
> diff --git a/libdtrace/dt_prov_sched.c b/libdtrace/dt_prov_sched.c
> index c691ea5d..5f717970 100644
> --- a/libdtrace/dt_prov_sched.c
> +++ b/libdtrace/dt_prov_sched.c
> @@ -147,7 +147,7 @@ static int populate(dtrace_hdl_t *dtp)
> int i;
> int n = 0;
>
> - prv = dt_provider_create(dtp, prvname, &dt_sched, &pattr);
> + prv = dt_provider_create(dtp, prvname, &dt_sched, &pattr, NULL);
> if (prv == NULL)
> return 0;
>
> diff --git a/libdtrace/dt_prov_sdt.c b/libdtrace/dt_prov_sdt.c
> index 0bbe0fa8..8690fc47 100644
> --- a/libdtrace/dt_prov_sdt.c
> +++ b/libdtrace/dt_prov_sdt.c
> @@ -68,7 +68,7 @@ static int populate(dtrace_hdl_t *dtp)
> char *p;
> size_t n;
>
> - prv = dt_provider_create(dtp, prvname, &dt_sdt, &pattr);
> + prv = dt_provider_create(dtp, prvname, &dt_sdt, &pattr, NULL);
> if (prv == NULL)
> return 0;
>
> diff --git a/libdtrace/dt_prov_syscall.c b/libdtrace/dt_prov_syscall.c
> index ff07d671..d2f2aac1 100644
> --- a/libdtrace/dt_prov_syscall.c
> +++ b/libdtrace/dt_prov_syscall.c
> @@ -75,7 +75,7 @@ static int populate(dtrace_hdl_t *dtp)
> char *buf = NULL;
> size_t n;
>
> - prv = dt_provider_create(dtp, prvname, &dt_syscall, &pattr);
> + prv = dt_provider_create(dtp, prvname, &dt_syscall, &pattr, NULL);
> if (prv == NULL)
> return 0;
>
> diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c
> index 929aa3f6..fb1a62b4 100644
> --- a/libdtrace/dt_prov_uprobe.c
> +++ b/libdtrace/dt_prov_uprobe.c
> @@ -62,11 +62,11 @@ dt_provimpl_t dt_usdt;
>
> static int populate(dtrace_hdl_t *dtp)
> {
> - dt_provider_create(dtp, dt_uprobe.name, &dt_uprobe, &pattr);
> + dt_provider_create(dtp, dt_uprobe.name, &dt_uprobe, &pattr, NULL);
> dt_provider_create(dtp, dt_uprobe_is_enabled.name,
> - &dt_uprobe_is_enabled, &pattr);
> - dt_provider_create(dtp, dt_pid.name, &dt_pid, &pattr);
> - dt_provider_create(dtp, dt_usdt.name, &dt_usdt, &pattr);
> + &dt_uprobe_is_enabled, &pattr, NULL);
> + dt_provider_create(dtp, dt_pid.name, &dt_pid, &pattr, NULL);
> + dt_provider_create(dtp, dt_usdt.name, &dt_usdt, &pattr, NULL);
>
> return 0;
> }
> @@ -231,7 +231,7 @@ static int provide_probe(dtrace_hdl_t *dtp, const pid_probespec_t *psp,
> /* Get (or create) the provider for the PID of the probe. */
> pvp = dt_provider_lookup(dtp, prv);
> if (pvp == NULL) {
> - pvp = dt_provider_create(dtp, prv, pvops, &pattr);
> + pvp = dt_provider_create(dtp, prv, pvops, &pattr, NULL);
> if (pvp == NULL)
> return -1;
> }
> diff --git a/libdtrace/dt_provider.c b/libdtrace/dt_provider.c
> index b18ffb50..e9509963 100644
> --- a/libdtrace/dt_provider.c
> +++ b/libdtrace/dt_provider.c
> @@ -91,7 +91,8 @@ dt_provider_lookup(dtrace_hdl_t *dtp, const char *name)
>
> dt_provider_t *
> dt_provider_create(dtrace_hdl_t *dtp, const char *name,
> - const dt_provimpl_t *impl, const dtrace_pattr_t *pattr)
> + const dt_provimpl_t *impl, const dtrace_pattr_t *pattr,
> + void *datap)
> {
> dt_provider_t *pvp;
>
> @@ -103,6 +104,7 @@ dt_provider_create(dtrace_hdl_t *dtp, const char *name,
> pvp->pv_probes = dt_idhash_create(pvp->desc.dtvd_name, NULL, 0, 0);
> pvp->pv_gen = dtp->dt_gen;
> pvp->pv_hdl = dtp;
> + pvp->pv_data = datap;
> dt_dprintf("creating provider %s\n", name);
>
> if (pvp->pv_probes == NULL) {
> diff --git a/libdtrace/dt_provider.h b/libdtrace/dt_provider.h
> index 14abc518..638e6c80 100644
> --- a/libdtrace/dt_provider.h
> +++ b/libdtrace/dt_provider.h
> @@ -62,7 +62,6 @@ typedef struct dt_provimpl {
> const struct dt_probe *prb);
> void (*probe_destroy)(dtrace_hdl_t *dtp, /* free provider data */
> void *datap);
> - void *prv_data; /* provider-specific data */
> } dt_provimpl_t;
>
> /* list dt_dtrace first */
> @@ -90,6 +89,7 @@ typedef struct dt_provider {
> ulong_t pv_gen; /* generation # that created me */
> dtrace_hdl_t *pv_hdl; /* pointer to containing dtrace_hdl */
> uint_t pv_flags; /* flags (see below) */
> + void *pv_data; /* provider-specific data */
> } dt_provider_t;
>
> #define DT_PROVIDER_INTF 0x1 /* provider interface declaration */
> @@ -99,7 +99,7 @@ typedef struct dt_provider {
> extern dt_provider_t *dt_provider_lookup(dtrace_hdl_t *, const char *);
> extern dt_provider_t *dt_provider_create(dtrace_hdl_t *, const char *,
> const dt_provimpl_t *,
> - const dtrace_pattr_t *);
> + const dtrace_pattr_t *, void *);
> extern int dt_provider_xref(dtrace_hdl_t *, dt_provider_t *, id_t);
>
> #ifdef __cplusplus
> --
> 2.40.1
>
>
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel
More information about the DTrace-devel
mailing list