[DTrace-devel] [PATCH 06/13] Add provider-dependent struct in dt_probe_t

Kris Van Hees kris.van.hees at oracle.com
Mon Jul 6 14:50:44 PDT 2020


On Wed, Jul 01, 2020 at 10:41:11PM -0400, eugene.loh at oracle.com wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
> 
> The dt_probe_t struct had members like event_id and event_fd.
> This made sense for the providers we were supporting, since they
> were all tracepoint-like.  For future providers, however, those
> members might not make sense and others will be needed.  Therefore,
> replace event_id and event_fd with an opaque pointer to a
> provider-dependent struct.
> 
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
>  libdtrace/dt_probe.c        | 19 +++-------
>  libdtrace/dt_probe.h        |  4 +--
>  libdtrace/dt_prov_dtrace.c  | 42 +++++++++++++++-------
>  libdtrace/dt_prov_fbt.c     | 41 +++++++++++++++------
>  libdtrace/dt_prov_sdt.c     | 72 +++++++++++++++++++++++--------------
>  libdtrace/dt_prov_syscall.c | 34 +++++++++++++-----
>  libdtrace/dt_provider.h     |  6 ++--
>  7 files changed, 143 insertions(+), 75 deletions(-)
> 
> diff --git a/libdtrace/dt_probe.c b/libdtrace/dt_probe.c
> index be407dd2..496ba11e 100644
> --- a/libdtrace/dt_probe.c
> +++ b/libdtrace/dt_probe.c
> @@ -505,6 +505,7 @@ dt_probe_destroy(dt_probe_t *prp)
>  
>  	if (prp->prov && prp->prov->impl && prp->prov->impl->probe_fini)
>  		prp->prov->impl->probe_fini(dtp, prp);
> +	dt_free(dtp, prp->prv_data);

OK

