[PATCH v2] Fix PPC64le cache line issues #692
Conversation
POWER8 has 128-byte cache lines Signed-off-by: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org>
POWER8 has 128-byte cache lines Signed-off-by: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org>
muvarov
changed the title
Signed-off-by: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org>
muvarov
changed the title
| @@ -53,6 +53,8 @@ struct pktio_if_ops; | |||
|
|
|||
| #if defined(ODP_NETMAP) | |||
| #define PKTIO_PRIVATE_SIZE 74752 | |||
| #elif defined(ODP_PKTIO_DPDK) && ODP_CACHE_LINE_SIZE == 128 | |||
| #define PKTIO_PRIVATE_SIZE 10240 | |||
| #elif defined(ODP_PKTIO_DPDK) | |||
| #define PKTIO_PRIVATE_SIZE 5632 | |||
There was a problem hiding this comment.
Would this be better as simply a standard multiple of ODP_CACHE_LINE_SIZE? Previous code assuming cache line size of 64 bytes is 88 lines, while you're setting it to 80 lines for 128 byte cache lines. These numbers seem arbitrary anyway.
There was a problem hiding this comment.
It's a sizeof(struct) aligned up, so it was hand-calculated.
There was a problem hiding this comment.
Can we then use that sizeof() in the #define?
There was a problem hiding this comment.
maybe make unsigned char ODP_ALIGNED_CACHE pkt_priv[PKTIO_PRIVATE_SIZE]; dynamic rather then fixed len? Remove static assert from pkios and increase private date with realloc() if needed? All this ifdefs are very annoying.
There was a problem hiding this comment.
@Bill-Fischofer-Linaro No, structs are private to drivers.
@muvarov I'd like to hear about realloc for shm-allocated memory.
Another approach might be to allocate pktio-private memory from per-pktio-type pool. However I can not predict impact on the performance.
@MatiasElo @psavol any ideas?
There was a problem hiding this comment.
@lumag memory might be not shared for priv and it should work. 1) if pktio open in control thread then workers run - it's the same memory 2) with workers run and do pktio_open then they have per worker internal non share priv struct which will be cleared after close.
There was a problem hiding this comment.
It looks like the only sane option is the per-pktio private data pool.
There was a problem hiding this comment.
API does not forbid to call open several times for the same pool. odp_pktio_open("eth0", pool, NULL) and then odp_pktio_open("netmap:eth0", pool, NULL) or odp_pktio_open("tap0", pool, NULL). Not sure that I understand what "the per-pktio private data pool" means.
There was a problem hiding this comment.
Changing DPDK pktio PKTIO_PRIVATE_SIZE to 10240 doesn't seem to have any performance impact, so the new ifdef branch seems quite unnecessary.
There was a problem hiding this comment.
@MatiasElo only memory usage 10k instead of 5k multiply on ODP_CONFIG_PKTIO_ENTRIES
|
I meant a pool that will return private data areas for each PtkIO type.
It is not possible to use plain dynamic memory since an application can
access single pktio from different threads, which may have different
memory areas.
|
|
@lumag yes, agree. pktio can have internal things which has to be common for all threads. Now I think that it's better to apply current PR as it fixes real issue. And we can polish memory usage later. |
|
Merged. |
No description provided.