Re: [PATCH v2 1/2] eal: provide leading and trailing zero bit count abstraction

[Tyler Retzlaff <roretzla@linux.microsoft.com>]

On Fri, Jan 06, 2023 at 12:10:45AM +0100, Morten Brørup wrote:
> > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > Sent: Thursday, 5 January 2023 23.06
> > 
> > doesn't create a new kind of mess.
> > 
> > so i fudged around a bit to see if i could get a happy medium. i ended
> > up with this.
> > 
> > remove include of rte_debug.h from rte_bitops.h
> > 
> >   * had to remove the RTE_ASSERT from existing rte_bitops.h functions
> >   * this breaks a good piece of the cycle debug -> log -> common ->
> > bitops -> debug
> >   * deal breaker? i don't think it was right that we were getting all
> >     of log, common just for using bitops anyway.
> > 
> > move pow2 functions from rte_common.h -> rte_pow2ops.h
> >   * new header includes rte_bitops.h
> > 
> > move log2 functions from rte_common.h -> rte_log2ops.h
> >   * new header includes rte_bitops.h, rte_pow2ops.h
> > 
> > include rte_bitops.h, rte_pow2ops.h and rte_log2ops.h back into
> > rte_common.h
> > 
> >   * this is done to reduce the impact of compatibility break by
> >     continuing to expose the pow2/log2/bitops via rte_common.h
> > 
> > so we end up with 3 standalone headers, where the whole tree builds
> > without having to add a pile of includes for the new headers. we can
> > later deprecate the exposure of the inline functions when including
> > rte_common.h
> > 
> >   * one caveat is that there was some contamination coming in via the
> >     removed rte_debug.h where rte_bitops.h was used. so technically
> >     a break of api too.
> > 
> > objections?
> > 
> > if this is no good i'll just fold my new functions into rte_common.h
> > and
> > leave the mess for the next person, though i am trying not to do that.
> > 
> > thanks for the discussion.
> 
> Here's some long term thinking: EAL has grown into a trashcan where too much is thrown in. It should only be a thin shim to abstract the underlying hardware and O/S environment. A step in that direction could be splitting the current EAL into a true EAL and a Utils library. Not now, but perhaps some day in a rosy future.
> 
> Your proposal effectively makes rte_common.h even bigger by including rte_bitops.h (which was intended for accessing hardware). I am not sure it is a step in the right direction. On the other hand, introducing yet another header file for bit-mathematical functions is probably worse than adding them to rte_bitops.h.
> 
> I can't come up with something good myself, but I lean towards simply adding your functions to rte_common.h and live with the existing mess. If you think your proposal is better, I will not object. I'm only voicing my thoughts.

it's hard when we are starting with something monolithic and trying to
split it. you always end up with these iterative transitions toward what
you want because you have to keep some kind of compat.

> 
> @Thomas may have another perspective on the matter.

Thomas any last word after this back and forth?  This change isn't
necessarily blocking me but is a distraction i'd like to finish/offload.
> 
> Thanks for all the work you put into this.

np, it is somewhat out of necessity since i need this stuff to be
portable.  but having it move in the right direction at the same time is
a benefit for anyone who comes later.

ty