Re: [PATCH 22/38] Fix aggs comment in dt_cg_tramp_prologue_act()

Eugene Loh
2024-07-19T23:15:47Z

On 7/19/24 16:44, Kris Van Hees wrote:

> On Thu, Jun 27, 2024 at 01:38:48AM -0400, eugene.loh@oracle.com wrote:
>> From: Eugene Loh <eugene.loh@oracle.com>
>>
>> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
> Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>
>
> ... provided you mention in the commit msg what you are fixing. Because
> looking at the patch you are removing a big part of a comment block but
> without any indication why, and the patch does not provide context to be
> able to assess that.

Fair enough.  I suppose I never really knew why, other than that the
comment didn't match the code.  Okay, I did my homework and added
Since 2dd92c02 ("Add support for aggregation keys") removed a bunch
of code, the corresponding comments should also have been removed.
to the commit msg, along with your R_b.

>> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
>> @@ -372,17 +372,6 @@ dt_cg_tramp_prologue_act(dt_pcb_t *pcb, dt_activity_t act)
>> * if (rc == 0) // jeq %r0, 0, lbl_exit
>> * goto exit;
>> *
>> - * key = 0; // stdw [%r9 + DCTX_AGG], 0
>> - * rc = bpf_map_lookup_elem(rc, &key);
>> - * // mov %r1, %r0
>> - * // mov %r2, %r9
>> - * // add %r2, DCTX_AGG
>> - * // call bpf_map_lookup_elem
>> - * // (%r1 ... %r5 clobbered)
>> - * // (%r0 = aggs[cpuid] BPF map value)
>> - * if (rc == 0) // jeq %r0, 0, lbl_exit
>> - * goto exit;
>> - *
>> * dctx.aggs = rc; // stdw [%r9 + DCTX_AGG], %r0
>> */
>> if (dtp->dt_maxaggdsize > 0) {