[dpdk-dev] [PATCH v1 03/32] eal/trace: implement trace register API

[Mattias Rönnblom]

On 2020-03-23 14:37, Jerin Jacob wrote:
>>> +     }
>>> +
>>> +     /* Initialize the trace point */
>>> +     if (rte_strscpy(tp->name, name, TRACE_POINT_NAME_SIZE) < 0) {
>>> +             trace_err("name is too long");
>>> +             rte_errno = E2BIG;
>>> +             goto free;
>>> +     }
>>> +
>>> +     /* Copy the field data for future use */
>>> +     if (rte_strscpy(tp->ctf_field, field, TRACE_CTF_FIELD_SIZE) < 0) {
>>> +             trace_err("CTF field size is too long");
>>> +             rte_errno = E2BIG;
>>> +             goto free;
>>> +     }
>>> +
>>> +     /* Clear field memory for the next event */
>>> +     memset(field, 0, TRACE_CTF_FIELD_SIZE);
>>> +
>>> +     /* Form the trace handle */
>>> +     *handle = sz;
>>> +     *handle |= trace.nb_trace_points << __RTE_TRACE_FIELD_ID_SHIFT;
>>> +     *handle |= (uint64_t)level << __RTE_TRACE_FIELD_LEVEL_SHIFT;
>> If *handle would be a struct, you could use a bitfield instead, and much
>> simplify this code.
> I thought that initially, Two reasons why I did not do that
> 1) The flags have been used in fastpath, I prefer to work with flags
> in fastpath so that
Is it really that obvious that flags are faster than bitfield 
operations? I think most modern architectures have machine instructions 
for bitfield manipulation.
> there is no performance impact using bitfields from the compiler _if any_.
> 2) In some of the places, I can simply operate on APIs like
> __atomic_and_fetch() with flags.

I think you may still use such atomic operations. Just convert the 
struct to a uint64_t, which will essentially be a no-operation, and fire 
away.


static uint64_t

__rte_trace_raw(struct trace *t)

{

     uint64_t raw;

     memcpy(&raw, t, sizeof(struct trace));

     return raw;

}