DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v5 0/1] mbuf: add bulk free function
@ 2019-10-09 13:55 Morten Brørup
  2019-10-09 13:55 ` [dpdk-dev] [PATCH v5 1/1] " Morten Brørup
  0 siblings, 1 reply; 9+ messages in thread
From: Morten Brørup @ 2019-10-09 13:55 UTC (permalink / raw)
  To: olivier.matz
  Cc: stephen, harry.van.haaren, konstantin.ananyev, mattias.ronnblom,
	bruce.richardson, arybchenko, dev, Morten Brørup

Add function for freeing a bulk of mbufs.

v5:
* Rename variables from "free" to "pending" for improved readability.
* Add prefix __ to rte_pktmbuf_free_seg_via_array().
* Add array size parameter to __rte_pktmbuf_free_seg_via_array().
  The compiler will optimize the parameter away anyway.
* Add description to __rte_pktmbuf_free_seg_via_array().
* Minor description updates.
v4:
* Mark as experimental by adding __rte_experimental.
* Add function to experimental section of map file.
* Fix source code formatting regarding pointer to pointer.
* Squash multiple modifications into one.
v3:
* Bugfix: Handle pakets with multiple segments.
* Add inline helper function, mainly for readability.
* Fix source code formatting regarding indentation.
v2:
* Function is not inline.
* Optimize to free multible mbufs belonging to the same mempool in
  bulk. Inspired by ixgbe_tx_free_bufs(), but allowing NULL pointers
  in the array, just like rte_pktmbuf_free() can take a NULL pointer.
* Use unsigned int instead of unsigned. Passes checkpatch, but
  mismatches the original coding style of the modified files.
* Fix a typo in the description headline: mempools is plural.

Morten Brørup (1):
  mbuf: add bulk free function

 lib/librte_mbuf/rte_mbuf.c           | 66 ++++++++++++++++++++++++++++
 lib/librte_mbuf/rte_mbuf.h           | 15 +++++++
 lib/librte_mbuf/rte_mbuf_version.map |  1 +
 3 files changed, 82 insertions(+)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [dpdk-dev] [PATCH v5 1/1] mbuf: add bulk free function
  2019-10-09 13:55 [dpdk-dev] [PATCH v5 0/1] mbuf: add bulk free function Morten Brørup
@ 2019-10-09 13:55 ` Morten Brørup
  2019-10-09 22:54   ` Stephen Hemminger
                     ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Morten Brørup @ 2019-10-09 13:55 UTC (permalink / raw)
  To: olivier.matz
  Cc: stephen, harry.van.haaren, konstantin.ananyev, mattias.ronnblom,
	bruce.richardson, arybchenko, dev, Morten Brørup

Add function for freeing a bulk of mbufs.

v5:
* Rename variables from "free" to "pending" for improved readability.
* Add prefix __ to rte_pktmbuf_free_seg_via_array().
* Add array size parameter to __rte_pktmbuf_free_seg_via_array().
  The compiler will optimize the parameter away anyway.
* Add description to __rte_pktmbuf_free_seg_via_array().
* Minor description updates.
v4:
* Mark as experimental by adding __rte_experimental.
* Add function to experimental section of map file.
* Fix source code formatting regarding pointer to pointer.
* Squash multiple modifications into one.
v3:
* Bugfix: Handle pakets with multiple segments.
* Add inline helper function, mainly for readability.
* Fix source code formatting regarding indentation.
v2:
* Function is not inline.
* Optimize to free multible mbufs belonging to the same mempool in
  bulk. Inspired by ixgbe_tx_free_bufs(), but allowing NULL pointers
  in the array, just like rte_pktmbuf_free() can take a NULL pointer.
* Use unsigned int instead of unsigned. Passes checkpatch, but
  mismatches the original coding style of the modified files.
* Fix a typo in the description headline: mempools is plural.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/librte_mbuf/rte_mbuf.c           | 66 ++++++++++++++++++++++++++++
 lib/librte_mbuf/rte_mbuf.h           | 15 +++++++
 lib/librte_mbuf/rte_mbuf_version.map |  1 +
 3 files changed, 82 insertions(+)

diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index 37718d49c..de5659baa 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -245,6 +245,72 @@ int rte_mbuf_check(const struct rte_mbuf *m, int is_header,
 	return 0;
 }
 
+/**
+ * @internal helper function for freeing a bulk of packet mbuf segments
+ * via an array holding the packet mbuf segments from the same mempool
+ * pending to be freed.
+ *
+ * @param m
+ *  The packet mbuf segment to be freed.
+ * @param pending
+ *  Pointer to the array of packet mbuf segments pending to be freed.
+ * @param nb_pending
+ *  Pointer to the number of elements held in the array.
+ * @param pending_sz
+ *  Number of elements the array can hold.
+ *  Note: The compiler should optimize this parameter away when using a
+ *  constant value, such as RTE_PKTMBUF_FREE_PENDING_SZ.
+ */
+static __rte_always_inline void
+__rte_pktmbuf_free_seg_via_array(struct rte_mbuf *m,
+	struct rte_mbuf ** const pending, unsigned int * const nb_pending,
+	const unsigned int pending_sz)
+{
+	m = rte_pktmbuf_prefree_seg(m);
+	if (likely(m != NULL)) {
+		if (*nb_pending == pending_sz ||
+		    (*nb_pending > 0 && m->pool != pending[0]->pool)) {
+			rte_mempool_put_bulk(pending[0]->pool,
+					(void **)pending, *nb_pending);
+			*nb_pending = 0;
+		}
+
+		pending[(*nb_pending)++] = m;
+	}
+}
+
+/**
+ * Size of the array holding mbufs from the same membool pending to be freed
+ * in bulk.
+ */
+#define RTE_PKTMBUF_FREE_PENDING_SZ 64
+
+/* Free a bulk of packet mbufs back into their original mempools. */
+void rte_pktmbuf_free_bulk(struct rte_mbuf **mbufs, unsigned int count)
+{
+	struct rte_mbuf *m, *m_next, *pending[RTE_PKTMBUF_FREE_PENDING_SZ];
+	unsigned int idx, nb_pending = 0;
+
+	for (idx = 0; idx < count; idx++) {
+		m = mbufs[idx];
+		if (unlikely(m == NULL))
+			continue;
+
+		__rte_mbuf_sanity_check(m, 1);
+
+		do {
+			m_next = m->next;
+			__rte_pktmbuf_free_seg_via_array(m,
+					pending, &nb_pending,
+					RTE_PKTMBUF_FREE_PENDING_SZ);
+			m = m_next;
+		} while (m != NULL);
+	}
+
+	if (nb_pending > 0)
+		rte_mempool_put_bulk(pending[0]->pool, (void **)pending, nb_pending);
+}
+
 /* dump a mbuf on console */
 void
 rte_pktmbuf_dump(FILE *f, const struct rte_mbuf *m, unsigned dump_len)
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 98225ec80..a02c8b12c 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -1907,6 +1907,21 @@ static inline void rte_pktmbuf_free(struct rte_mbuf *m)
 	}
 }
 
+/**
+ * Free a bulk of packet mbufs back into their original mempools.
+ *
+ * Free a bulk of mbufs, and all their segments in case of chained buffers.
+ * Each segment is added back into its original mempool.
+ *
+ *  @param mbufs
+ *    Array of pointers to packet mbufs.
+ *    The array may contain NULL pointers.
+ *  @param count
+ *    Array size.
+ */
+__rte_experimental
+void rte_pktmbuf_free_bulk(struct rte_mbuf **mbufs, unsigned int count);
+
 /**
  * Creates a "clone" of the given packet mbuf.
  *
diff --git a/lib/librte_mbuf/rte_mbuf_version.map b/lib/librte_mbuf/rte_mbuf_version.map
index 2662a37bf..cd6154ef2 100644
--- a/lib/librte_mbuf/rte_mbuf_version.map
+++ b/lib/librte_mbuf/rte_mbuf_version.map
@@ -50,4 +50,5 @@ EXPERIMENTAL {
 	global:
 
 	rte_mbuf_check;
+	rte_pktmbuf_free_bulk;
 } DPDK_18.08;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH v5 1/1] mbuf: add bulk free function
  2019-10-09 13:55 ` [dpdk-dev] [PATCH v5 1/1] " Morten Brørup
