[dpdk-dev] [PATCH] bitmap: deprecate rte_bsf64

[dpdk-dev] [PATCH] bitmap: deprecate rte_bsf64

[Anatoly Burakov]

The rte_bsf64 in rte_bitmap.h has a global namespace but does not
follow convention of existing rte_bsf32 function in rte_common.h.

Therefore, deprecate the current rte_bsf64 and introduce a new
rte_bitmap_bsf64 function that will do the same thing.

In later release cycles, rte_bsf64 will be removed from rte_bitmap.h
and a proper rte_bsf64 implementation will be added to rte_common.h.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 doc/guides/rel_notes/deprecation.rst       |  5 +++++
 lib/librte_eal/common/include/rte_bitmap.h | 16 ++++++++++------
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 34b28234c..63793979d 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_bitmap_bsf64``, and a new ``rte_bsf64`` function will be added in
+  release 19.05 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 7a36ce73c..fd4f0aa78 100644
--- a/lib/librte_eal/common/include/rte_bitmap.h
+++ b/lib/librte_eal/common/include/rte_bitmap.h
@@ -100,7 +100,7 @@ __rte_bitmap_index2_set(struct rte_bitmap *bmp)
 #if RTE_BITMAP_OPTIMIZATIONS
 
 static inline int
-rte_bsf64(uint64_t slab, uint32_t *pos)
+rte_bitmap_bsf64(uint64_t slab, uint32_t *pos)
 {
 	if (likely(slab == 0)) {
 		return 0;
@@ -113,7 +113,7 @@ rte_bsf64(uint64_t slab, uint32_t *pos)
 #else
 
 static inline int
-rte_bsf64(uint64_t slab, uint32_t *pos)
+rte_bitmap_bsf64(uint64_t slab, uint32_t *pos)
 {
 	uint64_t mask;
 	uint32_t i;
@@ -134,6 +134,12 @@ rte_bsf64(uint64_t slab, uint32_t *pos)
 
 #endif
 
+static inline int __rte_deprecated
+rte_bsf64(uint64_t slab, uint32_t *pos)
+{
+	return rte_bitmap_bsf64(slab, pos);
+}
+
 static inline uint32_t
 __rte_bitmap_get_memory_footprint(uint32_t n_bits,
 	uint32_t *array1_byte_offset, uint32_t *array1_slabs,
@@ -439,9 +445,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_bitmap_bsf64(value1, &bmp->offset1))
 		return 1;
-	}
 
 	__rte_bitmap_index1_inc(bmp);
 	bmp->offset1 = 0;
@@ -450,9 +455,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_bitmap_bsf64(value1, &bmp->offset1))
 			return 1;
-		}
 	}
 
 	return 0;
-- 
2.17.1

[dpdk-dev] [PATCH v2 1/5] bitmap: remove useless code

[Anatoly Burakov]

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>
---
 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

[dpdk-dev] [PATCH v2 2/5] bitmap: rename rte_bsf64 and move to common header

[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>
---
 doc/guides/rel_notes/deprecation.rst       |  5 +++++
 lib/librte_eal/common/include/rte_bitmap.h | 14 ++++----------
 lib/librte_eal/common/include/rte_common.h | 21 +++++++++++++++++++++
 3 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 34b28234c..5d447e8eb 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 release 19.05 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..7609840da 100644
--- a/lib/librte_eal/common/include/rte_common.h
+++ b/lib/librte_eal/common/include/rte_common.h
@@ -491,6 +491,27 @@ 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).
+ *
+ * @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

[dpdk-dev] [PATCH v2 3/5] common: add missing implementations

[Anatoly Burakov]

Implement missing functions for 32-bit safe bsf, as well as 64-bit
fls and log2.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/common/include/rte_common.h | 62 +++++++++++++++++++++-
 1 file changed, 61 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h
index 7609840da..baec734a1 100644
--- a/lib/librte_eal/common/include/rte_common.h
+++ b/lib/librte_eal/common/include/rte_common.h
@@ -456,6 +456,28 @@ rte_bsf32(uint32_t v)
 	return (uint32_t)__builtin_ctz(v);
 }
 
