On Thu, Sep 22, 2022 at 1:18 PM Sunil Kumar Kori <sk...@marvell.com> wrote:
> > int
> > -rte_trace_point_disable(rte_trace_point_t *trace)
> > +rte_trace_point_disable(rte_trace_point_t *t)
> > {
> > - if (trace_point_is_invalid(trace))
> > + uint64_t prev;
> > +
> > + if (trace_point_is_invalid(t))
> > return -ERANGE;
> >
> > - __atomic_and_fetch(trace, ~__RTE_TRACE_FIELD_ENABLE_MASK,
> > - __ATOMIC_RELEASE);
> > + prev = __atomic_fetch_and(t, ~__RTE_TRACE_FIELD_ENABLE_MASK,
> > __ATOMIC_RELEASE);
> > + if ((prev & __RTE_TRACE_FIELD_ENABLE_MASK) != 0)
> > + __atomic_sub_fetch(&trace.status, 1, __ATOMIC_RELEASE);
> > return 0;
> > }
> >
> IMO, above change should go as a separate commit as it just replaces the
> variable name.
The count of enabled tracepoints (stored in the 'trace' global
variable) must be updated as part of the change.
So the renaming is necessary.
[snip]
> > @@ -375,6 +380,10 @@ trace_meta_save(struct trace *trace)
> > FILE *f;
> > int rc;
> >
> > + rc = trace_mkdir();
> > + if (rc < 0)
> > + return rc;
> > +
>
> Trace directory is being created here and in trace_mem_save() function along
> with the logic to handle whether it is already done or not.
> Instead can't it be called in rte_trace_save() directly. That will suffice
> the intention, I believe.
Oh indeed, I had the false impression that there were other path than
rte_trace_save(), leading to trace_mkdir().
Will fix.
--
David Marchand