@ 2019-10-09 22:54   ` Stephen Hemminger
  2019-10-10  6:25     ` Andrew Rybchenko
  2019-10-10  0:01   ` Ananyev, Konstantin
  2019-10-10  8:33   ` [dpdk-dev] [PATCH v6 0/2] " Morten Brørup
  2 siblings, 1 reply; 9+ messages in thread
From: Stephen Hemminger @ 2019-10-09 22:54 UTC (permalink / raw)
  To: Morten Brørup
  Cc: olivier.matz, harry.van.haaren, konstantin.ananyev,
	mattias.ronnblom, bruce.richardson, arybchenko, dev

On Wed,  9 Oct 2019 13:55:11 +0000
Morten Brørup <mb@smartsharesystems.com> wrote:

>  
> +/**
> + * @internal helper function for freeing a bulk of packet mbuf segments
> + * via an array holding the packet mbuf segments from the same mempool
> + * pending to be freed.
> + *
> + * @param m
> + *  The packet mbuf segment to be freed.
> + * @param pending
> + *  Pointer to the array of packet mbuf segments pending to be freed.
> + * @param nb_pending
> + *  Pointer to the number of elements held in the array.
> + * @param pending_sz
> + *  Number of elements the array can hold.
> + *  Note: The compiler should optimize this parameter away when using a
> + *  constant value, such as RTE_PKTMBUF_FREE_PENDING_SZ.
> + */
> +static __rte_always_inline void
> +__rte_pktmbuf_free_seg_via_array(struct rte_mbuf *m,

Overall the patch looks good, but don't think always_inline
is required here. That should be reserved for things that use
inline assembly or other stuff that would be broken if it wasn't
inlined.

Most compilers would inline it without any modifier anyway.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH v5 1/1] mbuf: add bulk free function
  2019-10-09 13:55 ` [dpdk-dev] [PATCH v5 1/1] " Morten Brørup
  2019-10-09 22:54   ` Stephen Hemminger