+/**
+ * Searches the input parameter for the least significant set bit
+ * (starting from zero). Safe version (checks for input parameter being zero).
+ *
+ * @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_bsf32_safe(uint64_t v, uint32_t *pos)
+{
+	if (v == 0)
+		return 0;
+
+	*pos = rte_bsf32(v);
+	return 1;
+}
+
 /**
  * Return the rounded-up log2 of a integer.
  *
@@ -473,7 +495,6 @@ rte_log2_u32(uint32_t v)
 	return rte_bsf32(v);
 }
 
-
 /**
  * Return the last (most-significant) bit set.
  *
@@ -513,6 +534,45 @@ rte_bsf64_safe(uint64_t v, uint32_t *pos)
 	return 1;
 }
 
+/**
+ * Return the last (most-significant) bit set.
+ *
+ * @note The last (most significant) bit is at position 32.
+ * @note rte_fls_u32(0) = 0, rte_fls_u32(1) = 1, rte_fls_u32(0x80000000) = 32
+ *
+ * @param x
+ *     The input parameter.
+ * @return
+ *     The last (most-significant) bit set, or 0 if the input is 0.
+ */
+static inline int
+rte_fls_u64(uint64_t x)
+{
+	return (x == 0) ? 0 : 64 - __builtin_clzll(x);
+}
+
+/**
+ * Return the rounded-up log2 of a integer.
+ *
+ * @param v
+ *     The input parameter.
+ * @return
+ *     The rounded-up log2 of the input, or 0 if the input is 0.
+ */
+static inline uint32_t
+rte_log2_u64(uint64_t v)
+{
+	uint32_t pos = 0;
+	if (v == 0)
+		return 0;
+	v = rte_align64pow2(v);
+	/* TODO: replace with rte_bsf64 when that lands */
+	/* we checked for v being 0 already, so pos is always valid */
+	rte_bsf64_safe(v, &pos);
+	return pos;
+}
+
+
 #ifndef offsetof
 /** Return the offset of a field in a structure. */
 #define offsetof(TYPE, MEMBER)  __builtin_offsetof (TYPE, MEMBER)
-- 
2.17.1

[dpdk-dev] [PATCH v2 4/5] memalloc: use library implementation of 64-bit log2

[Anatoly Burakov]

Remove duplicated code and use library version of 64-bit log2.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/linuxapp/eal/eal_memalloc.c | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_memalloc.c b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
index 48b9c7360..7f9f0d027 100644
--- a/lib/librte_eal/linuxapp/eal/eal_memalloc.c
+++ b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
@@ -208,28 +208,13 @@ get_file_size(int fd)
 	return st.st_size;
 }
 
-static inline uint32_t
-bsf64(uint64_t v)
-{
-	return (uint32_t)__builtin_ctzll(v);
-}
-
-static inline uint32_t
-log2_u64(uint64_t v)
-{
-	if (v == 0)
-		return 0;
-	v = rte_align64pow2(v);
-	return bsf64(v);
-}
-
 static int
 pagesz_flags(uint64_t page_sz)
 {
 	/* as per mmap() manpage, all page sizes are log2 of page size
 	 * shifted by MAP_HUGE_SHIFT
 	 */
-	int log2 = log2_u64(page_sz);
+	int log2 = rte_log2_u64(page_sz);
 	return log2 << RTE_MAP_HUGE_SHIFT;
 }
 
-- 
2.17.1

[dpdk-dev] [PATCH v2 5/5] testpmd: use library implementation of 64-bit log2

[Anatoly Burakov]

Remove duplicated code and use library version of 64-bit log2.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 app/test-pmd/testpmd.c | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 9c0edcaed..89c6ffd53 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -649,28 +649,13 @@ calc_mem_size(uint32_t nb_mbufs, uint32_t mbuf_sz, size_t pgsz, size_t *out)
 	return 0;
 }
 
-static inline uint32_t
-bsf64(uint64_t v)
-{
-	return (uint32_t)__builtin_ctzll(v);
-}
-
-static inline uint32_t
-log2_u64(uint64_t v)
-{
-	if (v == 0)
-		return 0;
-	v = rte_align64pow2(v);
-	return bsf64(v);
-}
-
 static int
 pagesz_flags(uint64_t page_sz)
 {
 	/* as per mmap() manpage, all page sizes are log2 of page size
 	 * shifted by MAP_HUGE_SHIFT
 	 */
-	int log2 = log2_u64(page_sz);
+	int log2 = rte_log2_u64(page_sz);
 
 	return (log2 << HUGE_SHIFT);
 }
