* [dpdk-dev] [PATCH 1/2] bitmap: remove useless code @ 2018-11-21 12:05 Anatoly Burakov 2018-11-21 12:05 ` [dpdk-dev] [PATCH 2/2] bitmap: deprecate and rename rte_bsf64 Anatoly Burakov 0 siblings, 1 reply; 6+ messages in thread From: Anatoly Burakov @ 2018-11-21 12:05 UTC (permalink / raw) To: dev Cc: Cristian Dumitrescu, jasvinder.singh, ferruh.yigit, bruce.richardson, thomas RTE_BITMAP_OPTIMIZATIONS was never set to 0 and makes no sense anyway, so remove all code related to it. Also, drop the "likely" for bsf64 code, because it's a generic function and we cannot make any assumptions about likely values of incoming arguments. Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com> --- lib/librte_eal/common/include/rte_bitmap.h | 33 +--------------------- 1 file changed, 1 insertion(+), 32 deletions(-) diff --git a/lib/librte_eal/common/include/rte_bitmap.h b/lib/librte_eal/common/include/rte_bitmap.h index 7a36ce73c..d2ed6204c 100644 --- a/lib/librte_eal/common/include/rte_bitmap.h +++ b/lib/librte_eal/common/include/rte_bitmap.h @@ -43,10 +43,6 @@ extern "C" { #include <rte_branch_prediction.h> #include <rte_prefetch.h> -#ifndef RTE_BITMAP_OPTIMIZATIONS -#define RTE_BITMAP_OPTIMIZATIONS 1 -#endif - /* Slab */ #define RTE_BITMAP_SLAB_BIT_SIZE 64 #define RTE_BITMAP_SLAB_BIT_SIZE_LOG2 6 @@ -97,43 +93,16 @@ __rte_bitmap_index2_set(struct rte_bitmap *bmp) bmp->index2 = (((bmp->index1 << RTE_BITMAP_SLAB_BIT_SIZE_LOG2) + bmp->offset1) << RTE_BITMAP_CL_SLAB_SIZE_LOG2); } -#if RTE_BITMAP_OPTIMIZATIONS - static inline int rte_bsf64(uint64_t slab, uint32_t *pos) { - if (likely(slab == 0)) { + if (slab == 0) return 0; - } *pos = __builtin_ctzll(slab); return 1; } -#else - -static inline int -rte_bsf64(uint64_t slab, uint32_t *pos) -{ - uint64_t mask; - uint32_t i; - - if (likely(slab == 0)) { - return 0; - } - - for (i = 0, mask = 1; i < RTE_BITMAP_SLAB_BIT_SIZE; i ++, mask <<= 1) { - if (unlikely(slab & mask)) { - *pos = i; - return 1; - } - } - - return 0; -} - -#endif - static inline uint32_t __rte_bitmap_get_memory_footprint(uint32_t n_bits, uint32_t *array1_byte_offset, uint32_t *array1_slabs, -- 2.17.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [dpdk-dev] [PATCH 2/2] bitmap: deprecate and rename rte_bsf64 2018-11-21 12:05 [dpdk-dev] [PATCH 1/2] bitmap: remove useless code Anatoly Burakov @ 2018-11-21 12:05 ` Anatoly Burakov 2018-11-22 17:04 ` Thomas Monjalon 2018-11-22 18:56 ` Ananyev, Konstantin 0 siblings, 2 replies; 6+ messages in thread From: Anatoly Burakov @ 2018-11-21 12:05 UTC (permalink / raw) To: dev Cc: Neil Horman, John McNamara, Marko Kovacevic, Cristian Dumitrescu, jasvinder.singh, ferruh.yigit, bruce.richardson, thomas Rename rte_bsf64 to rte_bsf64_safe (this is a "safe" version in that it prevents undefined behavior by checking if incoming parameter is zero) and move it to common header. Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com> Acked-by: Jasvinder Singh <jasvinder.singh@intel.com> --- doc/guides/rel_notes/deprecation.rst | 5 +++++ lib/librte_eal/common/include/rte_bitmap.h | 14 ++++--------- lib/librte_eal/common/include/rte_common.h | 23 ++++++++++++++++++++++ 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst index 34b28234c..553e99171 100644 --- a/doc/guides/rel_notes/deprecation.rst +++ b/doc/guides/rel_notes/deprecation.rst @@ -22,6 +22,11 @@ Deprecation Notices + ``rte_eal_devargs_type_count`` +* eal: function ``rte_bsf64`` in ``rte_bitmap.h`` has been renamed to + ``rte_bsf64_safe`` and moved to ``rte_common.h``. A new ``rte_bsf64`` function + will be added in the next release in ``rte_common.h`` that follows convention + set by existing ``rte_bsf32`` function. + * pci: Several exposed functions are misnamed. The following functions are deprecated starting from v17.11 and are replaced: diff --git a/lib/librte_eal/common/include/rte_bitmap.h b/lib/librte_eal/common/include/rte_bitmap.h index d2ed6204c..77727c828 100644 --- a/lib/librte_eal/common/include/rte_bitmap.h +++ b/lib/librte_eal/common/include/rte_bitmap.h @@ -93,14 +93,10 @@ __rte_bitmap_index2_set(struct rte_bitmap *bmp) bmp->index2 = (((bmp->index1 << RTE_BITMAP_SLAB_BIT_SIZE_LOG2) + bmp->offset1) << RTE_BITMAP_CL_SLAB_SIZE_LOG2); } -static inline int +static inline int __rte_deprecated rte_bsf64(uint64_t slab, uint32_t *pos) { - if (slab == 0) - return 0; - - *pos = __builtin_ctzll(slab); - return 1; + return rte_bsf64_safe(slab, pos); } static inline uint32_t @@ -408,9 +404,8 @@ __rte_bitmap_scan_search(struct rte_bitmap *bmp) value1 = bmp->array1[bmp->index1]; value1 &= __rte_bitmap_mask1_get(bmp); - if (rte_bsf64(value1, &bmp->offset1)) { + if (rte_bsf64_safe(value1, &bmp->offset1)) return 1; - } __rte_bitmap_index1_inc(bmp); bmp->offset1 = 0; @@ -419,9 +414,8 @@ __rte_bitmap_scan_search(struct rte_bitmap *bmp) for (i = 0; i < bmp->array1_size; i ++, __rte_bitmap_index1_inc(bmp)) { value1 = bmp->array1[bmp->index1]; - if (rte_bsf64(value1, &bmp->offset1)) { + if (rte_bsf64_safe(value1, &bmp->offset1)) return 1; - } } return 0; diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h index 87f0f6302..d115b175c 100644 --- a/lib/librte_eal/common/include/rte_common.h +++ b/lib/librte_eal/common/include/rte_common.h @@ -491,6 +491,29 @@ rte_fls_u32(uint32_t x) return (x == 0) ? 0 : 32 - __builtin_clz(x); } +/** + * Searches the input parameter for the least significant set bit + * (starting from zero). Safe version (checks for input parameter being zero). + * + * @warning ``pos`` must be a valid pointer. It is not checked! + * + * @param v + * The input parameter. + * @param pos + * If ``v`` was not 0, this value will contain position of least significant + * bit within the input parameter. + * @return + * Returns 0 if ``v`` was 0, otherwise returns 1. + */ +static inline int +rte_bsf64_safe(uint64_t v, uint32_t *pos) +{ + if (v == 0) + return 0; + + *pos = __builtin_ctzll(v); + return 1; +} #ifndef offsetof /** Return the offset of a field in a structure. */ -- 2.17.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH 2/2] bitmap: deprecate and rename rte_bsf64 2018-11-21 12:05 ` [dpdk-dev] [PATCH 2/2] bitmap: deprecate and rename rte_bsf64 Anatoly Burakov @ 2018-11-22 17:04 ` Thomas Monjalon 2018-11-22 22:52 ` Thomas Monjalon 2018-11-22 18:56 ` Ananyev, Konstantin 1 sibling, 1 reply; 6+ messages in thread From: Thomas Monjalon @ 2018-11-22 17:04 UTC (permalink / raw) To: Anatoly Burakov Cc: dev, Neil Horman, John McNamara, Marko Kovacevic, Cristian Dumitrescu, jasvinder.singh, ferruh.yigit, bruce.richardson 21/11/2018 13:05, Anatoly Burakov: > Rename rte_bsf64 to rte_bsf64_safe (this is a "safe" version in > that it prevents undefined behavior by checking if incoming > parameter is zero) and move it to common header. > > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com> > Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com> > Acked-by: Jasvinder Singh <jasvinder.singh@intel.com> Acked-by: Thomas Monjalon <thomas@monjalon.net> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH 2/2] bitmap: deprecate and rename rte_bsf64 2018-11-22 17:04 ` Thomas Monjalon @ 2018-11-22 22:52 ` Thomas Monjalon 0 siblings, 0 replies; 6+ messages in thread From: Thomas Monjalon @ 2018-11-22 22:52 UTC (permalink / raw) To: Anatoly Burakov Cc: dev, Neil Horman, John McNamara, Marko Kovacevic, Cristian Dumitrescu, jasvinder.singh, ferruh.yigit, bruce.richardson 22/11/2018 18:04, Thomas Monjalon: > 21/11/2018 13:05, Anatoly Burakov: > > Rename rte_bsf64 to rte_bsf64_safe (this is a "safe" version in > > that it prevents undefined behavior by checking if incoming > > parameter is zero) and move it to common header. > > > > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com> > > Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com> > > Acked-by: Jasvinder Singh <jasvinder.singh@intel.com> > > Acked-by: Thomas Monjalon <thomas@monjalon.net> Series applied, thanks ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH 2/2] bitmap: deprecate and rename rte_bsf64 2018-11-21 12:05 ` [dpdk-dev] [PATCH 2/2] bitmap: deprecate and rename rte_bsf64 Anatoly Burakov 2018-11-22 17:04 ` Thomas Monjalon @ 2018-11-22 18:56 ` Ananyev, Konstantin 2018-11-22 22:44 ` Thomas Monjalon 1 sibling, 1 reply; 6+ messages in thread From: Ananyev, Konstantin @ 2018-11-22 18:56 UTC (permalink / raw) To: Burakov, Anatoly, dev Cc: Neil Horman, Mcnamara, John, Kovacevic, Marko, Dumitrescu, Cristian, Singh, Jasvinder, Yigit, Ferruh, Richardson, Bruce, thomas > > Rename rte_bsf64 to rte_bsf64_safe (this is a "safe" version in > that it prevents undefined behavior by checking if incoming > parameter is zero) and move it to common header. Probably a stupid one: why to rename? Why just not fix rte_bsf64 to make it work with zero value, and keep the same function name? Konstantin > > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com> > Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com> > Acked-by: Jasvinder Singh <jasvinder.singh@intel.com> > --- > doc/guides/rel_notes/deprecation.rst | 5 +++++ > lib/librte_eal/common/include/rte_bitmap.h | 14 ++++--------- > lib/librte_eal/common/include/rte_common.h | 23 ++++++++++++++++++++++ > 3 files changed, 32 insertions(+), 10 deletions(-) > > diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst > index 34b28234c..553e99171 100644 > --- a/doc/guides/rel_notes/deprecation.rst > +++ b/doc/guides/rel_notes/deprecation.rst > @@ -22,6 +22,11 @@ Deprecation Notices > > + ``rte_eal_devargs_type_count`` > > +* eal: function ``rte_bsf64`` in ``rte_bitmap.h`` has been renamed to > + ``rte_bsf64_safe`` and moved to ``rte_common.h``. A new ``rte_bsf64`` function > + will be added in the next release in ``rte_common.h`` that follows convention > + set by existing ``rte_bsf32`` function. > + > * pci: Several exposed functions are misnamed. > The following functions are deprecated starting from v17.11 and are replaced: > > diff --git a/lib/librte_eal/common/include/rte_bitmap.h b/lib/librte_eal/common/include/rte_bitmap.h > index d2ed6204c..77727c828 100644 > --- a/lib/librte_eal/common/include/rte_bitmap.h > +++ b/lib/librte_eal/common/include/rte_bitmap.h > @@ -93,14 +93,10 @@ __rte_bitmap_index2_set(struct rte_bitmap *bmp) > bmp->index2 = (((bmp->index1 << RTE_BITMAP_SLAB_BIT_SIZE_LOG2) + bmp->offset1) << RTE_BITMAP_CL_SLAB_SIZE_LOG2); > } > > -static inline int > +static inline int __rte_deprecated > rte_bsf64(uint64_t slab, uint32_t *pos) > { > - if (slab == 0) > - return 0; > - > - *pos = __builtin_ctzll(slab); > - return 1; > + return rte_bsf64_safe(slab, pos); > } > > static inline uint32_t > @@ -408,9 +404,8 @@ __rte_bitmap_scan_search(struct rte_bitmap *bmp) > value1 = bmp->array1[bmp->index1]; > value1 &= __rte_bitmap_mask1_get(bmp); > > - if (rte_bsf64(value1, &bmp->offset1)) { > + if (rte_bsf64_safe(value1, &bmp->offset1)) > return 1; > - } > > __rte_bitmap_index1_inc(bmp); > bmp->offset1 = 0; > @@ -419,9 +414,8 @@ __rte_bitmap_scan_search(struct rte_bitmap *bmp) > for (i = 0; i < bmp->array1_size; i ++, __rte_bitmap_index1_inc(bmp)) { > value1 = bmp->array1[bmp->index1]; > > - if (rte_bsf64(value1, &bmp->offset1)) { > + if (rte_bsf64_safe(value1, &bmp->offset1)) > return 1; > - } > } > > return 0; > diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h > index 87f0f6302..d115b175c 100644 > --- a/lib/librte_eal/common/include/rte_common.h > +++ b/lib/librte_eal/common/include/rte_common.h > @@ -491,6 +491,29 @@ rte_fls_u32(uint32_t x) > return (x == 0) ? 0 : 32 - __builtin_clz(x); > } > > +/** > + * Searches the input parameter for the least significant set bit > + * (starting from zero). Safe version (checks for input parameter being zero). > + * > + * @warning ``pos`` must be a valid pointer. It is not checked! > + * > + * @param v > + * The input parameter. > + * @param pos > + * If ``v`` was not 0, this value will contain position of least significant > + * bit within the input parameter. > + * @return > + * Returns 0 if ``v`` was 0, otherwise returns 1. > + */ > +static inline int > +rte_bsf64_safe(uint64_t v, uint32_t *pos) > +{ > + if (v == 0) > + return 0; > + > + *pos = __builtin_ctzll(v); > + return 1; > +} > > #ifndef offsetof > /** Return the offset of a field in a structure. */ > -- > 2.17.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH 2/2] bitmap: deprecate and rename rte_bsf64 2018-11-22 18:56 ` Ananyev, Konstantin @ 2018-11-22 22:44 ` Thomas Monjalon 0 siblings, 0 replies; 6+ messages in thread From: Thomas Monjalon @ 2018-11-22 22:44 UTC (permalink / raw) To: Ananyev, Konstantin Cc: dev, Burakov, Anatoly, Neil Horman, Mcnamara, John, Kovacevic, Marko, Dumitrescu, Cristian, Singh, Jasvinder, Yigit, Ferruh, Richardson, Bruce 22/11/2018 19:56, Ananyev, Konstantin: > > Rename rte_bsf64 to rte_bsf64_safe (this is a "safe" version in > > that it prevents undefined behavior by checking if incoming > > parameter is zero) and move it to common header. > > Probably a stupid one: why to rename? > Why just not fix rte_bsf64 to make it work with zero value, > and keep the same function name? Because there are different parameters and returned value than rte_bsf32. In the next release, we will have rte_bsf64 function, behaving as rte_bsf32. This is explained in the deprecation notice. [...] > > +static inline int > > +rte_bsf64_safe(uint64_t v, uint32_t *pos) ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-11-22 22:52 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-21 12:05 [dpdk-dev] [PATCH 1/2] bitmap: remove useless code Anatoly Burakov 2018-11-21 12:05 ` [dpdk-dev] [PATCH 2/2] bitmap: deprecate and rename rte_bsf64 Anatoly Burakov 2018-11-22 17:04 ` Thomas Monjalon 2018-11-22 22:52 ` Thomas Monjalon 2018-11-22 18:56 ` Ananyev, Konstantin 2018-11-22 22:44 ` Thomas Monjalon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).