@ 2019-10-10  0:01   ` Ananyev, Konstantin
  2019-10-10  8:33   ` [dpdk-dev] [PATCH v6 0/2] " Morten Brørup
  2 siblings, 0 replies; 9+ messages in thread
From: Ananyev, Konstantin @ 2019-10-10  0:01 UTC (permalink / raw)
  To: Morten Brørup, olivier.matz
  Cc: stephen, Van Haaren, Harry, mattias.ronnblom, Richardson, Bruce,
	arybchenko, dev


> Add function for freeing a bulk of mbufs.
> 
> v5:
> * Rename variables from "free" to "pending" for improved readability.
> * Add prefix __ to rte_pktmbuf_free_seg_via_array().
> * Add array size parameter to __rte_pktmbuf_free_seg_via_array().
>   The compiler will optimize the parameter away anyway.
> * Add description to __rte_pktmbuf_free_seg_via_array().
> * Minor description updates.
> v4:
> * Mark as experimental by adding __rte_experimental.
> * Add function to experimental section of map file.
> * Fix source code formatting regarding pointer to pointer.
> * Squash multiple modifications into one.
> v3:
> * Bugfix: Handle pakets with multiple segments.
> * Add inline helper function, mainly for readability.
> * Fix source code formatting regarding indentation.
> v2:
> * Function is not inline.
> * Optimize to free multible mbufs belonging to the same mempool in
>   bulk. Inspired by ixgbe_tx_free_bufs(), but allowing NULL pointers
>   in the array, just like rte_pktmbuf_free() can take a NULL pointer.
> * Use unsigned int instead of unsigned. Passes checkpatch, but
>   mismatches the original coding style of the modified files.
> * Fix a typo in the description headline: mempools is plural.
> 
> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> ---
>  lib/librte_mbuf/rte_mbuf.c           | 66 ++++++++++++++++++++++++++++
>  lib/librte_mbuf/rte_mbuf.h           | 15 +++++++
>  lib/librte_mbuf/rte_mbuf_version.map |  1 +
>  3 files changed, 82 insertions(+)
> 
> diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
> index 37718d49c..de5659baa 100644
> --- a/lib/librte_mbuf/rte_mbuf.c
> +++ b/lib/librte_mbuf/rte_mbuf.c
> @@ -245,6 +245,72 @@ int rte_mbuf_check(const struct rte_mbuf *m, int is_header,
>  	return 0;
>  }
> 
> +/**
> + * @internal helper function for freeing a bulk of packet mbuf segments
> + * via an array holding the packet mbuf segments from the same mempool
> + * pending to be freed.
> + *
> + * @param m
> + *  The packet mbuf segment to be freed.
> + * @param pending
> + *  Pointer to the array of packet mbuf segments pending to be freed.
> + * @param nb_pending
> + *  Pointer to the number of elements held in the array.
> + * @param pending_sz
> + *  Number of elements the array can hold.
> + *  Note: The compiler should optimize this parameter away when using a
> + *  constant value, such as RTE_PKTMBUF_FREE_PENDING_SZ.
> + */
> +static __rte_always_inline void
> +__rte_pktmbuf_free_seg_via_array(struct rte_mbuf *m,
> +	struct rte_mbuf ** const pending, unsigned int * const nb_pending,
> +	const unsigned int pending_sz)
> +{
> +	m = rte_pktmbuf_prefree_seg(m);
> +	if (likely(m != NULL)) {
> +		if (*nb_pending == pending_sz ||
> +		    (*nb_pending > 0 && m->pool != pending[0]->pool)) {
> +			rte_mempool_put_bulk(pending[0]->pool,
> +					(void **)pending, *nb_pending);
> +			*nb_pending = 0;
> +		}
> +
> +		pending[(*nb_pending)++] = m;
> +	}
> +}
> +
> +/**
> + * Size of the array holding mbufs from the same membool pending to be freed
> + * in bulk.
> + */
> +#define RTE_PKTMBUF_FREE_PENDING_SZ 64
> +
> +/* Free a bulk of packet mbufs back into their original mempools. */
> +void rte_pktmbuf_free_bulk(struct rte_mbuf **mbufs, unsigned int count)
> +{
> +	struct rte_mbuf *m, *m_next, *pending[RTE_PKTMBUF_FREE_PENDING_SZ];
> +	unsigned int idx, nb_pending = 0;
> +
> +	for (idx = 0; idx < count; idx++) {
> +		m = mbufs[idx];
> +		if (unlikely(m == NULL))
> +			continue;
> +
> +		__rte_mbuf_sanity_check(m, 1);
> +
> +		do {
> +			m_next = m->next;
> +			__rte_pktmbuf_free_seg_via_array(m,
> +					pending, &nb_pending,
> +					RTE_PKTMBUF_FREE_PENDING_SZ);
> +			m = m_next;
> +		} while (m != NULL);
> +	}
> +
> +	if (nb_pending > 0)
> +		rte_mempool_put_bulk(pending[0]->pool, (void **)pending, nb_pending);
> +}
> +
>  /* dump a mbuf on console */
>  void
>  rte_pktmbuf_dump(FILE *f, const struct rte_mbuf *m, unsigned dump_len)
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index 98225ec80..a02c8b12c 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -1907,6 +1907,21 @@ static inline void rte_pktmbuf_free(struct rte_mbuf *m)
>  	}
>  }
> 
> +/**
> + * Free a bulk of packet mbufs back into their original mempools.
> + *
> + * Free a bulk of mbufs, and all their segments in case of chained buffers.
> + * Each segment is added back into its original mempool.
> + *
> + *  @param mbufs
> + *    Array of pointers to packet mbufs.
> + *    The array may contain NULL pointers.
> + *  @param count
> + *    Array size.
> + */
> +__rte_experimental
> +void rte_pktmbuf_free_bulk(struct rte_mbuf **mbufs, unsigned int count);
> +
>  /**
>   * Creates a "clone" of the given packet mbuf.
>   *
> diff --git a/lib/librte_mbuf/rte_mbuf_version.map b/lib/librte_mbuf/rte_mbuf_version.map
> index 2662a37bf..cd6154ef2 100644
> --- a/lib/librte_mbuf/rte_mbuf_version.map
> +++ b/lib/librte_mbuf/rte_mbuf_version.map
> @@ -50,4 +50,5 @@ EXPERIMENTAL {
>  	global:
> 
>  	rte_mbuf_check;
> +	rte_pktmbuf_free_bulk;
>  } DPDK_18.08;
> --

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

> 2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH v5 1/1] mbuf: add bulk free function
  2019-10-09 22:54   ` Stephen Hemminger
@ 2019-10-10  6:25     ` Andrew Rybchenko
  0 siblings, 0 replies; 9+ messages in thread
From: Andrew Rybchenko @ 2019-10-10  6:25 UTC (permalink / raw)
  To: Stephen Hemminger, Morten Brørup
  Cc: olivier.matz, harry.van.haaren, konstantin.ananyev,
	mattias.ronnblom, bruce.richardson, dev

On 10/10/19 1:54 AM, Stephen Hemminger wrote:
> On Wed,  9 Oct 2019 13:55:11 +0000
> Morten Brørup <mb@smartsharesystems.com> wrote:
>
>>   
>> +/**
>> + * @internal helper function for freeing a bulk of packet mbuf segments
>> + * via an array holding the packet mbuf segments from the same mempool
>> + * pending to be freed.
>> + *
>> + * @param m
>> + *  The packet mbuf segment to be freed.
>> + * @param pending
>> + *  Pointer to the array of packet mbuf segments pending to be freed.
>> + * @param nb_pending
>> + *  Pointer to the number of elements held in the array.
>> + * @param pending_sz
>> + *  Number of elements the array can hold.
>> + *  Note: The compiler should optimize this parameter away when using a
>> + *  constant value, such as RTE_PKTMBUF_FREE_PENDING_SZ.
>> + */
>> +static __rte_always_inline void
>> +__rte_pktmbuf_free_seg_via_array(struct rte_mbuf *m,
> Overall the patch looks good, but don't think always_inline
> is required here. That should be reserved for things that use
> inline assembly or other stuff that would be broken if it wasn't
> inlined.
>
> Most compilers would inline it without any modifier anyway.

I agree with Stephen that always_inline is not required here.

Anyway,

Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [dpdk-dev] [PATCH v6 0/2] mbuf: add bulk free function
  2019-10-09 13:55 ` [dpdk-dev] [PATCH v5 1/1] " Morten Brørup
  2019-10-09 22:54   ` Stephen Hemminger
  2019-10-10  0:01   ` Ananyev, Konstantin
@ 2019-10-10  8:33   ` Morten Brørup
  2019-10-10  8:33     ` [dpdk-dev] [PATCH v6 1/2] " Morten Brørup
                       ` (2 more replies)
  2 siblings, 3 replies; 9+ messages in thread
From: Morten Brørup @ 2019-10-10  8:33 UTC (permalink / raw)
  To: dev
  Cc: olivier.matz, stephen, harry.van.haaren, konstantin.ananyev,
	mattias.ronnblom, bruce.richardson, arybchenko,
	Morten Brørup

Add function for freeing a bulk of mbufs.

v6:
* Remove __rte_always_inline from static function.
  The compiler will inline anyway.
v5:
* Rename variables from "free" to "pending" for improved readability.
* Add prefix __ to rte_pktmbuf_free_seg_via_array().
* Add array size parameter to __rte_pktmbuf_free_seg_via_array().
  The compiler will optimize the parameter away anyway.
* Add description to __rte_pktmbuf_free_seg_via_array().
* Minor description updates.
v4:
* Mark as experimental by adding __rte_experimental.
* Add function to experimental section of map file.
* Fix source code formatting regarding pointer to pointer.
* Squash multiple modifications into one.
v3:
* Bugfix: Handle pakets with multiple segments.
* Add inline helper function, mainly for readability.
* Fix source code formatting regarding indentation.
v2:
* Function is not inline.
* Optimize to free multible mbufs belonging to the same mempool in
  bulk. Inspired by ixgbe_tx_free_bufs(), but allowing NULL pointers
  in the array, just like rte_pktmbuf_free() can take a NULL pointer.
* Use unsigned int instead of unsigned. Passes checkpatch, but
  mismatches the original coding style of the modified files.
* Fix a typo in the description headline: mempools is plural.

Morten Brørup (2):
  mbuf: add bulk free function
  mbuf: add bulk free function

 lib/librte_mbuf/rte_mbuf.c           | 66 ++++++++++++++++++++++++++++
 lib/librte_mbuf/rte_mbuf.h           | 15 +++++++
 lib/librte_mbuf/rte_mbuf_version.map |  1 +
 3 files changed, 82 insertions(+)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [dpdk-dev] [PATCH v6 1/2] mbuf: add bulk free function
  2019-10-10  8:33   ` [dpdk-dev] [PATCH v6 0/2] " Morten Brørup
@ 2019-10-10  8:33     ` Morten Brørup
  2019-10-10  8:33     ` [dpdk-dev] [PATCH v6 2/2] " Morten Brørup
  2019-10-10  8:49     ` [dpdk-dev] [PATCH v6 0/2] " Morten Brørup
  2 siblings, 0 replies; 9+ messages in thread
From: Morten Brørup @ 2019-10-10  8:33 UTC (permalink / raw)
  To: dev
  Cc: olivier.matz, stephen, harry.van.haaren, konstantin.ananyev,
	mattias.ronnblom, bruce.richardson, arybchenko,
	Morten Brørup

Add function for freeing a bulk of mbufs.

v6:
* Remove __rte_always_inline from static function.
  The compiler will inline anyway.
v5:
* Rename variables from "free" to "pending" for improved readability.
* Add prefix __ to rte_pktmbuf_free_seg_via_array().
* Add array size parameter to __rte_pktmbuf_free_seg_via_array().
  The compiler will optimize the parameter away anyway.
* Add description to __rte_pktmbuf_free_seg_via_array().
* Minor description updates.
v4:
* Mark as experimental by adding __rte_experimental.
* Add function to experimental section of map file.
* Fix source code formatting regarding pointer to pointer.
* Squash multiple modifications into one.
v3:
* Bugfix: Handle pakets with multiple segments.
* Add inline helper function, mainly for readability.
* Fix source code formatting regarding indentation.
v2:
* Function is not inline.
* Optimize to free multible mbufs belonging to the same mempool in
  bulk. Inspired by ixgbe_tx_free_bufs(), but allowing NULL pointers
  in the array, just like rte_pktmbuf_free() can take a NULL pointer.
* Use unsigned int instead of unsigned. Passes checkpatch, but
  mismatches the original coding style of the modified files.
* Fix a typo in the description headline: mempools is plural.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/librte_mbuf/rte_mbuf.c           | 66 ++++++++++++++++++++++++++++
 lib/librte_mbuf/rte_mbuf.h           | 15 +++++++
 lib/librte_mbuf/rte_mbuf_version.map |  1 +
 3 files changed, 82 insertions(+)

diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index 37718d49c..de5659baa 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -245,6 +245,72 @@ int rte_mbuf_check(const struct rte_mbuf *m, int is_header,
 	return 0;
 }
 
+/**
+ * @internal helper function for freeing a bulk of packet mbuf segments
+ * via an array holding the packet mbuf segments from the same mempool
+ * pending to be freed.
+ *
+ * @param m
+ *  The packet mbuf segment to be freed.
+ * @param pending
+ *  Pointer to the array of packet mbuf segments pending to be freed.
+ * @param nb_pending
+ *  Pointer to the number of elements held in the array.
+ * @param pending_sz
+ *  Number of elements the array can hold.
+ *  Note: The compiler should optimize this parameter away when using a
+ *  constant value, such as RTE_PKTMBUF_FREE_PENDING_SZ.
+ */
+static __rte_always_inline void
+__rte_pktmbuf_free_seg_via_array(struct rte_mbuf *m,
+	struct rte_mbuf ** const pending, unsigned int * const nb_pending,
+	const unsigned int pending_sz)
+{
+	m = rte_pktmbuf_prefree_seg(m);
+	if (likely(m != NULL)) {
+		if (*nb_pending == pending_sz ||
+		    (*nb_pending > 0 && m->pool != pending[0]->pool)) {
+			rte_mempool_put_bulk(pending[0]->pool,
+					(void **)pending, *nb_pending);
+			*nb_pending = 0;
+		}
+
+		pending[(*nb_pending)++] = m;
+	}
+}
+
+/**
+ * Size of the array holding mbufs from the same membool pending to be freed
+ * in bulk.
+ */
+#define RTE_PKTMBUF_FREE_PENDING_SZ 64
+
+/* Free a bulk of packet mbufs back into their original mempools. */
+void rte_pktmbuf_free_bulk(struct rte_mbuf **mbufs, unsigned int count)
+{
+	struct rte_mbuf *m, *m_next, *pending[RTE_PKTMBUF_FREE_PENDING_SZ];
+	unsigned int idx, nb_pending = 0;
+
+	for (idx = 0; idx < count; idx++) {
+		m = mbufs[idx];
+		if (unlikely(m == NULL))
+			continue;
+
+		__rte_mbuf_sanity_check(m, 1);
+
+		do {
+			m_next = m->next;
+			__rte_pktmbuf_free_seg_via_array(m,
+					pending, &nb_pending,
+					RTE_PKTMBUF_FREE_PENDING_SZ);
+			m = m_next;
+		} while (m != NULL);
+	}
+
+	if (nb_pending > 0)
+		rte_mempool_put_bulk(pending[0]->pool, (void **)pending, nb_pending);
+}
+
 /* dump a mbuf on console */
 void
 rte_pktmbuf_dump(FILE *f, const struct rte_mbuf *m, unsigned dump_len)
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 98225ec80..a02c8b12c 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -1907,6 +1907,21 @@ static inline void rte_pktmbuf_free(struct rte_mbuf *m)
 	}
 }
 