-- 
2.17.1

Re: [dpdk-dev] [PATCH v2 1/5] bitmap: remove useless code

[Dumitrescu, Cristian]

> -----Original Message-----
> From: Burakov, Anatoly
> Sent: Wednesday, November 14, 2018 4:30 PM
> To: dev@dpdk.org
> Cc: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>;
> thomas@monjalon.net; Richardson, Bruce <bruce.richardson@intel.com>;
> Yigit, Ferruh <ferruh.yigit@intel.com>; Singh, Jasvinder
> <jasvinder.singh@intel.com>
> Subject: [PATCH v2 1/5] bitmap: remove useless code
> 
> 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>
> ---
>  lib/librte_eal/common/include/rte_bitmap.h | 33 +---------------------
>  1 file changed, 1 insertion(+), 32 deletions(-)
> 

Series-acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>

[dpdk-dev] [PATCH v3 1/5] bitmap: remove useless code

[Anatoly Burakov]

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

[dpdk-dev] [PATCH v3 2/5] bitmap: rename rte_bsf64 and move to common header

[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>
---

Notes:
    v3:
    - Added clarification that pos is not checked

 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..5d447e8eb 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 release 19.05 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

[dpdk-dev] [PATCH v3 3/5] common: add missing implementations

[Anatoly Burakov]

Implement missing functions for 32-bit safe bsf, as well as 64-bit
fls and log2.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
---

Notes:
    v3:
    - Added clarification that pos is not checked

 lib/librte_eal/common/include/rte_common.h | 64 +++++++++++++++++++++-
 1 file changed, 63 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h
index d115b175c..6883e5c3b 100644
--- a/lib/librte_eal/common/include/rte_common.h
+++ b/lib/librte_eal/common/include/rte_common.h
@@ -456,6 +456,30 @@ rte_bsf32(uint32_t v)
 	return (uint32_t)__builtin_ctz(v);
 }
 
+/**
+ * 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_bsf32_safe(uint64_t v, uint32_t *pos)
+{
+	if (v == 0)
+		return 0;
+
+	*pos = rte_bsf32(v);
+	return 1;
+}
+
 /**
  * Return the rounded-up log2 of a integer.
  *
@@ -473,7 +497,6 @@ rte_log2_u32(uint32_t v)
 	return rte_bsf32(v);
 }
 
-
 /**
  * Return the last (most-significant) bit set.
  *
@@ -515,6 +538,45 @@ rte_bsf64_safe(uint64_t v, uint32_t *pos)
 	return 1;
 }
 
+/**
+ * Return the last (most-significant) bit set.
+ *
+ * @note The last (most significant) bit is at position 32.
+ * @note rte_fls_u32(0) = 0, rte_fls_u32(1) = 1, rte_fls_u32(0x80000000) = 32
+ *
+ * @param x
+ *     The input parameter.
+ * @return
+ *     The last (most-significant) bit set, or 0 if the input is 0.
+ */
+static inline int
+rte_fls_u64(uint64_t x)
+{
+	return (x == 0) ? 0 : 64 - __builtin_clzll(x);
+}
+
+/**
+ * Return the rounded-up log2 of a integer.
+ *
+ * @param v
+ *     The input parameter.
+ * @return
+ *     The rounded-up log2 of the input, or 0 if the input is 0.
+ */
+static inline uint32_t
+rte_log2_u64(uint64_t v)
+{
+	uint32_t pos = 0;
+	if (v == 0)
+		return 0;
+	v = rte_align64pow2(v);
+	/* TODO: replace with rte_bsf64 when that lands */
+	/* we checked for v being 0 already, so pos is always valid */
+	rte_bsf64_safe(v, &pos);
+	return pos;
+}
+
+
 #ifndef offsetof
 /** Return the offset of a field in a structure. */
 #define offsetof(TYPE, MEMBER)  __builtin_offsetof (TYPE, MEMBER)
