[v3,2/6] trace: support dereferencing arguments
Checks
Commit Message
Rather than use an intermediate variable, allow use of * to dereference
a trace point argument.
Update dmadev traces accordingly (and adjust the emitter type).
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changes since v2:
- split this change out of patch 2, as it required updating CTF metadata
fixup,
---
lib/dmadev/rte_dmadev_trace_fp.h | 12 +++------
lib/eal/common/eal_common_trace_ctf.c | 35 ++++++++++++++++++---------
2 files changed, 27 insertions(+), 20 deletions(-)
Comments
> diff --git a/lib/eal/common/eal_common_trace_ctf.c
> b/lib/eal/common/eal_common_trace_ctf.c
> index 04c4f71462..3e4228ee7f 100644
> --- a/lib/eal/common/eal_common_trace_ctf.c
> +++ b/lib/eal/common/eal_common_trace_ctf.c
> @@ -373,6 +373,11 @@ rte_trace_metadata_dump(FILE *f)
>
> char *trace_metadata_fixup_field(const char *field) {
> + static const char * const tokens[] = {
> + ".",
> + "->",
> + "*",
> + };
As this patch is intended to support dereferencing operation but as a side effect it applies to multiplication operator too.
Hence for A * B, field is generated like A___B (one extra underscore). So IMO, multiplication string shouldn't be used.
> const char *ctf_reserved_words[] = {
> "align",
> "event",
> 2.48.1
On Tue, Feb 11, 2025 at 9:44 AM Sunil Kumar Kori <skori@marvell.com> wrote:
>
> > diff --git a/lib/eal/common/eal_common_trace_ctf.c
> > b/lib/eal/common/eal_common_trace_ctf.c
> > index 04c4f71462..3e4228ee7f 100644
> > --- a/lib/eal/common/eal_common_trace_ctf.c
> > +++ b/lib/eal/common/eal_common_trace_ctf.c
> > @@ -373,6 +373,11 @@ rte_trace_metadata_dump(FILE *f)
> >
> > char *trace_metadata_fixup_field(const char *field) {
> > + static const char * const tokens[] = {
> > + ".",
> > + "->",
> > + "*",
> > + };
> As this patch is intended to support dereferencing operation but as a side effect it applies to multiplication operator too.
Support for multiplication helps with the next patch too, when passing
an array size as a blob length.
> Hence for A * B, field is generated like A___B (one extra underscore). So IMO, multiplication string shouldn't be used.
I am not following the consequence.. why should it be prevented?
> On Tue, Feb 11, 2025 at 9:44 AM Sunil Kumar Kori <skori@marvell.com>
> wrote:
> >
> > > diff --git a/lib/eal/common/eal_common_trace_ctf.c
> > > b/lib/eal/common/eal_common_trace_ctf.c
> > > index 04c4f71462..3e4228ee7f 100644
> > > --- a/lib/eal/common/eal_common_trace_ctf.c
> > > +++ b/lib/eal/common/eal_common_trace_ctf.c
> > > @@ -373,6 +373,11 @@ rte_trace_metadata_dump(FILE *f)
> > >
> > > char *trace_metadata_fixup_field(const char *field) {
> > > + static const char * const tokens[] = {
> > > + ".",
> > > + "->",
> > > + "*",
> > > + };
> > As this patch is intended to support dereferencing operation but as a side
> effect it applies to multiplication operator too.
>
> Support for multiplication helps with the next patch too, when passing an
> array size as a blob length.
>
> > Hence for A * B, field is generated like A___B (one extra underscore). So
> IMO, multiplication string shouldn't be used.
>
> I am not following the consequence.. why should it be prevented?
As such there is no consequence on functionality. It's just about readability.
For example, num_entries * size will result in num_entries___size which actually
not representing that this is used to size. Instead tot_size = num_entries * size and
then using tot_size clearly specifies that it is the total size of array.
>
>
> --
> David Marchand
@@ -37,10 +37,9 @@ RTE_TRACE_POINT_FP(
enum rte_dma_vchan_status __status = 0;
status = &__status;
#endif /* _RTE_TRACE_POINT_REGISTER_H_ */
- int vchan_status = *status;
rte_trace_point_emit_i16(dev_id);
rte_trace_point_emit_u16(vchan);
- rte_trace_point_emit_int(vchan_status);
+ rte_trace_point_emit_int(*status);
rte_trace_point_emit_int(ret);
)
@@ -107,13 +106,11 @@ RTE_TRACE_POINT_FP(
last_idx = &__last_idx;
has_error = &__has_error;
#endif /* _RTE_TRACE_POINT_REGISTER_H_ */
- int has_error_val = *has_error;
- int last_idx_val = *last_idx;
rte_trace_point_emit_i16(dev_id);
rte_trace_point_emit_u16(vchan);
rte_trace_point_emit_u16(nb_cpls);
- rte_trace_point_emit_int(last_idx_val);
- rte_trace_point_emit_int(has_error_val);
+ rte_trace_point_emit_u16(*last_idx);
+ rte_trace_point_emit_u8(*has_error);
rte_trace_point_emit_u16(ret);
)
@@ -126,11 +123,10 @@ RTE_TRACE_POINT_FP(
uint16_t __last_idx = 0;
last_idx = &__last_idx;
#endif /* _RTE_TRACE_POINT_REGISTER_H_ */
- int last_idx_val = *last_idx;
rte_trace_point_emit_i16(dev_id);
rte_trace_point_emit_u16(vchan);
rte_trace_point_emit_u16(nb_cpls);
- rte_trace_point_emit_int(last_idx_val);
+ rte_trace_point_emit_u16(*last_idx);
rte_trace_point_emit_ptr(status);
rte_trace_point_emit_u16(ret);
)
@@ -373,6 +373,11 @@ rte_trace_metadata_dump(FILE *f)
char *trace_metadata_fixup_field(const char *field)
{
+ static const char * const tokens[] = {
+ ".",
+ "->",
+ "*",
+ };
const char *ctf_reserved_words[] = {
"align",
"event",
@@ -390,23 +395,29 @@ char *trace_metadata_fixup_field(const char *field)
return out;
}
+ for (i = 0; i < RTE_DIM(tokens); i++) {
+ if (strstr(field, tokens[i]) == NULL)
+ continue;
+ goto fixup;
+ }
+
/* nothing to replace, return early */
- if (strstr(field, ".") == NULL && strstr(field, "->") == NULL)
- return NULL;
+ return NULL;
+fixup:
out = strdup(field);
if (out == NULL)
return NULL;
- p = out;
- while ((p = strstr(p, ".")) != NULL) {
- p[0] = '_';
- p++;
- }
- p = out;
- while ((p = strstr(p, "->")) != NULL) {
- p[0] = '_';
- p++;
- memmove(p, p + 1, strlen(p));
+ for (i = 0; i < RTE_DIM(tokens); i++) {
+ p = out;
+ while ((p = strstr(p, tokens[i])) != NULL) {
+ p[0] = '_';
+ p++;
+ if (strlen(tokens[i]) != 1) {
+ memmove(p, p + (strlen(tokens[i]) - 1),
+ strlen(p) - (strlen(tokens[i]) - 2));
+ }
+ }
}
return out;
}