[dpdk-dev] [PATCH v3 3/3] mempool: allow for user-owned mempool caches
Hi,
On 06/18/2016 06:15 PM, Lazaros Koromilas wrote:
>> What do you think of having:
>>
>> #define LOG_ERR() do { \
>> printf("test failed at %s():%d\n", __func__, __LINE__); \
>> } while (0)
>> #define RET_ERR() do { LOG_ERR(); return -1; } while (0)
>> #define GOTO_ERR() do { LOG_ERR(); ret = -1; goto out; } while (0)
>>
>> Then use GOTO_ERR() when appropriate. It would also factorize
>> the printf.
>
> The downside of GOTO_ERR() is that it assumes a variable and a label
> name. And you may need to have multiple labels 'out0', 'out1', etc for
> the error path. How about:
>
> #define GOTO_ERR(ret, out) do { LOG_ERR(); ret = -1; goto out; } while (0)
Yep, looks better indeed.
Thanks,
Olivier
[dpdk-dev] [PATCH v3 3/3] mempool: allow for user-owned mempool caches
On Fri, Jun 17, 2016 at 11:37 AM, Olivier Matz
wrote:
>
>
> On 06/16/2016 01:02 PM, Lazaros Koromilas wrote:
>> The mempool cache is only available to EAL threads as a per-lcore
>> resource. Change this so that the user can create and provide their own
>> cache on mempool get and put operations. This works with non-EAL threads
>> too. This commit introduces the new API calls:
>>
>> rte_mempool_cache_create(size, socket_id)
>> rte_mempool_cache_free(cache)
>> rte_mempool_cache_flush(cache, mp)
>> rte_mempool_default_cache(mp, lcore_id)
>
> These new functions should be added in the .map file, else it will
> break the compilation in with shared_lib=y.
Oops, thanks!
>> Changes the API calls:
>>
>> rte_mempool_generic_put(mp, obj_table, n, cache, flags)
>> rte_mempool_generic_get(mp, obj_table, n, cache, flags)
>>
>> The cache-oblivious API calls use the per-lcore default local cache.
>>
>> Signed-off-by: Lazaros Koromilas
>> ---
>> app/test/test_mempool.c | 94 --
>> app/test/test_mempool_perf.c | 70 ++---
>> lib/librte_mempool/rte_mempool.c | 66 +++-
>> lib/librte_mempool/rte_mempool.h | 163
>> ---
>> 4 files changed, 310 insertions(+), 83 deletions(-)
>>
>> diff --git a/app/test/test_mempool.c b/app/test/test_mempool.c
>> index 10d706f..723cd39 100644
>> --- a/app/test/test_mempool.c
>> +++ b/app/test/test_mempool.c
>> @@ -79,6 +79,9 @@
>> printf("test failed at %s():%d\n", __func__, __LINE__); \
>> return -1; \
>> } while (0)
>> +#define LOG_ERR() do {
>> \
>> + printf("test failed at %s():%d\n", __func__, __LINE__); \
>> + } while (0)
>>
>
> I see that the usage of this macro is always like this:
>
> LOG_ERR();
> ret = -1;
> goto out;
>
> What do you think of having:
>
> #define LOG_ERR() do { \
> printf("test failed at %s():%d\n", __func__, __LINE__); \
> } while (0)
> #define RET_ERR() do { LOG_ERR(); return -1; } while (0)
> #define GOTO_ERR() do { LOG_ERR(); ret = -1; goto out; } while (0)
>
> Then use GOTO_ERR() when appropriate. It would also factorize
> the printf.
The downside of GOTO_ERR() is that it assumes a variable and a label
name. And you may need to have multiple labels 'out0', 'out1', etc for
the error path. How about:
#define GOTO_ERR(ret, out) do { LOG_ERR(); ret = -1; goto out; } while (0)
Lazaros.
[dpdk-dev] [PATCH v3 3/3] mempool: allow for user-owned mempool caches
On 06/16/2016 01:02 PM, Lazaros Koromilas wrote:
> The mempool cache is only available to EAL threads as a per-lcore
> resource. Change this so that the user can create and provide their own
> cache on mempool get and put operations. This works with non-EAL threads
> too. This commit introduces the new API calls:
>
> rte_mempool_cache_create(size, socket_id)
> rte_mempool_cache_free(cache)
> rte_mempool_cache_flush(cache, mp)
> rte_mempool_default_cache(mp, lcore_id)
These new functions should be added in the .map file, else it will
break the compilation in with shared_lib=y.
> Changes the API calls:
>
> rte_mempool_generic_put(mp, obj_table, n, cache, flags)
> rte_mempool_generic_get(mp, obj_table, n, cache, flags)
>
> The cache-oblivious API calls use the per-lcore default local cache.
>
> Signed-off-by: Lazaros Koromilas
> ---
> app/test/test_mempool.c | 94 --
> app/test/test_mempool_perf.c | 70 ++---
> lib/librte_mempool/rte_mempool.c | 66 +++-
> lib/librte_mempool/rte_mempool.h | 163
> ---
> 4 files changed, 310 insertions(+), 83 deletions(-)
>
> diff --git a/app/test/test_mempool.c b/app/test/test_mempool.c
> index 10d706f..723cd39 100644
> --- a/app/test/test_mempool.c
> +++ b/app/test/test_mempool.c
> @@ -79,6 +79,9 @@
> printf("test failed at %s():%d\n", __func__, __LINE__); \
> return -1; \
> } while (0)
> +#define LOG_ERR() do {
> \
> + printf("test failed at %s():%d\n", __func__, __LINE__); \
> + } while (0)
>
I see that the usage of this macro is always like this:
LOG_ERR();
ret = -1;
goto out;
What do you think of having:
#define LOG_ERR() do { \
printf("test failed at %s():%d\n", __func__, __LINE__); \
} while (0)
#define RET_ERR() do { LOG_ERR(); return -1; } while (0)
#define GOTO_ERR() do { LOG_ERR(); ret = -1; goto out; } while (0)
Then use GOTO_ERR() when appropriate. It would also factorize
the printf.
[dpdk-dev] [PATCH v3 3/3] mempool: allow for user-owned mempool caches
The mempool cache is only available to EAL threads as a per-lcore
resource. Change this so that the user can create and provide their own
cache on mempool get and put operations. This works with non-EAL threads
too. This commit introduces the new API calls:
rte_mempool_cache_create(size, socket_id)
rte_mempool_cache_free(cache)
rte_mempool_cache_flush(cache, mp)
rte_mempool_default_cache(mp, lcore_id)
Changes the API calls:
rte_mempool_generic_put(mp, obj_table, n, cache, flags)
rte_mempool_generic_get(mp, obj_table, n, cache, flags)
The cache-oblivious API calls use the per-lcore default local cache.
Signed-off-by: Lazaros Koromilas
---
app/test/test_mempool.c | 94 --
app/test/test_mempool_perf.c | 70 ++---
lib/librte_mempool/rte_mempool.c | 66 +++-
lib/librte_mempool/rte_mempool.h | 163 ---
4 files changed, 310 insertions(+), 83 deletions(-)
diff --git a/app/test/test_mempool.c b/app/test/test_mempool.c
index 10d706f..723cd39 100644
--- a/app/test/test_mempool.c
+++ b/app/test/test_mempool.c
@@ -79,6 +79,9 @@
printf("test failed at %s():%d\n", __func__, __LINE__); \
return -1; \
} while (0)
+#define LOG_ERR() do { \
+ printf("test failed at %s():%d\n", __func__, __LINE__); \
+ } while (0)
static rte_atomic32_t synchro;
@@ -191,7 +194,7 @@ my_obj_init(struct rte_mempool *mp, __attribute__((unused))
void *arg,
/* basic tests (done on one core) */
static int
-test_mempool_basic(struct rte_mempool *mp)
+test_mempool_basic(struct rte_mempool *mp, int use_external_cache)
{
uint32_t *objnum;
void **objtable;
@@ -199,47 +202,79 @@ test_mempool_basic(struct rte_mempool *mp)
char *obj_data;
int ret = 0;
unsigned i, j;
+ int offset;
+ struct rte_mempool_cache *cache;
+
+ if (use_external_cache) {
+ /* Create a user-owned mempool cache. */
+ cache = rte_mempool_cache_create(RTE_MEMPOOL_CACHE_MAX_SIZE,
+SOCKET_ID_ANY);
+ if (cache == NULL)
+ RET_ERR();
+ } else {
+ /* May be NULL if cache is disabled. */
+ cache = rte_mempool_default_cache(mp, rte_lcore_id());
+ }
/* dump the mempool status */
rte_mempool_dump(stdout, mp);
printf("get an object\n");
- if (rte_mempool_get(mp, &obj) < 0)
- RET_ERR();
+ if (rte_mempool_generic_get(mp, &obj, 1, cache, 0) < 0) {
+ LOG_ERR();
+ ret = -1;
+ goto out;
+ }
rte_mempool_dump(stdout, mp);
/* tests that improve coverage */
printf("get object count\n");
- if (rte_mempool_count(mp) != MEMPOOL_SIZE - 1)
- RET_ERR();
+ /* We have to count the extra caches, one in this case. */
+ offset = use_external_cache ? 1 * cache->len : 0;
+ if (rte_mempool_count(mp) + offset != MEMPOOL_SIZE - 1) {
+ LOG_ERR();
+ ret = -1;
+ goto out;
+ }
printf("get private data\n");
if (rte_mempool_get_priv(mp) != (char *)mp +
- MEMPOOL_HEADER_SIZE(mp, mp->cache_size))
- RET_ERR();
+ MEMPOOL_HEADER_SIZE(mp, mp->cache_size)) {
+ LOG_ERR();
+ ret = -1;
+ goto out;
+ }
#ifndef RTE_EXEC_ENV_BSDAPP /* rte_mem_virt2phy() not supported on bsd */
printf("get physical address of an object\n");
- if (rte_mempool_virt2phy(mp, obj) != rte_mem_virt2phy(obj))
- RET_ERR();
+ if (rte_mempool_virt2phy(mp, obj) != rte_mem_virt2phy(obj)) {
+ LOG_ERR();
+ ret = -1;
+ goto out;
+ }
#endif
printf("put the object back\n");
- rte_mempool_put(mp, obj);
+ rte_mempool_generic_put(mp, &obj, 1, cache, 0);
rte_mempool_dump(stdout, mp);
printf("get 2 objects\n");
- if (rte_mempool_get(mp, &obj) < 0)
- RET_ERR();
- if (rte_mempool_get(mp, &obj2) < 0) {
- rte_mempool_put(mp, obj);
- RET_ERR();
+ if (rte_mempool_generic_get(mp, &obj, 1, cache, 0) < 0) {
+ LOG_ERR();
+ ret = -1;
+ goto out;
+ }
+ if (rte_mempool_generic_get(mp, &obj2, 1, cache, 0) < 0) {
+ rte_mempool_generic_put(mp, &obj, 1, cache, 0);
+ LOG_ERR();
+ ret = -1;
+ goto out;
}
rte_mempool_dump(stdout, mp);
printf("put the objects back\n");
- rte_mempool_put(mp, obj);
- rte_mempool_put(mp, obj2);
+ rte_mempool_generic_put(mp, &obj