-- 
2.17.1

[dpdk-dev] [PATCH v3 4/5] memalloc: use library implementation of 64-bit log2

[Anatoly Burakov]

Remove duplicated code and use library version of 64-bit log2.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
---
 lib/librte_eal/linuxapp/eal/eal_memalloc.c | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_memalloc.c b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
index 48b9c7360..7f9f0d027 100644
--- a/lib/librte_eal/linuxapp/eal/eal_memalloc.c
+++ b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
@@ -208,28 +208,13 @@ get_file_size(int fd)
 	return st.st_size;
 }
 
-static inline uint32_t
-bsf64(uint64_t v)
-{
-	return (uint32_t)__builtin_ctzll(v);
-}
-
-static inline uint32_t
-log2_u64(uint64_t v)
-{
-	if (v == 0)
-		return 0;
-	v = rte_align64pow2(v);
-	return bsf64(v);
-}
-
 static int
 pagesz_flags(uint64_t page_sz)
 {
 	/* as per mmap() manpage, all page sizes are log2 of page size
 	 * shifted by MAP_HUGE_SHIFT
 	 */
-	int log2 = log2_u64(page_sz);
+	int log2 = rte_log2_u64(page_sz);
 	return log2 << RTE_MAP_HUGE_SHIFT;
 }
 
-- 
2.17.1

[dpdk-dev] [PATCH v3 5/5] testpmd: use library implementation of 64-bit log2

[Anatoly Burakov]

Remove duplicated code and use library version of 64-bit log2.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
---
 app/test-pmd/testpmd.c | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 9c0edcaed..89c6ffd53 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -649,28 +649,13 @@ calc_mem_size(uint32_t nb_mbufs, uint32_t mbuf_sz, size_t pgsz, size_t *out)
 	return 0;
 }
 
-static inline uint32_t
-bsf64(uint64_t v)
-{
-	return (uint32_t)__builtin_ctzll(v);
-}
-
-static inline uint32_t
-log2_u64(uint64_t v)
-{
-	if (v == 0)
-		return 0;
-	v = rte_align64pow2(v);
-	return bsf64(v);
-}
-
 static int
 pagesz_flags(uint64_t page_sz)
 {
 	/* as per mmap() manpage, all page sizes are log2 of page size
 	 * shifted by MAP_HUGE_SHIFT
 	 */
-	int log2 = log2_u64(page_sz);
+	int log2 = rte_log2_u64(page_sz);
 
 	return (log2 << HUGE_SHIFT);
 }
-- 
2.17.1

Re: [dpdk-dev] [PATCH v3 2/5] bitmap: rename rte_bsf64 and move to common header

[Singh, Jasvinder]

> -----Original Message-----
> From: Burakov, Anatoly
> Sent: Wednesday, November 14, 2018 4:47 PM
> To: dev@dpdk.org
> Cc: Neil Horman <nhorman@tuxdriver.com>; Mcnamara, John
> <john.mcnamara@intel.com>; Kovacevic, Marko
> <marko.kovacevic@intel.com>; Dumitrescu, Cristian
> <cristian.dumitrescu@intel.com>; thomas@monjalon.net; Richardson, Bruce
> <bruce.richardson@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Singh,
> Jasvinder <jasvinder.singh@intel.com>
> Subject: [PATCH v3 2/5] bitmap: rename rte_bsf64 and move to common
> header
> 
> 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>
> ---
> 
> Notes:
>     v3:
>     - Added clarification that pos is not checked
> 
>  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(-)
> 

Acked-by: Jasvinder Singh <jasvinder.singh@intel.com>

Re: [dpdk-dev] [PATCH v3 3/5] common: add missing implementations

[Jerin Jacob]

-----Original Message-----
> Date: Wed, 14 Nov 2018 16:47:08 +0000
> From: Anatoly Burakov <anatoly.burakov@intel.com>
> To: dev@dpdk.org
> CC: cristian.dumitrescu@intel.com, thomas@monjalon.net,
>  bruce.richardson@intel.com, ferruh.yigit@intel.com,
>  jasvinder.singh@intel.com
> Subject: [dpdk-dev] [PATCH v3 3/5] common: add missing implementations
> X-Mailer: git-send-email 1.7.0.7
> 
> External Email
> 
> Implement missing functions for 32-bit safe bsf, as well as 64-bit
> fls and log2.
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> ---

