[DTrace-devel] [PATCH 46/61] Change aid to act in dt_agg_one_agg()

Kris Van Hees kris.van.hees at oracle.com
Sun Aug 14 05:07:14 UTC 2022


On Fri, Jul 08, 2022 at 10:45:30AM -0400, eugene.loh--- via DTrace-devel wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
> 
> ---
>  libdtrace/dt_aggregate.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/libdtrace/dt_aggregate.c b/libdtrace/dt_aggregate.c
> index ccd1934b..191aef52 100644
> --- a/libdtrace/dt_aggregate.c
> +++ b/libdtrace/dt_aggregate.c
> @@ -417,7 +417,7 @@ typedef struct dt_snapstate {
>  } dt_snapstate_t;
>  
>  static void
> -dt_agg_one_agg(dt_ident_t *aid, int64_t *dst, int64_t *src, uint_t realsz)
> +dt_agg_one_agg(uint_t act, int64_t *dst, int64_t *src, uint_t realsz)

I do not agree that this is desirable.  While it is (in some sense) a code
optimization, it does not seem right because dt_agg_one_agg() is supposed to
be acting on an aggregation rather than an action.  So, passing aid makes more
sense from a semantaic perspective.

>  {
>  	uint_t i, cnt;
>  
> @@ -425,7 +425,7 @@ dt_agg_one_agg(dt_ident_t *aid, int64_t *dst, int64_t *src, uint_t realsz)
>  		return;
>  
>  	src++;  /* skip latch sequence number */
> -	switch (((dt_ident_t *)aid->di_iarg)->di_id) {
> +	switch (act) {
>  	case DT_AGG_MAX:
>  		if (*src > *dst)
>  			*dst = *src;
> @@ -475,19 +475,22 @@ dt_aggregate_snap_one(dt_idhash_t *dhp, dt_ident_t *aid, dt_snapstate_t *st)
>  
>  	/* See if we already have an entry for this aggregation. */
>  	for (h = agh->dtah_hash[ndx]; h != NULL; h = h->dtahe_next) {
> +		uint_t act;
> +
>  		if (h->dtahe_hval != hval || h->dtahe_size != agg->dtagd_size)
>  			continue;
>  
>  		/* Entry found - process the data. */
>  		agd = &h->dtahe_data;
> +		act = ((dt_ident_t *)aid->di_iarg)->di_id;
>  
>  		rec = &agg->dtagd_recs[nrecs - 1];
> -		dt_agg_one_agg(aid, (int64_t *)(agd->dtada_data + rec->dtrd_offset),
> +		dt_agg_one_agg(act, (int64_t *)(agd->dtada_data + rec->dtrd_offset),
>  			       src, datasz);
>  
>  		/* If we keep per-CPU data - process that as well. */
>  		if (agd->dtada_percpu != NULL)
> -			dt_agg_one_agg(aid, (int64_t *)agd->dtada_percpu[st->cpu],
> +			dt_agg_one_agg(act, (int64_t *)agd->dtada_percpu[st->cpu],
>  			    src, datasz);
>  
>  		return 0;
> -- 
> 2.18.4
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel



More information about the DTrace-devel mailing list