[DTrace-devel] [PATCH v2 02/08] The exit() action must set the activity state to DRAINING

[Eugene Loh]

On 09/17/2020 01:28 PM, Kris Van Hees wrote:

> On Thu, Sep 17, 2020 at 12:48:27PM -0700, Eugene Loh wrote:
>> On 09/14/2020 12:50 PM, Kris Van Hees wrote:
>>
>>> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
>>> @@ -684,19 +686,51 @@ dt_cg_act_discard(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
>>>    static void
>>>    dt_cg_act_exit(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
>>>    {
>>> -	struct bpf_insn	instr;
>>>    	dt_irlist_t	*dlp = &pcb->pcb_ir;
>>> -	uint_t		off;
>>> -
>>> -	dt_cg_node(dnp->dn_args, &pcb->pcb_ir, pcb->pcb_regs);
>>> +	dt_ident_t	*state = dt_dlib_get_map(pcb->pcb_hdl, "state");
>>> +	struct bpf_insn	instr;
>>>    
>>> -	off = dt_rec_add(pcb->pcb_hdl, dt_cg_fill_gap, DTRACEACT_EXIT,
>>> -			 sizeof(uint32_t), sizeof(uint32_t), NULL,
>>> -			 DT_ACT_EXIT);
>>> +	/* Record the exit code. */
>>> +	dt_cg_store_val(pcb, dnp->dn_args, DTRACEACT_EXIT, NULL, DT_ACT_EXIT);
>>>    
>>> -	instr = BPF_STORE(BPF_W, BPF_REG_9, off, dnp->dn_args->dn_reg);
>>> +	/*
>>> +	 * Force the activity state to DRAINING.
>>> +	 *
>>> +	 *	key = DT_STATE_ACTIVITY;// stw [%fp + DT_STK_SPILL(0)],
>>> +	 *				//		DT_STATE_ACTIVITY
>>> +	 *	val = DT_ACTIVITY_DRAINING;
>>> +	 *				// stw [%fp + DT_STK_SPILL(1)],
>>> +	 *				//		DT_ACTIVITY_DRAINING
>>> +	 *	rc = 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
>>>
>> Should we be allocating (and freeing) registers 0-5 before (and after)
>> the helper call?
> Yes indeed, we should.  I shamelessly copied this code from the trampoline
> where I don't (yet) because that is code that doesn't call dynamic code
> generation functions - i.e. we have total control over the regs usage.

Thanks for the explanation.  And yes I agree that trampoline code is 
managing regs explicitly.

> I'll add the alloc/free of call regs.

Okay, great.  Out of curiosity, do you anticipate that for this patch or 
a future one?