+/**
+ * Free a bulk of packet mbufs back into their original mempools.
+ *
+ * Free a bulk of mbufs, and all their segments in case of chained buffers.
+ * Each segment is added back into its original mempool.
+ *
+ *  @param mbufs
+ *    Array of pointers to packet mbufs.
+ *    The array may contain NULL pointers.
+ *  @param count
+ *    Array size.
+ */
+__rte_experimental
+void rte_pktmbuf_free_bulk(struct rte_mbuf **mbufs, unsigned int count);
+
 /**
  * Creates a "clone" of the given packet mbuf.
  *
diff --git a/lib/librte_mbuf/rte_mbuf_version.map b/lib/librte_mbuf/rte_mbuf_version.map
index 2662a37bf..cd6154ef2 100644
--- a/lib/librte_mbuf/rte_mbuf_version.map
+++ b/lib/librte_mbuf/rte_mbuf_version.map
@@ -50,4 +50,5 @@ EXPERIMENTAL {
 	global:
 
 	rte_mbuf_check;
+	rte_pktmbuf_free_bulk;
 } DPDK_18.08;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [dpdk-dev] [PATCH v6 2/2] mbuf: add bulk free function
  2019-10-10  8:33   ` [dpdk-dev] [PATCH v6 0/2] " Morten Brørup
  2019-10-10  8:33     ` [dpdk-dev] [PATCH v6 1/2] " Morten Brørup
@ 2019-10-10  8:33     ` Morten Brørup
  2019-10-10  8:49     ` [dpdk-dev] [PATCH v6 0/2] " Morten Brørup
  2 siblings, 0 replies; 9+ messages in thread
From: Morten Brørup @ 2019-10-10  8:33 UTC (permalink / raw)
  To: dev
  Cc: olivier.matz, stephen, harry.van.haaren, konstantin.ananyev,
	mattias.ronnblom, bruce.richardson, arybchenko,
	Morten Brørup

Add function for freeing a bulk of mbufs.

v6:
* Remove __rte_always_inline from static function.
  The compiler will inline anyway.
v5:
* Rename variables from "free" to "pending" for improved readability.
* Add prefix __ to rte_pktmbuf_free_seg_via_array().
* Add array size parameter to __rte_pktmbuf_free_seg_via_array().
  The compiler will optimize the parameter away anyway.
* Add description to __rte_pktmbuf_free_seg_via_array().
* Minor description updates.
v4:
* Mark as experimental by adding __rte_experimental.
* Add function to experimental section of map file.
* Fix source code formatting regarding pointer to pointer.
* Squash multiple modifications into one.
v3:
* Bugfix: Handle pakets with multiple segments.
* Add inline helper function, mainly for readability.
* Fix source code formatting regarding indentation.
v2:
* Function is not inline.
* Optimize to free multible mbufs belonging to the same mempool in
  bulk. Inspired by ixgbe_tx_free_bufs(), but allowing NULL pointers
  in the array, just like rte_pktmbuf_free() can take a NULL pointer.
* Use unsigned int instead of unsigned. Passes checkpatch, but
  mismatches the original coding style of the modified files.
* Fix a typo in the description headline: mempools is plural.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/librte_mbuf/rte_mbuf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index de5659baa..854af1104 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -261,7 +261,7 @@ int rte_mbuf_check(const struct rte_mbuf *m, int is_header,
  *  Note: The compiler should optimize this parameter away when using a
  *  constant value, such as RTE_PKTMBUF_FREE_PENDING_SZ.
  */
