[dpdk-dev] [EXT] Re: [PATCH 1/1] eal: increase TRACE CTF SIZE to recommended size

[David Marchand]

On Tue, Oct 6, 2020 at 11:58 AM Jerin Jacob <jerinjacobk at gmail.com> wrote:
>
> On Tue, Oct 6, 2020 at 3:10 PM David Marchand <david.marchand at redhat.com> wrote:
> >
> > On Tue, Oct 6, 2020 at 11:22 AM Sunil Kumar Kori <skori at marvell.com> wrote:
> > > >On Mon, Oct 5, 2020 at 10:16 PM Timothy McDaniel
> > > ><timothy.mcdaniel at intel.com> wrote:
> > > >>
> > > >> Increase TRACE_CTF_FIELD_SIZE to 448, the recommended size.
> > > >
> > > >Repeating the same sentence in the title and the commitlog does not give
> > > >much info.
> > > >
> > > >Plus, what is this "recommendation"?
> > > When analyzed this issue, only one more byte was needed to fix this issue but in future similar issue can occur again.
> > > So increasing this value by 64 bytes which actually equals to a cache line. That’s why we have suggested this size.
> >
> > 384 is aligned to both 64 and 128 bytes cache lines.
> > 448 is only aligned to 64 bytes.
> >
> > Should we care about 128 bytes cache lines systems?
>
> it is on a slow path. 448 is OK.

Ah yes, this is for the ctf description.
Could it be changed to rely on dynamic allocations and we simply
remove this limit?


>
> >
> > >
> > > >
> > > >
> > > >> Fixes "CTF field is too long" error when running with trace enabled.
> > > >
> > > >Ok, you hit this limit, but it would help to get some context here.
> > > >Looking at this patch in the future, we won't know why it was necessary.
> >
> > How about following commitlog:
> >
> > """
> > trace: increase trace point buffer size
> >
> > The current buffer size is not big enough to accomodate traces for new
> > additions in the eventdev subsystem.
> > Increase this buffer size by XXX for reason YYY.
> > """
>
> Looks good to me.

Well in this case, there is no actual reason.
The increased value is deemed "enough for now", unless we change this
to dynamic allocations.



-- 
David Marchand