On Mon, Mar 26, 2018 at 05:12:56PM +0100, Andrew Rybchenko wrote:
> From: "Artem V. Andreev" <Artem.Andreev@oktetlabs.ru>
>
> If mempool manager supports object blocks (physically and virtual
> contiguous set of objects), it is sufficient to get the first
> object only and the function allows to avoid filling in of
> information about each block member.
>
> Signed-off-by: Artem V. Andreev <Artem.Andreev@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
Minor things here, please see below.
[...]
> @@ -1531,6 +1615,71 @@ rte_mempool_get(struct rte_mempool *mp, void **obj_p)
> }
>
> /**
> + * @internal Get contiguous blocks of objects from the pool. Used internally.
> + * @param mp
> + * A pointer to the mempool structure.
> + * @param first_obj_table
> + * A pointer to a pointer to the first object in each block.
> + * @param n
> + * A number of blocks to get.
> + * @return
> + * - >0: Success
> + * - <0: Error
I guess it is 0 here, not >0.
> + */
> +static __rte_always_inline int
> +__mempool_generic_get_contig_blocks(struct rte_mempool *mp,
> + void **first_obj_table, unsigned int n)
> +{
> + int ret;
> +
> + ret = rte_mempool_ops_dequeue_contig_blocks(mp, first_obj_table, n);
> + if (ret < 0)
> + __MEMPOOL_CONTIG_BLOCKS_STAT_ADD(mp, get_fail, n);
> + else
> + __MEMPOOL_CONTIG_BLOCKS_STAT_ADD(mp, get_success, n);
> +
> + return ret;
> +}
> +
Is it worth having this function?
I think it would be simple to include the code in
rte_mempool_get_contig_blocks() below... or am I missing something?
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Get a contiguous blocks of objects from the mempool.
> + *
> + * If cache is enabled, consider to flush it first, to reuse objects
> + * as soon as possible.
> + *
> + * The application should check that the driver supports the operation
> + * by calling rte_mempool_ops_get_info() and checking that `contig_block_size`
> + * is not zero.
> + *
> + * @param mp
> + * A pointer to the mempool structure.
> + * @param first_obj_table
> + * A pointer to a pointer to the first object in each block.
> + * @param n
> + * The number of blocks to get from mempool.
> + * @return
> + * - 0: Success; blocks taken.
> + * - -ENOBUFS: Not enough entries in the mempool; no object is retrieved.
> + * - -EOPNOTSUPP: The mempool driver does not support block dequeue
> + */
> +static __rte_always_inline int
> +__rte_experimental
> +rte_mempool_get_contig_blocks(struct rte_mempool *mp,
> + void **first_obj_table, unsigned int n)
> +{
> + int ret;
> +
> + ret = __mempool_generic_get_contig_blocks(mp, first_obj_table, n);
> + if (ret == 0)
> + __mempool_contig_blocks_check_cookies(mp, first_obj_table, n,
> + 1);
> + return ret;
> +}
On 04/19/2018 07:41 PM, Olivier Matz wrote:
> On Mon, Mar 26, 2018 at 05:12:56PM +0100, Andrew Rybchenko wrote:
> [...]
>> @@ -1531,6 +1615,71 @@ rte_mempool_get(struct rte_mempool *mp, void **obj_p)
>> }
>>
>> /**
>> + * @internal Get contiguous blocks of objects from the pool. Used internally.
>> + * @param mp
>> + * A pointer to the mempool structure.
>> + * @param first_obj_table
>> + * A pointer to a pointer to the first object in each block.
>> + * @param n
>> + * A number of blocks to get.
>> + * @return
>> + * - >0: Success
>> + * - <0: Error
> I guess it is 0 here, not >0.
Yes, thanks.
>> + */
>> +static __rte_always_inline int
>> +__mempool_generic_get_contig_blocks(struct rte_mempool *mp,
>> + void **first_obj_table, unsigned int n)
>> +{
>> + int ret;
>> +
>> + ret = rte_mempool_ops_dequeue_contig_blocks(mp, first_obj_table, n);
>> + if (ret < 0)
>> + __MEMPOOL_CONTIG_BLOCKS_STAT_ADD(mp, get_fail, n);
>> + else
>> + __MEMPOOL_CONTIG_BLOCKS_STAT_ADD(mp, get_success, n);
>> +
>> + return ret;
>> +}
>> +
> Is it worth having this function?
Just to follow the same code structure as usual dequeue.
> I think it would be simple to include the code in
> rte_mempool_get_contig_blocks() below... or am I missing something?
I agree. Will do in v3.
[...]
@@ -59,13 +59,6 @@ Deprecation Notices
- ``rte_eal_mbuf_default_mempool_ops``
-* mempool: several API and ABI changes are planned in v18.05.
-
- The following changes are planned:
-
- - addition of new op to allocate contiguous
- block of objects if underlying driver supports it.
-
* mbuf: The control mbuf API will be removed in v18.05. The impacted
functions and macros are:
@@ -10,6 +10,7 @@ CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3
# Allow deprecated symbol to use deprecated rte_mempool_populate_iova_tab()
# from earlier deprecated rte_mempool_populate_phys_tab()
CFLAGS += -Wno-deprecated-declarations
+CFLAGS += -DALLOW_EXPERIMENTAL_API
LDLIBS += -lrte_eal -lrte_ring
EXPORT_MAP := rte_mempool_version.map
@@ -1,6 +1,8 @@
# SPDX-License-Identifier: BSD-3-Clause
# Copyright(c) 2017 Intel Corporation
+allow_experimental_apis = true
+
extra_flags = []
# Allow deprecated symbol to use deprecated rte_mempool_populate_iova_tab()
@@ -1125,6 +1125,36 @@ void rte_mempool_check_cookies(const struct rte_mempool *mp,
#endif
}
+void
+rte_mempool_contig_blocks_check_cookies(const struct rte_mempool *mp,
+ void * const *first_obj_table_const, unsigned int n, int free)
+{
+#ifdef RTE_LIBRTE_MEMPOOL_DEBUG
+ struct rte_mempool_info info;
+ const size_t total_elt_sz =
+ mp->header_size + mp->elt_size + mp->trailer_size;
+ unsigned int i, j;
+
+ rte_mempool_ops_get_info(mp, &info);
+
+ for (i = 0; i < n; ++i) {
+ void *first_obj = first_obj_table_const[i];
+
+ for (j = 0; j < info.contig_block_size; ++j) {
+ void *obj;
+
+ obj = (void *)((uintptr_t)first_obj + j * total_elt_sz);
+ rte_mempool_check_cookies(mp, &obj, 1, free);
+ }
+ }
+#else
+ RTE_SET_USED(mp);
+ RTE_SET_USED(first_obj_table_const);
+ RTE_SET_USED(n);
+ RTE_SET_USED(free);
+#endif
+}
+
#ifdef RTE_LIBRTE_MEMPOOL_DEBUG
static void
mempool_obj_audit(struct rte_mempool *mp, __rte_unused void *opaque,
@@ -1190,6 +1220,7 @@ void
rte_mempool_dump(FILE *f, struct rte_mempool *mp)
{
#ifdef RTE_LIBRTE_MEMPOOL_DEBUG
+ struct rte_mempool_info info;
struct rte_mempool_debug_stats sum;
unsigned lcore_id;
#endif
@@ -1231,6 +1262,7 @@ rte_mempool_dump(FILE *f, struct rte_mempool *mp)
/* sum and dump statistics */
#ifdef RTE_LIBRTE_MEMPOOL_DEBUG
+ rte_mempool_ops_get_info(mp, &info);
memset(&sum, 0, sizeof(sum));
for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
sum.put_bulk += mp->stats[lcore_id].put_bulk;
@@ -1239,6 +1271,8 @@ rte_mempool_dump(FILE *f, struct rte_mempool *mp)
sum.get_success_objs += mp->stats[lcore_id].get_success_objs;
sum.get_fail_bulk += mp->stats[lcore_id].get_fail_bulk;
sum.get_fail_objs += mp->stats[lcore_id].get_fail_objs;
+ sum.get_success_blks += mp->stats[lcore_id].get_success_blks;
+ sum.get_fail_blks += mp->stats[lcore_id].get_fail_blks;
}
fprintf(f, " stats:\n");
fprintf(f, " put_bulk=%"PRIu64"\n", sum.put_bulk);
@@ -1247,6 +1281,11 @@ rte_mempool_dump(FILE *f, struct rte_mempool *mp)
fprintf(f, " get_success_objs=%"PRIu64"\n", sum.get_success_objs);
fprintf(f, " get_fail_bulk=%"PRIu64"\n", sum.get_fail_bulk);
fprintf(f, " get_fail_objs=%"PRIu64"\n", sum.get_fail_objs);
+ if (info.contig_block_size > 0) {
+ fprintf(f, " get_success_blks=%"PRIu64"\n",
+ sum.get_success_blks);
+ fprintf(f, " get_fail_blks=%"PRIu64"\n", sum.get_fail_blks);
+ }
#else
fprintf(f, " no statistics available\n");
#endif
@@ -70,6 +70,10 @@ struct rte_mempool_debug_stats {
uint64_t get_success_objs; /**< Objects successfully allocated. */
uint64_t get_fail_bulk; /**< Failed allocation number. */
uint64_t get_fail_objs; /**< Objects that failed to be allocated. */
+ /** Successful allocation number of contiguous blocks. */
+ uint64_t get_success_blks;
+ /** Failed allocation number of contiguous blocks. */
+ uint64_t get_fail_blks;
} __rte_cache_aligned;
#endif
@@ -195,7 +199,10 @@ struct rte_mempool_memhdr {
*
* Additional information about the mempool
*/
-struct rte_mempool_info;
+struct rte_mempool_info {
+ /** Number of objects in the contiguous block */
+ unsigned int contig_block_size;
+};
/**
* The RTE mempool structure.
@@ -273,8 +280,16 @@ struct rte_mempool {
mp->stats[__lcore_id].name##_bulk += 1; \
} \
} while(0)
+#define __MEMPOOL_CONTIG_BLOCKS_STAT_ADD(mp, name, n) do { \
+ unsigned int __lcore_id = rte_lcore_id(); \
+ if (__lcore_id < RTE_MAX_LCORE) { \
+ mp->stats[__lcore_id].name##_blks += n; \
+ mp->stats[__lcore_id].name##_bulk += 1; \
+ } \
+ } while (0)
#else
#define __MEMPOOL_STAT_ADD(mp, name, n) do {} while(0)
+#define __MEMPOOL_CONTIG_BLOCKS_STAT_ADD(mp, name, n) do {} while (0)
#endif
/**
@@ -342,6 +357,38 @@ void rte_mempool_check_cookies(const struct rte_mempool *mp,
#define __mempool_check_cookies(mp, obj_table_const, n, free) do {} while(0)
#endif /* RTE_LIBRTE_MEMPOOL_DEBUG */
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * @internal Check contiguous object blocks and update cookies or panic.
+ *
+ * @param mp
+ * Pointer to the memory pool.
+ * @param first_obj_table_const
+ * Pointer to a table of void * pointers (first object of the contiguous
+ * object blocks).
+ * @param n
+ * Number of contiguous object blocks.
+ * @param free
+ * - 0: object is supposed to be allocated, mark it as free
+ * - 1: object is supposed to be free, mark it as allocated
+ * - 2: just check that cookie is valid (free or allocated)
+ */
+void rte_mempool_contig_blocks_check_cookies(const struct rte_mempool *mp,
+ void * const *first_obj_table_const, unsigned int n, int free);
+
+#ifdef RTE_LIBRTE_MEMPOOL_DEBUG
+#define __mempool_contig_blocks_check_cookies(mp, first_obj_table_const, n, \
+ free) \
+ rte_mempool_contig_blocks_check_cookies(mp, first_obj_table_const, n, \
+ free)
+#else
+#define __mempool_contig_blocks_check_cookies(mp, first_obj_table_const, n, \
+ free) \
+ do {} while (0)
+#endif /* RTE_LIBRTE_MEMPOOL_DEBUG */
+
#define RTE_MEMPOOL_OPS_NAMESIZE 32 /**< Max length of ops struct name. */
/**
@@ -374,6 +421,15 @@ typedef int (*rte_mempool_dequeue_t)(struct rte_mempool *mp,
void **obj_table, unsigned int n);
/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Dequeue a number of contiquous object blocks from the external pool.
+ */
+typedef int (*rte_mempool_dequeue_contig_blocks_t)(struct rte_mempool *mp,
+ void **first_obj_table, unsigned int n);
+
+/**
* Return the number of available objects in the external pool.
*/
typedef unsigned (*rte_mempool_get_count)(const struct rte_mempool *mp);
@@ -539,6 +595,10 @@ struct rte_mempool_ops {
* Get mempool info
*/
rte_mempool_get_info_t get_info;
+ /**
+ * Dequeue a number of contiguous object blocks.
+ */
+ rte_mempool_dequeue_contig_blocks_t dequeue_contig_blocks;
} __rte_cache_aligned;
#define RTE_MEMPOOL_MAX_OPS_IDX 16 /**< Max registered ops structs */
@@ -617,6 +677,30 @@ rte_mempool_ops_dequeue_bulk(struct rte_mempool *mp,
}
/**
+ * @internal Wrapper for mempool_ops dequeue_contig_blocks callback.
+ *
+ * @param[in] mp
+ * Pointer to the memory pool.
+ * @param[out] first_obj_table
+ * Pointer to a table of void * pointers (first objects).
+ * @param[in] n
+ * Number of blocks to get.
+ * @return
+ * - 0: Success; got n objects.
+ * - <0: Error; code of dequeue function.
+ */
+static inline int
+rte_mempool_ops_dequeue_contig_blocks(struct rte_mempool *mp,
+ void **first_obj_table, unsigned int n)
+{
+ struct rte_mempool_ops *ops;
+
+ ops = rte_mempool_get_ops(mp->ops_index);
+ RTE_ASSERT(ops->dequeue_contig_blocks != NULL);
+ return ops->dequeue_contig_blocks(mp, first_obj_table, n);
+}
+
+/**
* @internal wrapper for mempool_ops enqueue callback.
*
* @param mp
@@ -1531,6 +1615,71 @@ rte_mempool_get(struct rte_mempool *mp, void **obj_p)
}
/**
+ * @internal Get contiguous blocks of objects from the pool. Used internally.
+ * @param mp
+ * A pointer to the mempool structure.
+ * @param first_obj_table
+ * A pointer to a pointer to the first object in each block.
+ * @param n
+ * A number of blocks to get.
+ * @return
+ * - >0: Success
+ * - <0: Error
+ */
+static __rte_always_inline int
+__mempool_generic_get_contig_blocks(struct rte_mempool *mp,
+ void **first_obj_table, unsigned int n)
+{
+ int ret;
+
+ ret = rte_mempool_ops_dequeue_contig_blocks(mp, first_obj_table, n);
+ if (ret < 0)
+ __MEMPOOL_CONTIG_BLOCKS_STAT_ADD(mp, get_fail, n);
+ else
+ __MEMPOOL_CONTIG_BLOCKS_STAT_ADD(mp, get_success, n);
+
+ return ret;
+}
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Get a contiguous blocks of objects from the mempool.
+ *
+ * If cache is enabled, consider to flush it first, to reuse objects
+ * as soon as possible.
+ *
+ * The application should check that the driver supports the operation
+ * by calling rte_mempool_ops_get_info() and checking that `contig_block_size`
+ * is not zero.
+ *
+ * @param mp
+ * A pointer to the mempool structure.
+ * @param first_obj_table
+ * A pointer to a pointer to the first object in each block.
+ * @param n
+ * The number of blocks to get from mempool.
+ * @return
+ * - 0: Success; blocks taken.
+ * - -ENOBUFS: Not enough entries in the mempool; no object is retrieved.
+ * - -EOPNOTSUPP: The mempool driver does not support block dequeue
+ */
+static __rte_always_inline int
+__rte_experimental
+rte_mempool_get_contig_blocks(struct rte_mempool *mp,
+ void **first_obj_table, unsigned int n)
+{
+ int ret;
+
+ ret = __mempool_generic_get_contig_blocks(mp, first_obj_table, n);
+ if (ret == 0)
+ __mempool_contig_blocks_check_cookies(mp, first_obj_table, n,
+ 1);
+ return ret;
+}
+
+/**
* Return the number of entries in the mempool.
*
* When cache is enabled, this function has to browse the length of
@@ -60,6 +60,7 @@ rte_mempool_register_ops(const struct rte_mempool_ops *h)
ops->calc_mem_size = h->calc_mem_size;
ops->populate = h->populate;
ops->get_info = h->get_info;
+ ops->dequeue_contig_blocks = h->dequeue_contig_blocks;
rte_spinlock_unlock(&rte_mempool_ops_table.sl);
@@ -53,6 +53,7 @@ DPDK_17.11 {
DPDK_18.05 {
global:
+ rte_mempool_contig_blocks_check_cookies;
rte_mempool_op_calc_mem_size_default;
rte_mempool_op_populate_default;