-static __rte_always_inline void
+static void
 __rte_pktmbuf_free_seg_via_array(struct rte_mbuf *m,
 	struct rte_mbuf ** const pending, unsigned int * const nb_pending,
 	const unsigned int pending_sz)
-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH v6 0/2] mbuf: add bulk free function
  2019-10-10  8:33   ` [dpdk-dev] [PATCH v6 0/2] " Morten Brørup
  2019-10-10  8:33     ` [dpdk-dev] [PATCH v6 1/2] " Morten Brørup
  2019-10-10  8:33     ` [dpdk-dev] [PATCH v6 2/2] " Morten Brørup
@ 2019-10-10  8:49     ` Morten Brørup
  2 siblings, 0 replies; 9+ messages in thread
From: Morten Brørup @ 2019-10-10  8:49 UTC (permalink / raw)
  To: olivier.matz, stephen, harry.van.haaren, konstantin.ananyev,
	mattias.ronnblom, bruce.richardson, arybchenko
  Cc: dev

Olivier, Stephen, Harry, Konstantin, Mattias, Bruce and Andrew,

Thank you for all your constructive feedback on my first non-trivial source code contribution to DPDK.

And thank you for tolerating all the noise from my unfamiliarity with git and patchwork.


Med venlig hilsen / kind regards
- Morten Brørup

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-10-10  8:49 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-09 13:55 [dpdk-dev] [PATCH v5 0/1] mbuf: add bulk free function Morten Brørup
2019-10-09 13:55 ` [dpdk-dev] [PATCH v5 1/1] " Morten Brørup
2019-10-09 22:54   ` Stephen Hemminger
2019-10-10  6:25     ` Andrew Rybchenko
2019-10-10  0:01   ` Ananyev, Konstantin
2019-10-10  8:33   ` [dpdk-dev] [PATCH v6 0/2] " Morten Brørup
2019-10-10  8:33     ` [dpdk-dev] [PATCH v6 1/2] " Morten Brørup
2019-10-10  8:33     ` [dpdk-dev] [PATCH v6 2/2] " Morten Brørup
2019-10-10  8:49     ` [dpdk-dev] [PATCH v6 0/2] " Morten Brørup

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