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

[Eugene Loh]

On 07/06/2020 08:31 AM, Kris Van Hees wrote:

> 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.

Good point.  Yes, I agree.

> 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.

It seems to me that such an optimization would change semantics. E.g., 
let's imagine that these two probes are provided on demand:
     foo:bar:baz:name1
     foo:bar:baz:name2
What would "-n foo:bar:baz:name1 -n foo:bar:baz:*" do?

With provide-look, the second probe description would find both name1 
and name2.

With look-provide-look, the second probe description would see only name1.

Or am I missing something?

> 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.

How about we defer discussion of this change (mainly because I'm 
concerned about the change in semantics).

>>   	/*
>>   	 * 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).

Okay.

>>   	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