>  
>  	dt_node_list_free(&prp->nargs);
>  	dt_node_list_free(&prp->xargs);
> @@ -665,7 +666,8 @@ dt_probe_tag(dt_probe_t *prp, uint_t argn, dt_node_t *dnp)
>  }
>  
>  dt_probe_t *
> -dt_probe_insert(dtrace_hdl_t *dtp, dt_provider_t *prov, const char *prv,
> +dt_probe_insert(dtrace_hdl_t *dtp, dt_provider_t *prov, const void *prv_data,
> +		const char *prv,
>  		const char *mod, const char *fun, const char *prb)

Please move the prv_data to the end of the argument list, which is more
consistent with other code that supports opaque pointers.  It also shows its
status as auxilliary information rather than being mixed with the elements
that are part of the generic probe.  And perhaps name it datap since that is
the name you used for the provider-specific data variable in other places.

I would also modify dt_probe_insert() to free datap (if not NULL) if the probe
cannot be created.  That avoids needing to have each caller check if the probe 
got created, and if not, free datap.  You already do a freeing of the datap in
dt_probr_destroy() so doing it in dt_probe_insert() as well (when needed) is
OK.  The caller can count on the fact that if the probe could not be created,
datap is no longer valid.

>  {
>  	dt_probe_t		*prp;
> @@ -711,8 +713,7 @@ dt_probe_insert(dtrace_hdl_t *dtp, dt_provider_t *prov, const char *prv,
>  
>  	prp->desc = desc;
>  	prp->prov = prov;
> -	prp->event_id = -1;
> -	prp->event_fd = -1;
> +	prp->prv_data = prv_data;
>  
>  	dt_htab_insert(dtp->dt_byprv, prp);
>  	dt_htab_insert(dtp->dt_bymod, prp);
> @@ -872,26 +873,16 @@ dt_probe_delete(dtrace_hdl_t *dtp, dt_probe_t *prp)
>  static void
>  dt_probe_args_info(dtrace_hdl_t *dtp, dt_probe_t *prp)
>  {
> -	int			id = DTRACE_IDNONE;
>  	int			argc = 0;
>  	dt_argdesc_t		*argv = NULL;
>  	int			i, nc, xc;
>  	dtrace_typeinfo_t	dtt;
>  
> -	/*
> -	 * If we already have an event ID information for this probe, there is
> -	 * no need to retrieve it again.
> -	 */
> -	if (prp->event_id != -1)
> -		return;
> -
>  	if (!prp->prov->impl->probe_info)
>  		return;
> -	if (prp->prov->impl->probe_info(dtp, prp, &id, &argc, &argv) < 0)
> +	if (prp->prov->impl->probe_info(dtp, prp, &argc, &argv) < 0)
>  		return;
>  
> -	if (id > 0)
> -		prp->event_id = id;
>  	if (!argc || !argv)
>  		return;
>  
> diff --git a/libdtrace/dt_probe.h b/libdtrace/dt_probe.h
> index 7feeeff2..92768f9b 100644
> --- a/libdtrace/dt_probe.h
> +++ b/libdtrace/dt_probe.h
> @@ -38,8 +38,7 @@ typedef struct dt_probe {
>  	struct dt_hentry he_fun;	/* function name htab links */
>  	struct dt_hentry he_prb;	/* probe name htab link */
>  	struct dt_hentry he_fqn;	/* fully qualified name htab link */
> -	int event_id;			/* tracing event id */
> -	int event_fd;			/* perf event file descriptor */
> +	void *prv_data;			/* provider-specific data */

OK.

>  	dt_ident_t *pr_ident;		/* pointer to probe identifier */
>  	const char *pr_name;		/* pointer to name component */
>  	dt_node_t *nargs;		/* native argument list */
> @@ -70,6 +69,7 @@ extern int dt_probe_define(dt_provider_t *, dt_probe_t *,
>  extern dt_node_t *dt_probe_tag(dt_probe_t *, uint_t, dt_node_t *);
>  
>  extern dt_probe_t *dt_probe_insert(dtrace_hdl_t *dtp, dt_provider_t *prov,
> +				     const void *prv_data,
>  				     const char *prv, const char *mod,
>  				     const char *fun, const char *prb);

Move to end of parameter list.

>  extern dt_probe_t *dt_probe_lookup(dtrace_hdl_t *dtp,
> diff --git a/libdtrace/dt_prov_dtrace.c b/libdtrace/dt_prov_dtrace.c
> index 34c99388..c5c2a84d 100644
> --- a/libdtrace/dt_prov_dtrace.c
> +++ b/libdtrace/dt_prov_dtrace.c
> @@ -15,6 +15,11 @@
>  #include "dt_provider.h"
>  #include "dt_probe.h"
>  
> +typedef struct prv_data {
> +	int	event_id;
> +	int	event_fd;
> +} prv_data_t;

Since multiple providers use the tracepoint-based mechanism, the provider
specific data is identical for these providers and can therefore be declared
in a more central location.  I would put it in dt_provider.h (where the
prototypes for tp_event_info() and tp_attach() sre defined..

I would not use the same name for the struct, arg name and member name.
Since this is common for all tracepoint-based providers, how about using
tp_probe (and tp_probe_t)?  Other names can be profile_probe_t, and so on.
That is similar to the naming in DTrace v1 (at the kernel level) where e.g.
FBT specific information was in fbt_probe_t.

> +
>  static const char		prvname[] = "dtrace";
>  static const char		modname[] = "";
>  static const char		funname[] = "";
> @@ -34,18 +39,26 @@ static const dtrace_pattr_t	pattr = {
>  static int populate(dtrace_hdl_t *dtp)
>  {
>  	dt_provider_t	*prv;
> -	int		n = 0;
> +	int		i, n = 0;
> +	char		*prbnames[] = { "BEGIN", "END", "ERROR" };

This (along with the code below) is a bit of an issue because the ERROR probe
is not the same as the BEGIN and END probes.  BEGIN/END are implemented as
uprobes, whereas ERROR is not implemented as a probe that is attached to
anything at the kernel level.  All three belong in the dtrace provider, but
the behaviour is definitely different.
 
>  	prv = dt_provider_create(dtp, prvname, &dt_dtrace, &pattr);
>  	if (prv == NULL)
>  		return 0;
>  
> -	if (dt_probe_insert(dtp, prv, prvname, modname, funname, "BEGIN"))
> -		n++;
> -	if (dt_probe_insert(dtp, prv, prvname, modname, funname, "END"))
> -		n++;
> -	if (dt_probe_insert(dtp, prv, prvname, modname, funname, "ERROR"))
> -		n++;
> +	for (i = 0; i < sizeof (prbnames) / sizeof (*prbnames); i++) {
> +		prv_data_t *datap;
> +
> +		datap = dt_zalloc(dtp, sizeof (prv_data_t));
> +		if (datap == NULL)
> +			continue;
> +		datap->event_id = -1;
> +		datap->event_fd = -1;

I think that the above should be done in a tp_probe_alloc() function in the
SDT provider code (where the other generic tp_* functions are) since we should
use the same struct for all tracepoint-based providers anyway.

So the next statement becomes:

		if (dt_probe_insert(dtp, prv, datap, prvname, modname, funname,
				    prbnames[i], tp_probe_event()))

It is also possible to add a tp_probe_insert() function that wraps the call
to dt_probe_insert along with allocation of the tp_probe_t.  Either solution
seems reasonable to me.

If you prefer the tp_probe_insert() wrapper function, adjust your reading of
my further comments in that context (I am commenting as if there is a
tp_probe_alloc() function that will be used in dt_probe_insert() calls).

> +		if (dt_probe_insert(dtp, prv, datap, prvname, modname, funname, prbnames[i]))

The datap argument would move to the end per my comments above.

> +			n++;
> +		else
> +			dt_free(dtp, datap);

Already done in dt_probe_insert() if the probe cannot be created (see above).
> +	}
>  
>  	return n;
>  }
> @@ -230,7 +243,7 @@ out:
>  }
>  
>  static int probe_info(dtrace_hdl_t *dtp, const dt_probe_t *prp,
> -		      int *idp, int *argcp, dt_argdesc_t **argvp)
> +		      int *argcp, dt_argdesc_t **argvp)
>  {
>  	char	*spec;
>  	char	*fn = NULL;
> @@ -238,8 +251,12 @@ static int probe_info(dtrace_hdl_t *dtp, const dt_probe_t *prp,
>  	FILE	*f;
>  	int	rc = -ENOENT;
>  	size_t	len;
> +	int	*idp = &((prv_data_t *)(prp->prv_data))->event_id;
> +
> +        /* if we have an event ID, no need to retrieve it again */
> +        if (*idp != -1)
> +		return -1;

I think it would be more clear if you did this as:

	struct tp_probe_t	*datap = prp->prv_data;

	if (datap->event_id != -1)
		return -1;

> -	*idp = -1;
>  	*argcp = 0;			/* no arguments */
>  	*argvp = NULL;

and then later on you can do:

	rc = tp_event_info(dtp, f, 0, datap, NULL, NULL);

and tp_event_info() can take a tp_probe_t * argument and do with it whatever
it needs to (in this case, assign event_id).

> @@ -295,10 +312,11 @@ out:
>  static int probe_fini(dtrace_hdl_t *dtp, dt_probe_t *prp)
>  {
>  	int	fd;
> +	prv_data_t *datap = prp->prv_data;
>  
> -	if (prp->event_fd != -1) {
> -		close(prp->event_fd);
> -		prp->event_fd = -1;
> +	if (datap->event_fd != -1) {
> +		close(datap->event_fd);
> +		datap->event_fd = -1;
>  	}

Make this into a standard tp_probe_finit and use it in providers that are
tracepoint-based and need no further cleanup?  Or you could introduce a new
provider function (detach) to handle this, and leave probe_fini in cases
where we need to perform further cleanup,  Perhaps that is nice (even though
it means more functions) because it provides a balanced set of provider
operations attach vs detach, populate/provide vs probe_fini).  What do you
think?

>  	fd = open(UPROBE_EVENTS, O_WRONLY | O_APPEND);
> diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c
> index cb80082e..a58584c0 100644
> --- a/libdtrace/dt_prov_fbt.c
> +++ b/libdtrace/dt_prov_fbt.c
> @@ -37,6 +37,11 @@
>  #include "dt_probe.h"
>  #include "dt_pt_regs.h"
>  
> +typedef struct prv_data {
> +	int	event_id;
> +	int	event_fd;
> +} prv_data_t;

See above.

> +
>  static const char		prvname[] = "fbt";
>  static const char		modname[] = "vmlinux";
>  
> @@ -65,9 +70,10 @@ static int populate(dtrace_hdl_t *dtp)
>  	char			buf[256];
>  	char			*p;
>  	const char		*mod = modname;
> -	int			n = 0;
> +	int			i, n = 0;
>  	dtrace_syminfo_t	sip;
>  	dtrace_probedesc_t	pd;
> +	char			*prbnames[] = { "entry", "return" };

Not necessary (see below).

>  	prv = dt_provider_create(dtp, prvname, &dt_fbt, &pattr);
>  	if (prv == NULL)
> @@ -136,10 +142,20 @@ static int populate(dtrace_hdl_t *dtp)
>  		if (dt_probe_lookup(dtp, &pd) != NULL)
>  			continue;
>  
> -		if (dt_probe_insert(dtp, prv, prvname, mod, buf, "entry"))
> -			n++;
> -		if (dt_probe_insert(dtp, prv, prvname, mod, buf, "return"))
> -			n++;
> +		for (i = 0; i < sizeof (prbnames) / sizeof (*prbnames); i++) {
> +			prv_data_t *datap;
> +
> +			datap = dt_zalloc(dtp, sizeof (prv_data_t));
> +			if (datap == NULL)
> +				continue;
> +			datap->event_id = -1;
> +			datap->event_fd = -1;
> +			if (dt_probe_insert(dtp, prv, datap, prvname, mod,
> +				            buf, prbnames[i]))
> +				n++;
> +			else
> +				dt_free(dtp, datap);
> +		}

This could become (based on earlier comments):

		if (dt_probe_insert(dtp, prv, prvname, mod, buf, "entry",
				    tp_probe_alloc()))
			n++
		if (dt_probe_insert(dtp, prv, prvname, mod, buf, "return",
				    tp_probe_alloc()))
			n++
>  	}
>  
>  	fclose(f);
> @@ -273,15 +289,19 @@ static void trampoline(dt_pcb_t *pcb)
>  }
>  
>  static int probe_info(dtrace_hdl_t *dtp, const dt_probe_t *prp,
> -		      int *idp, int *argcp, dt_argdesc_t **argvp)
> +		      int *argcp, dt_argdesc_t **argvp)
>  {
>  	int	fd;
>  	char	*fn;
>  	size_t	len;
>  	FILE	*f;
>  	int	rc = -1;
> +	int	*idp = &((prv_data_t *)(prp->prv_data))->event_id;
> +
> +        /* if we have an event ID, no need to retrieve it again */
> +        if (*idp != -1)
> +		return -1;

See above (for dt_prov_dtrace).

> -	*idp = -1;
>  	*argcp = 0;			/* no arguments by default */
>  	*argvp = NULL;
>  
> @@ -335,10 +355,11 @@ out:
>  static int probe_fini(dtrace_hdl_t *dtp, dt_probe_t *prp)
>  {
>  	int	fd;
> +	prv_data_t *datap = prp->prv_data;
>  
> -	if (prp->event_fd != -1) {
> -		close(prp->event_fd);
> -		prp->event_fd = -1;
> +	if (datap->event_fd != -1) {
> +		close(datap->event_fd);
> +		datap->event_fd = -1;
>  	}

See above (for dt_prov_dtrace).  If you introduce a detach(dtp, prp) then you
can have a custom detahc here that calls tp_detach(dtp, prp) to do the actual
detach and then do the rest of the cleanup.  Or if you prefer to stick with
probe_fini (no detach) then you can have this on call a tp_probe_fini(dtp, prp)
to handle the common detach-cleanup and then continue with the provider specific
cleanup.

>  	fd = open(KPROBE_EVENTS, O_WRONLY | O_APPEND);
> diff --git a/libdtrace/dt_prov_sdt.c b/libdtrace/dt_prov_sdt.c
> index 1a445c65..9a76f029 100644
> --- a/libdtrace/dt_prov_sdt.c
> +++ b/libdtrace/dt_prov_sdt.c
> @@ -34,6 +34,11 @@
>  #include "dt_probe.h"
>  #include "dt_pt_regs.h"
>  
> +typedef struct prv_data {
> +	int	event_id;
> +	int	event_fd;
> +} prv_data_t;

See above.  Put this in dt_provider.h, with names tp_probe / tp_probe_t.

> +
>  static const char		prvname[] = "sdt";
>  static const char		modname[] = "vmlinux";
>  
> @@ -226,29 +231,31 @@ done:
>  	return 0;
>  }
>  
> -int tp_event_attach(dtrace_hdl_t *dtp, dt_probe_t *prp, int bpf_fd)
> +int tp_event_attach(dtrace_hdl_t *dtp, const dt_probe_t *prp, int bpf_fd)

tp_attach (per comments to an earlier patch)

>  {
> -        if (prp->event_fd == -1) {
> -                int                     fd;
> -                struct perf_event_attr  attr = { 0, };
> +	prv_data_t *datap = prp->prv_data;

tp_probe_t *

Add:
	if (datap->event_id == -1)
		return 0;

because if there is no event id, there is nothing to attach to, and we are
done.  But it might still be a valid attach request, so we return 0.

> +	if (datap->event_fd == -1) {
> +		int                     fd;
> +		struct perf_event_attr  attr = { 0, };
>  
> -                attr.type = PERF_TYPE_TRACEPOINT;
> -                attr.sample_type = PERF_SAMPLE_RAW;
> -                attr.sample_period = 1;
> -                attr.wakeup_events = 1;
> -                attr.config = prp->event_id;
> +		attr.type = PERF_TYPE_TRACEPOINT;
> +		attr.sample_type = PERF_SAMPLE_RAW;
> +		attr.sample_period = 1;
> +		attr.wakeup_events = 1;
> +		attr.config = datap->event_id;
>  
> -                fd = perf_event_open(&attr, -1, 0, -1, 0);
> -                if (fd < 0)
> -                        return dt_set_errno(dtp, errno);
> +		fd = perf_event_open(&attr, -1, 0, -1, 0);
> +		if (fd < 0)
> +			return dt_set_errno(dtp, errno);
>  
> -                prp->event_fd = fd;
> -        }
> +		datap->event_fd = fd;
> +	}
>  
> -        if (ioctl(prp->event_fd, PERF_EVENT_IOC_SET_BPF, bpf_fd) < 0)
> -                return dt_set_errno(dtp, errno);
> +	if (ioctl(datap->event_fd, PERF_EVENT_IOC_SET_BPF, bpf_fd) < 0)
> +		return dt_set_errno(dtp, errno);
>  
> -        return 0;
> +	return 0;
>  }
>  
>  /*
> @@ -275,8 +282,10 @@ static int populate(dtrace_hdl_t *dtp)
>  		return 0;
>  
>  	while (fgets(buf, sizeof(buf), f)) {
> -		int	dummy;
> -		char	str[sizeof(buf)];
> +		int		dummy;
> +		char		str[sizeof(buf)];
> +		const char	*mod, *prb;
> +		prv_data_t	*datap;
>  
>  		p = strchr(buf, '\n');
>  		if (p)
> @@ -301,13 +310,21 @@ static int populate(dtrace_hdl_t *dtp)
>  				 strcmp(buf, UPROBES) == 0)
>  				continue;
>  
> -			if (dt_probe_insert(dtp, prv, prvname, buf, "", p))
> -				n++;
> +			mod = buf;
> +			prb = p;
>  		} else {
> -			if (dt_probe_insert(dtp, prv, prvname, modname, "",
> -					    buf))
> -				n++;
> +			mod = modname;
> +			prb = buf;
>  		}
> +		datap = dt_zalloc(dtp, sizeof (prv_data_t));
> +		if (datap == NULL)
> +			continue;
> +		datap->event_id = -1;
> +		datap->event_fd = -1;
> +		if (dt_probe_insert(dtp, prv, datap, prvname, mod, "", prb))
> +			n++;
> +		else
> +			dt_free(dtp, datap);

This can become:

			if (dt_probe_insert(dtp, prv, prvname, buf, "", p,
					    tp_probe_alloc()))
				n++;
		} else {
			if (dt_probe_insert(dtp, prv, prvname, modname, "", buf,
					    tp_probe_alloc()))
				n++;
		}

>  	}
>  
>  	fclose(f);
> @@ -408,13 +425,16 @@ static void trampoline(dt_pcb_t *pcb)
>  }
>  
>  static int probe_info(dtrace_hdl_t *dtp, const dt_probe_t *prp,
> -		      int *idp, int *argcp, dt_argdesc_t **argvp)
> +		      int *argcp, dt_argdesc_t **argvp)
>  {
>  	FILE	*f;
>  	char	fn[256];
>  	int	rc;
> +	int	*idp = &((prv_data_t *)(prp->prv_data))->event_id;
>  
> -	*idp = -1;
> +	/* if we have an event ID, no need to retrieve it again */
> +	if (*idp != -1)
> +		return -1;

See above.

>  	strcpy(fn, EVENTSFS);
>  	strcat(fn, prp->desc->mod);
> diff --git a/libdtrace/dt_prov_syscall.c b/libdtrace/dt_prov_syscall.c
> index cb54ce5a..ecab4545 100644
> --- a/libdtrace/dt_prov_syscall.c
> +++ b/libdtrace/dt_prov_syscall.c
> @@ -35,6 +35,11 @@
>  #include "dt_probe.h"
>  #include "dt_pt_regs.h"
>  
> +typedef struct prv_data {
> +	int	event_id;
> +	int	event_fd;
> +} prv_data_t;

See above.

>  static const char		prvname[] = "syscall";
>  static const char		modname[] = "vmlinux";
>  
> @@ -85,6 +90,7 @@ static int populate(dtrace_hdl_t *dtp)
>  
>  	while (fgets(buf, sizeof(buf), f)) {
>  		char	*p;
> +		char	*prb;
>  
>  		/* Here buf is "group:event".  */
>  		p = strchr(buf, '\n');
> @@ -107,22 +113,31 @@ static int populate(dtrace_hdl_t *dtp)
>  		p = buf;
>  		if (memcmp(p, PROV_PREFIX, sizeof(PROV_PREFIX) - 1) != 0)
>  			continue;
> -

I think the empty line here adds clarity, in the sense that the conditional
is important to highlight as a condition to not continue execution in this
function.  The next statement is part of the actual work done in this function.

>  		p += sizeof(PROV_PREFIX) - 1;
> +
>  		/*
>  		 * Now p will be just "event", and we are only interested in
>  		 * events that match "sys_enter_*" or "sys_exit_*".
>  		 */
> +		prb = NULL;
>  		if (!memcmp(p, ENTRY_PREFIX, sizeof(ENTRY_PREFIX) - 1)) {
>  			p += sizeof(ENTRY_PREFIX) - 1;
> -			if (dt_probe_insert(dtp, prv, prvname, modname, p,
> -					    "entry"))
> -				n++;
> +			prb = "entry";
>  		} else if (!memcmp(p, EXIT_PREFIX, sizeof(EXIT_PREFIX) - 1)) {
>  			p += sizeof(EXIT_PREFIX) - 1;
> -			if (dt_probe_insert(dtp, prv, prvname, modname, p,
> -					    "return"))
> +			prb = "return";
> +		}
> +		if (prb != NULL) {
> +			prv_data_t *datap = dt_zalloc(dtp, sizeof (prv_data_t));
> +
> +			if (datap == NULL)
> +				continue;
> +			datap->event_id = -1;
> +			datap->event_fd = -1;
> +			if (dt_probe_insert(dtp, prv, datap, prvname, modname, p, prb))
>  				n++;
> +			else
> +				dt_free(dtp, datap);

Becomes:
		if (!memcmp(p, ENTRY_PREFIX, sizeof(ENTRY_PREFIX) - 1)) {
			p += sizeof(ENTRY_PREFIX) - 1;
			if (dt_probe_insert(dtp, prv, prvname, modname, p,
					    "entry", tp_probe_alloc()))
				n++;
		} else if (!memcmp(p, EXIT_PREFIX, sizeof(EXIT_PREFIX) - 1)) {
			p += sizeof(EXIT_PREFIX) - 1;
			if (dt_probe_insert(dtp, prv, prvname, modname, p,
					    "return", tp_probe_alloc()))
				n++;

>  		}
>  	}
>  
> @@ -235,13 +250,16 @@ static void trampoline(dt_pcb_t *pcb)
>  }
>  
>  static int probe_info(dtrace_hdl_t *dtp, const dt_probe_t *prp,
> -		      int *idp, int *argcp, dt_argdesc_t **argvp)
> +		      int *argcp, dt_argdesc_t **argvp)
>  {
>  	FILE	*f;
>  	char	fn[256];
>  	int	rc;
> +	int	*idp = &((prv_data_t *)(prp->prv_data))->event_id;
>  
> -	*idp = -1;
> +	/* if we have an event ID, no need to retrieve it again */
> +	if (*idp != -1)
> +		return -1;

See above.

>  	/*
>  	 * We know that the probe name is either "entry" or "return", so we can
> diff --git a/libdtrace/dt_provider.h b/libdtrace/dt_provider.h
> index acd01983..9ea89898 100644
> --- a/libdtrace/dt_provider.h
> +++ b/libdtrace/dt_provider.h
> @@ -61,19 +61,19 @@ typedef struct dt_provimpl {
>  	int (*populate)(dtrace_hdl_t *dtp);	/* register probes */
>  	int (*probe_info)(dtrace_hdl_t *dtp,	/* get probe info */
>  			  const struct dt_probe *prp,
> -			  int *idp, int *argcp, dt_argdesc_t **argvp);
> +			  int *argcp, dt_argdesc_t **argvp);
>  	int (*provide)(dtrace_hdl_t *dtp,	/* provide probes */
>  		       const dtrace_probedesc_t *pdp);
>  	void (*trampoline)(dt_pcb_t *pcb);	/* generate BPF trampoline */
>  	int (*attach)(dtrace_hdl_t *dtp,	/* attach BPF program to probe */
> -		      struct dt_probe *prp, int bpf_fd);
> +		      const struct dt_probe *prp, int bpf_fd);
>  	int (*probe_fini)(dtrace_hdl_t *dtp,	/* probe cleanup */
>  			  struct dt_probe *prb);

const dt_probe *prp

>  } dt_provimpl_t;
>  
>  extern int tp_event_info(dtrace_hdl_t *dtp, FILE *f, int skip, int *idp,
>  			 int *argcp, dt_argdesc_t **argvp);
> -extern int tp_event_attach(dtrace_hdl_t *dtp, struct dt_probe *prp, int bpf_fd);
> +extern int tp_event_attach(dtrace_hdl_t *dtp, const struct dt_probe *prp, int bpf_fd);

tp_attach (per comments to an earlier patch)

>  extern dt_provimpl_t dt_dtrace;
>  extern dt_provimpl_t dt_fbt;
> -- 
> 2.18.2
> 
> 
> _______________________________________________
> 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