[DTrace-devel] [PATCH 07/17] providers: allow providers with no trampoline

[Eugene Loh]

Reviewed-by: Eugene Loh <eugene.loh at oracle.com>

It's a simple patch and looks right, but I'm a little confused. When is 
the dt_cg.c diff needed?  I tried some pid$foo probes (with the later 
patch that removes .trampoline for that provider), and it seems that I 
never get trampoline==NULL even though I do see trampoline==NULL for the 
dt_bpf.c code site.  Is the dt_cg.c diff "just to be safe?"  "For some 
future case?"  Unnecessary?  Or am I missing something?

On 8/10/22 18:06, Nick Alcock via DTrace-devel wrote:

> These can be enabled just like any other, but generate no code.

This sentence confuses me.  "These what?"  At the very least, "these 
providers"?  And presumably there are no such providers at the time of 
this patch.  How about dumping this sentence?

> Meant for use by things like the pid provider which have two
> probes associated with each system-side probe, one of which
> gets fired and generates code for the others in its trampoline
> method.

This also confuses me.  How about, "In a future patch, the PID-specific 
provider dt_pid_proc will no longer have a trampoline."  For the current 
patch, we only need to handle the case of .trampoline==NULL.  As far as 
the motivation goes, it suffices to refer to the future patch.

> ---
>   libdtrace/dt_bpf.c | 7 +++++++
>   libdtrace/dt_cg.c  | 3 ++-
>   2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
> index 5ff348ca6370..42b4b80c0a68 100644
> --- a/libdtrace/dt_bpf.c
> +++ b/libdtrace/dt_bpf.c
> @@ -719,6 +719,13 @@ dt_bpf_load_progs(dtrace_hdl_t *dtp, uint_t cflags)
>   		if (prp == dtp->dt_error)
>   			continue;
>   
> +		/*
> +		 * Enabled probes with no trampoline act like they exist but
> +		 * no code is generated for them.
> +		 */
> +		if (prp->prov->impl->trampoline == NULL)
> +			continue;
> +
>   		dp = dt_program_construct(dtp, prp, cflags, NULL);
>   		if (dp == NULL)
>   			return -1;
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index ebd4190698f7..9f9d0fb03028 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -6846,7 +6846,8 @@ dt_cg(dt_pcb_t *pcb, dt_node_t *dnp)
>   	} else if (dnp->dn_kind == DT_NODE_TRAMPOLINE) {
>   		assert(pcb->pcb_probe != NULL);
>   
> -		pcb->pcb_probe->prov->impl->trampoline(pcb);
> +		if (pcb->pcb_probe->prov->impl->trampoline != NULL)
> +			pcb->pcb_probe->prov->impl->trampoline(pcb);
>   	} else
>   		dt_cg_node(dnp, &pcb->pcb_ir, pcb->pcb_regs);
>