> +/**
> + * Return the last (most-significant) bit set.
> + *
> + * @note The last (most significant) bit is at position 32.

s/position 32/position 64

> + * @note rte_fls_u32(0) = 0, rte_fls_u32(1) = 1, rte_fls_u32(0x80000000) = 32

Fix here too

IMO, it is better to add unit test case for newly added common functions
in test/test/test_common.c

> + *
> + * @param x
> + *     The input parameter.
> + * @return
> + *     The last (most-significant) bit set, or 0 if the input is 0.
> + */
> +static inline int
> +rte_fls_u64(uint64_t x)
> +{
> +       return (x == 0) ? 0 : 64 - __builtin_clzll(x);
> +}
> +

Re: [dpdk-dev] [PATCH v3 3/5] common: add missing implementations

[Burakov, Anatoly]

On 15-Nov-18 8:40 AM, Jerin Jacob wrote:
> -----Original Message-----
>> Date: Wed, 14 Nov 2018 16:47:08 +0000
>> From: Anatoly Burakov <anatoly.burakov@intel.com>
>> To: dev@dpdk.org
>> CC: cristian.dumitrescu@intel.com, thomas@monjalon.net,
>>   bruce.richardson@intel.com, ferruh.yigit@intel.com,
>>   jasvinder.singh@intel.com
>> Subject: [dpdk-dev] [PATCH v3 3/5] common: add missing implementations
>> X-Mailer: git-send-email 1.7.0.7
>>
>> External Email
>>
>> Implement missing functions for 32-bit safe bsf, as well as 64-bit
>> fls and log2.
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
>> ---
> 
>> +/**
>> + * Return the last (most-significant) bit set.
>> + *
>> + * @note The last (most significant) bit is at position 32.
> 
> s/position 32/position 64
> 
>> + * @note rte_fls_u32(0) = 0, rte_fls_u32(1) = 1, rte_fls_u32(0x80000000) = 32
> 
> Fix here too
> 
> IMO, it is better to add unit test case for newly added common functions
> in test/test/test_common.c

Good catches and good ideas, thanks!

> 
>> + *
>> + * @param x
>> + *     The input parameter.
>> + * @return
>> + *     The last (most-significant) bit set, or 0 if the input is 0.
>> + */
>> +static inline int
>> +rte_fls_u64(uint64_t x)
>> +{
>> +       return (x == 0) ? 0 : 64 - __builtin_clzll(x);
>> +}
>> +
> 


-- 
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH v2 1/5] bitmap: remove useless code

[Burakov, Anatoly]

On 14-Nov-18 4:30 PM, Anatoly Burakov wrote:
> 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>
> ---

OK, since this requires more rework and can't be merged into 18.11 
anyway, i'll mark it as deferred and postpone it till 19.02.

-- 
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH v3 2/5] bitmap: rename rte_bsf64 and move to common header

[Hunt, David]

On 14/11/2018 4:52 PM, Singh, Jasvinder wrote:
>
>> -----Original Message-----
>> From: Burakov, Anatoly
>> Sent: Wednesday, November 14, 2018 4:47 PM
>> To: dev@dpdk.org
>> Cc: Neil Horman <nhorman@tuxdriver.com>; Mcnamara, John
>> <john.mcnamara@intel.com>; Kovacevic, Marko
>> <marko.kovacevic@intel.com>; Dumitrescu, Cristian
>> <cristian.dumitrescu@intel.com>; thomas@monjalon.net; Richardson, Bruce
>> <bruce.richardson@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Singh,
>> Jasvinder <jasvinder.singh@intel.com>
>> Subject: [PATCH v3 2/5] bitmap: rename rte_bsf64 and move to common
>> header
>>
>> 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>
>> ---
>>
>> Notes:
>>      v3:
>>      - Added clarification that pos is not checked
>>
>>   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(-)
>>
> Acked-by: Jasvinder Singh <jasvinder.singh@intel.com>


Acked-by: David Hunt <david.hunt@intel.com>