[DTrace-devel] [PATCH 04/13] Add ability to provide user-specified probes on demand

[Kris Van Hees]

On Wed, Jul 01, 2020 at 10:41:09PM -0400, eugene.loh at oracle.com wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
> 
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
>  libdtrace/dt_probe.c    | 20 ++++++++++++++++++++
>  libdtrace/dt_provider.h |  2 ++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/libdtrace/dt_probe.c b/libdtrace/dt_probe.c
> index 46c47fbe..be407dd2 100644
> --- a/libdtrace/dt_probe.c
> +++ b/libdtrace/dt_probe.c
> @@ -1141,6 +1141,26 @@ dt_probe_iter(dtrace_hdl_t *dtp, const dtrace_probedesc_t *pdp,
>  
>  	tmpl.desc = pdp;
>  
> +	/*
> +	 * Special case: if the probe name is specified,
> +	 * give all providers the opportunity to provide it.
> +	 */
> +	if (n_is_glob == 0) {
> +		dt_provider_t *pvp;
> +		dtrace_probedesc_t pd;
> +
> +		/* loop over providers */
> +		for (pvp = dt_list_next(&dtp->dt_provlist);
> +		    pvp != NULL; pvp = dt_list_next(pvp)) {
> +			if (pvp->impl->provide == NULL ||
> +			    !dt_gmatch(pvp->desc.dtvd_name, pdp->prv))
> +				continue;
> +			memcpy(&pd, pdp, sizeof (pd));
> +			pd.prv = pvp->desc.dtvd_name;
> +			pvp->impl->provide(dtp, &pd);
> +		}
> +	}
> +

You introduce a limitation here that did not exist in DTrace v1 and I do not
see a reason why we would be more restrictive than DTrace v1.  Why would we not
allow globs in the probe name?  Any filtering on probe descriptions that a
provider might be able to work with (nor not) should be at the level of the
provider.

Now, for efficiency sake, I do think we might be able to implement this slightly
differently from DTrace v1 without creating different behaviour.  I think it
would be valid to first try to find a matching probe first and if one is not
found, to then give all providers a chance to provide it.  We could then loop
back to performing the lookup one more time.

That is an optimiztion that we could leave for later but it seems to be a very
simple and obvious one.  Most of the time, probes people refer to will already
exist.  The lookup process is also a lot faster than DTrace v1 could provide
(due to the userspace-kernel interaction), so the penalty for
lookup-provide-lookup is very small.

>  	/*
>  	 * Special case: if the probe is fully specified (none of the elements
>  	 * are empty of a glob pattern, we can do a direct lookup based on the
> diff --git a/libdtrace/dt_provider.h b/libdtrace/dt_provider.h
> index e98bfac1..50f663f1 100644
> --- a/libdtrace/dt_provider.h
> +++ b/libdtrace/dt_provider.h
> @@ -62,6 +62,8 @@ typedef struct dt_provimpl {
>  	int (*probe_info)(dtrace_hdl_t *dtp,	/* get probe info */
>  			  const struct dt_probe *prp,
>  			  int *idp, int *argcp, dt_argdesc_t **argvp);
> +	int (*provide)(dtrace_hdl_t *dtp,	/* provide probes */
> +		       const dtrace_probedesc_t *pdp);

I think it would be more appropriate to place this right after the populate
callback because they perform the same kind of function (add probes to the
global probe list).

>  	void (*trampoline)(dt_pcb_t *pcb);	/* generate BPF trampoline */
>  	int (*probe_fini)(dtrace_hdl_t *dtp,	/* probe cleanup */
>  			  struct dt_probe *prb);
> -- 
> 2.18.2
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel