[DTrace-devel] [PATCH v3 03/08] Record CPU that BEGIN and END probes run on in the 'state' BPF map

[Eugene Loh]

On 09/17/2020 03:06 PM, Kris Van Hees wrote:

> On Thu, Sep 17, 2020 at 02:50:46PM -0700, Eugene Loh wrote:
>> On 09/16/2020 04:11 PM, Kris Van Hees wrote:
>>> diff --git a/libdtrace/dt_prov_dtrace.c b/libdtrace/dt_prov_dtrace.c
>>> @@ -102,6 +113,47 @@ static void trampoline(dt_pcb_t *pcb)
>>>    
>>>    	dt_cg_tramp_prologue_act(pcb, lbl_exit, act);
>>>    
>>> +	if (key) {
>>> +		/*
>>> +		 *     key = DT_STATE_BEGANON;
>>> +		 *			// stw [%fp + DT_STK_SPILL(0)],
>>> +		 *			//			DT_STATE_BEGANON
>> Incidentally, there is something else I do not understand.  You use
>> spill slots to stage arguments for function calls.  (E.g., something
>> similar in staging the exit() action activity state.)  But don't we need
>> to worry about if those spill slots are in use for some registers?
> There is no code that can be using spill slots at this point because we're
> generating the trampoline code.  At this point it is guaranteed that no
> register spilling can have occured.

Fair enough, but I'd still like to understand the bigger picture.

There are a number of times we want to look up or update map elements.  
E.g.,

*) dt_cg_tramp_prologue_act() has to look up the activity state

*) dt_cg_tramp_prologue_act() has to look up mem

*) dt_cg_tramp_epilogue_advance() has to look up the activity state (for 
the sneaky purpose of incrementing in-place)

*) dt_cg_act_exit() has to update the activity state

*) (dtrace provider) trampoline() has to update the activity state

In these cases, the keys (and values, for update) are known at code 
generation but have to be passed to a helper function by reference. So 
we have to write them somewhere and pass the address(es) of where we 
wrote them.  Apparently, the selection of that temporary memory is made 
differently at different times.  That is,

*) dt_cg_tramp_*() uses %fp+DCTX_FP(DCTX_MST)

*) dt_cg_act_exit() uses %fp+DT_STK_SPILL(*)

*) (dtrace provider) trampoline() uses %fp+DT_STK_SPILL(*)

Two things make me uncomfortable:

*)  We're expecting the cg programmer to get creative and find a place 
to write these temporary values each time.  The programmer has to think, 
"What space is not being used at this moment?"

*)  I'm not sure whether it's okay for exit() to be using those spill 
slots.  Maybe it's fine, but in any case the reasoning for "trampoline" 
does not apply for exit().

Do those concerns make sense?

>>> +		 *     val = bpf_get_smp_processor_id();
>>> +		 *			// call bpf_get_smp_processor_id
>>> +		 *			// stw [%fp + DT_STK_SPILL(1)], %r0
>>> +		 *     bpf_map_update_elem(state, &key, &val, BPF_ANY);
>>> +		 *			// lddw %r1, &state
>>> +		 *			// mov %r2, %fp
>>> +		 *			// add %r2, DT_STK_SPILL(0)
>>> +		 *			// mov %r3, %fp
>>> +		 *			// add %r3, DT_STK_SPILL(1)
>>> +		 *			// mov %r4, BPF_ANY
>>> +		 *			// call bpf_map_update_elem
>>> +		 */
>>> +		dt_ident_t	*state = dt_dlib_get_map(pcb->pcb_hdl, "state");
>>> +
>>> +		instr = BPF_STORE_IMM(BPF_W, BPF_REG_FP, DT_STK_SPILL(0), key);
>>> +		dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
>>> +		instr = BPF_CALL_HELPER(BPF_FUNC_get_smp_processor_id);
>>> +		dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
>>> +		instr = BPF_STORE(BPF_W, BPF_REG_FP, DT_STK_SPILL(1),
>>> +				  BPF_REG_0);
>>> +		dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
>>> +		dt_cg_xsetx(dlp, state, DT_LBL_NONE, BPF_REG_1, state->di_id);
>>> +		instr = BPF_MOV_REG(BPF_REG_2, BPF_REG_FP);
>>> +		dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
>>> +		instr = BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, DT_STK_SPILL(0));
>>> +		dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
>>> +		instr = BPF_MOV_REG(BPF_REG_3, BPF_REG_FP);
>>> +		dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
>>> +		instr = BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, DT_STK_SPILL(1));
>>> +		dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
>>> +		instr = BPF_MOV_IMM(BPF_REG_4, BPF_ANY);
>>> +		dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
>>> +		instr = BPF_CALL_HELPER(BPF_FUNC_map_update_elem);
>>> +		dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
>>> +	}