* [dpdk-dev] [PATCH v6 0/2] mbuf: add bulk free function @ 2019-10-10 9:34 Morten Brørup 2019-10-10 9:34 ` [dpdk-dev] [PATCH v6 1/2] " Morten Brørup 2019-10-10 9:34 ` [dpdk-dev] [PATCH v6 2/2] " Morten Brørup 0 siblings, 2 replies; 6+ messages in thread From: Morten Brørup @ 2019-10-10 9:34 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. Signed-off-by: Morten Brørup <mb@smartsharesystems.com> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com> --- 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] 6+ messages in thread
* [dpdk-dev] [PATCH v6 1/2] mbuf: add bulk free function 2019-10-10 9:34 [dpdk-dev] [PATCH v6 0/2] mbuf: add bulk free function Morten Brørup @ 2019-10-10 9:34 ` Morten Brørup 2019-10-10 14:13 ` Morten Brørup 2019-10-10 9:34 ` [dpdk-dev] [PATCH v6 2/2] " Morten Brørup 1 sibling, 1 reply; 6+ messages in thread From: Morten Brørup @ 2019-10-10 9:34 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. Signed-off-by: Morten Brørup <mb@smartsharesystems.com> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com> --- 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. --- 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] 6+ messages in thread
* Re: [dpdk-dev] [PATCH v6 1/2] mbuf: add bulk free function 2019-10-10 9:34 ` [dpdk-dev] [PATCH v6 1/2] " Morten Brørup @ 2019-10-10 14:13 ` Morten Brørup 2019-10-10 16:13 ` [dpdk-dev] [dpdk-ci] " Jeremy Plsek 2019-10-10 16:28 ` [dpdk-dev] " Ananyev, Konstantin 0 siblings, 2 replies; 6+ messages in thread From: Morten Brørup @ 2019-10-10 14:13 UTC (permalink / raw) To: ci; +Cc: dev Hi CI Team, Although my patch only adds a single function, which is not being called from anywhere, the patch failed the iol-intel-Performance test: http://mails.dpdk.org/archives/test-report/2019-October/101829.html So I think something went wrong with the test environment. Is there any way to trigger a re-run of the test? Med venlig hilsen / kind regards - Morten Brørup > -----Original Message----- > From: Morten Brørup [mailto:mb@smartsharesystems.com] > Sent: Thursday, October 10, 2019 11:35 AM > To: dev@dpdk.org > Cc: olivier.matz@6wind.com; stephen@networkplumber.org; > harry.van.haaren@intel.com; konstantin.ananyev@intel.com; > mattias.ronnblom@ericsson.com; bruce.richardson@intel.com; > arybchenko@solarflare.com; Morten Brørup > Subject: [PATCH v6 1/2] mbuf: add bulk free function > > Add function for freeing a bulk of mbufs. > > Signed-off-by: Morten Brørup <mb@smartsharesystems.com> > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com> > Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com> > > --- > > 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. > > --- > 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] 6+ messages in thread
* Re: [dpdk-dev] [dpdk-ci] [PATCH v6 1/2] mbuf: add bulk free function 2019-10-10 14:13 ` Morten Brørup @ 2019-10-10 16:13 ` Jeremy Plsek 2019-10-10 16:28 ` [dpdk-dev] " Ananyev, Konstantin 1 sibling, 0 replies; 6+ messages in thread From: Jeremy Plsek @ 2019-10-10 16:13 UTC (permalink / raw) To: Morten Brørup; +Cc: ci, dev Hi Morten, Sometimes the tests report a false positive. Unfortunately, there isn't a way yet for tests to be reran by contributors. At the moment, only the environment owners (in this case, some members from the Intel team) and I (who maintains the "iol" prefixed CI) can rerun the test. Depending on the patch, the maintainers should realize that a patch shouldn't be reporting a failure and ignore the result (or rerun if they are able to). I've reran the test for you. Although, I've noticed a new patch version was sent (and passed), so it won't really make a difference. Thanks On Thu, Oct 10, 2019 at 10:13 AM Morten Brørup <mb@smartsharesystems.com> wrote: > > Hi CI Team, > > Although my patch only adds a single function, which is not being called from anywhere, the patch failed the iol-intel-Performance test: > http://mails.dpdk.org/archives/test-report/2019-October/101829.html > > So I think something went wrong with the test environment. > > Is there any way to trigger a re-run of the test? > > > Med venlig hilsen / kind regards > - Morten Brørup > > > > -----Original Message----- > > From: Morten Brørup [mailto:mb@smartsharesystems.com] > > Sent: Thursday, October 10, 2019 11:35 AM > > To: dev@dpdk.org > > Cc: olivier.matz@6wind.com; stephen@networkplumber.org; > > harry.van.haaren@intel.com; konstantin.ananyev@intel.com; > > mattias.ronnblom@ericsson.com; bruce.richardson@intel.com; > > arybchenko@solarflare.com; Morten Brørup > > Subject: [PATCH v6 1/2] mbuf: add bulk free function > > > > Add function for freeing a bulk of mbufs. > > > > Signed-off-by: Morten Brørup <mb@smartsharesystems.com> > > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com> > > Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com> > > > > --- > > > > 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. > > > > --- > > 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 > -- Jeremy Plsek UNH InterOperability Laboratory ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH v6 1/2] mbuf: add bulk free function 2019-10-10 14:13 ` Morten Brørup 2019-10-10 16:13 ` [dpdk-dev] [dpdk-ci] " Jeremy Plsek @ 2019-10-10 16:28 ` Ananyev, Konstantin 1 sibling, 0 replies; 6+ messages in thread From: Ananyev, Konstantin @ 2019-10-10 16:28 UTC (permalink / raw) To: Morten Brørup, ci; +Cc: dev > > Hi CI Team, > > Although my patch only adds a single function, which is not being called from anywhere, Which actually means we forgot that UT would also probably be needed for that addition :( > the patch failed the iol-intel-Performance test: > http://mails.dpdk.org/archives/test-report/2019-October/101829.html > > So I think something went wrong with the test environment. > > Is there any way to trigger a re-run of the test? > > > Med venlig hilsen / kind regards > - Morten Brørup > > > > -----Original Message----- > > From: Morten Brørup [mailto:mb@smartsharesystems.com] > > Sent: Thursday, October 10, 2019 11:35 AM > > To: dev@dpdk.org > > Cc: olivier.matz@6wind.com; stephen@networkplumber.org; > > harry.van.haaren@intel.com; konstantin.ananyev@intel.com; > > mattias.ronnblom@ericsson.com; bruce.richardson@intel.com; > > arybchenko@solarflare.com; Morten Brørup > > Subject: [PATCH v6 1/2] mbuf: add bulk free function > > > > Add function for freeing a bulk of mbufs. > > > > Signed-off-by: Morten Brørup <mb@smartsharesystems.com> > > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com> > > Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com> > > > > --- > > > > 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. > > > > --- > > 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] 6+ messages in thread
* [dpdk-dev] [PATCH v6 2/2] mbuf: add bulk free function 2019-10-10 9:34 [dpdk-dev] [PATCH v6 0/2] mbuf: add bulk free function Morten Brørup 2019-10-10 9:34 ` [dpdk-dev] [PATCH v6 1/2] " Morten Brørup @ 2019-10-10 9:34 ` Morten Brørup 1 sibling, 0 replies; 6+ messages in thread From: Morten Brørup @ 2019-10-10 9:34 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. Signed-off-by: Morten Brørup <mb@smartsharesystems.com> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com> --- 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. --- 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] 6+ messages in thread
end of thread, other threads:[~2019-10-10 16:28 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-10 9:34 [dpdk-dev] [PATCH v6 0/2] mbuf: add bulk free function Morten Brørup 2019-10-10 9:34 ` [dpdk-dev] [PATCH v6 1/2] " Morten Brørup 2019-10-10 14:13 ` Morten Brørup 2019-10-10 16:13 ` [dpdk-dev] [dpdk-ci] " Jeremy Plsek 2019-10-10 16:28 ` [dpdk-dev] " Ananyev, Konstantin 2019-10-10 9:34 ` [dpdk-dev] [PATCH v6 2/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).