[DTrace-devel] [PATCH 5/5] Add -xcpu support for general providers
Eugene Loh
eugene.loh at oracle.com
Wed Dec 20 21:49:02 UTC 2023
On 12/20/23 12:09, Kris Van Hees wrote:
> On Tue, Sep 05, 2023 at 12:11:42AM -0400, eugene.loh at oracle.com wrote:
>> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> I do not understand this patch, or rather why you rename
> dt_cg_tramp_prologue_act
> to be
> dt_cg_tramp_prologue_sub
> yet the implementation does not change?
Yeah, good question. I no longer remember. Possibly I did something
else to motivate the name change and then changed my mind and forgot to
get rid of the name change. Okay: back to _act.
> In all, I think it might be better to add a 2nd argument to
> dt_cg_tramp_prologue() to indicate whether CPU filtering should be done (and
> those who do it by means of how probes are attched can disable that). Maybe
> an argument named 'no_cpu' to be 1 to skip CPU filtering if -xcpu is set.
I don't understand in what sense this is better.
> Or alterntively, add a new dt_cg_tramp_prologue_cpu(dt_pcb_t *pcb) that simply
> does dt_cg_tramp_prologue_act(pcb, DT_ACTIVITY_ACTIVE);
I definitely do not understand this. This sounds like the name change
that puzzled you except that _cpu is an even worse name than _sub. You
must mean something different from what I'm reading here.
> and then change the
> dt_cg_tramp_prologue() function to generate the CPU filtering if -xcpu is set,
> and then call dt_cg_tramp_prologue_cpu(). That was, as much as of this as can
> be done remains hidden from the provider code.
I do not see how having to choose between no_cpu = 0 and 1 hides any
more than having to choose between trampoline and trampoline_act.
I must be missing what you're trying to say.
>> diff --git a/CODING-STYLE b/CODING-STYLE
>> index c8013f05..f6382c65 100644
>> --- a/CODING-STYLE
>> +++ b/CODING-STYLE
>> @@ -26,7 +26,7 @@ E.g.,
>> uint_t
>> dt_cg_tramp_prologue(dt_pcb_t *pcb)
>> {
>> - return dt_cg_tramp_prologue_act(pcb, DT_ACTIVITY_ACTIVE);
>> + return dt_cg_tramp_prologue_sub(pcb, DT_ACTIVITY_ACTIVE);
>> }
>>
>> In an "if...else if...else" statement, omit braces for the final branch if
>> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
>> index 0ed662f9..a79b8352 100644
>> --- a/libdtrace/dt_cg.c
>> +++ b/libdtrace/dt_cg.c
>> @@ -49,7 +49,7 @@ static void dt_cg_node(dt_node_t *, dt_irlist_t *, dt_regset_t *);
>> * %r9 contains a pointer to dctx
>> */
>> void
>> -dt_cg_tramp_prologue_act(dt_pcb_t *pcb, dt_activity_t act)
>> +dt_cg_tramp_prologue_sub(dt_pcb_t *pcb, dt_activity_t act)
>> {
>> dtrace_hdl_t *dtp = pcb->pcb_hdl;
>> dt_irlist_t *dlp = &pcb->pcb_ir;
>> @@ -274,7 +274,24 @@ dt_cg_tramp_prologue_act(dt_pcb_t *pcb, dt_activity_t act)
>> void
>> dt_cg_tramp_prologue(dt_pcb_t *pcb)
>> {
>> - dt_cg_tramp_prologue_act(pcb, DT_ACTIVITY_ACTIVE);
>> + dtrace_hdl_t *dtp = pcb->pcb_hdl;
>> +
>> + /* Check if we are on the specified CPU (if any). */
>> + if (dtp->dt_options[DTRACEOPT_CPU] != DTRACEOPT_UNSET) {
>> + dt_irlist_t *dlp = &pcb->pcb_ir;
>> +
>> + emit(dlp, BPF_MOV_REG(BPF_REG_8, BPF_REG_1));
>> +
>> + emit(dlp, BPF_CALL_HELPER(BPF_FUNC_get_smp_processor_id));
>> + emit(dlp, BPF_BRANCH_IMM(BPF_JNE, BPF_REG_0,
>> + dtp->dt_options[DTRACEOPT_CPU],
>> + pcb->pcb_exitlbl));
>> +
>> + emit(dlp, BPF_MOV_REG(BPF_REG_1, BPF_REG_8));
>> + }
>> +
>> + /* Call the rest of the prologue code generation. */
>> + dt_cg_tramp_prologue_sub(pcb, DT_ACTIVITY_ACTIVE);
>> }
>>
>> /*
>> @@ -421,8 +438,8 @@ dt_cg_tramp_copy_args_from_regs(dt_pcb_t *pcb, int called)
>> * So put the PC in both arg0 and arg1, test the PC, and then zero out
>> * either arg0 or arg1, as apropriate.
>> *
>> - * The caller must ensure that %r7 and %r8 contain the values set by
>> - * the dt_cg_tramp_prologue*() functions.
>> + * The caller must ensure that %r7 and %r8 contain the values set by the
>> + * dt_cg_tramp_prologue*() functions.
>> */
>> void
>> dt_cg_tramp_copy_pc_from_regs(dt_pcb_t *pcb)
>> diff --git a/libdtrace/dt_cg.h b/libdtrace/dt_cg.h
>> index 0ced6dd2..1a40b2de 100644
>> --- a/libdtrace/dt_cg.h
>> +++ b/libdtrace/dt_cg.h
>> @@ -20,7 +20,7 @@ extern "C" {
>> extern void dt_cg(dt_pcb_t *, dt_node_t *);
>> extern void dt_cg_xsetx(dt_irlist_t *, dt_ident_t *, uint_t, int, uint64_t);
>> extern dt_irnode_t *dt_cg_node_alloc(uint_t, struct bpf_insn);
>> -extern void dt_cg_tramp_prologue_act(dt_pcb_t *pcb, dt_activity_t act);
>> +extern void dt_cg_tramp_prologue_sub(dt_pcb_t *pcb, dt_activity_t act);
>> extern void dt_cg_tramp_prologue(dt_pcb_t *pcb);
>> extern void dt_cg_tramp_clear_regs(dt_pcb_t *pcb);
>> extern void dt_cg_tramp_copy_regs(dt_pcb_t *pcb);
>> diff --git a/libdtrace/dt_prov_cpc.c b/libdtrace/dt_prov_cpc.c
>> index 9e0f5542..706d729a 100644
>> --- a/libdtrace/dt_prov_cpc.c
>> +++ b/libdtrace/dt_prov_cpc.c
>> @@ -395,7 +395,7 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
>> int i;
>> dt_irlist_t *dlp = &pcb->pcb_ir;
>>
>> - dt_cg_tramp_prologue(pcb);
>> + dt_cg_tramp_prologue_sub(pcb, DT_ACTIVITY_ACTIVE);
>>
>> /*
>> * After the dt_cg_tramp_prologue() call, we have:
>> diff --git a/libdtrace/dt_prov_dtrace.c b/libdtrace/dt_prov_dtrace.c
>> index a76534f8..277edae2 100644
>> --- a/libdtrace/dt_prov_dtrace.c
>> +++ b/libdtrace/dt_prov_dtrace.c
>> @@ -119,10 +119,10 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
>> key = DT_STATE_ENDEDON;
>> }
>>
>> - dt_cg_tramp_prologue_act(pcb, act);
>> + dt_cg_tramp_prologue_sub(pcb, act);
>>
>> /*
>> - * After the dt_cg_tramp_prologue_act() call, we have:
>> + * After the dt_cg_tramp_prologue_sub() call, we have:
>> * // (%r7 = dctx->mst)
>> * // (%r8 = dctx->ctx)
>> */
>> diff --git a/libdtrace/dt_prov_profile.c b/libdtrace/dt_prov_profile.c
>> index f3f3bf23..08216b08 100644
>> --- a/libdtrace/dt_prov_profile.c
>> +++ b/libdtrace/dt_prov_profile.c
>> @@ -220,7 +220,7 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
>> int i;
>> dt_irlist_t *dlp = &pcb->pcb_ir;
>>
>> - dt_cg_tramp_prologue(pcb);
>> + dt_cg_tramp_prologue_sub(pcb, DT_ACTIVITY_ACTIVE);
>>
>> /*
>> * After the dt_cg_tramp_prologue() call, we have:
More information about the DTrace-devel
mailing list