On 13 May 2015 at 05:03, Maxim Uvarov <[email protected]> wrote:
> On 05/12/2015 20:52, Mike Holmes wrote: > >> I dont think you can implement a mandatory ODP API based on an optional >> helper api. >> >> The ring implementation should be inside the linux-generic implementation >> if you want to use it, other implementations can copy it as they do now >> from linux-generic for other features such as timers. >> >> Helpers need to be supporting the tests, examples and applications and >> not the implementations. >> >> We have been walking a fuzzy line previously but I think the complexities >> and coupling get unwound when linux-generic does not rely on or compile in >> any helper code. >> >> Mike >> > > I think it depends if someone else uses odp ring from helper or not. Ring > itself might be useful as middle buffer between processes. Also I just > added shm flag which is in our api to have smaller changes. I think later > we can decided where it's better to move odp ring. > I dont agree that we can decide later. The test infrastructure that we decided to punt on to get 1.0 done is proving very difficult to fix now that it has established a pattern of use that is coupled to the platform is awkward ways, lets not do the same with helpers. Basing an implementation on optional code is not very robust, the makefile looks wrong with ../../ (below), we have made helpers not optional in this way. We compile in the now not optional ring.c and we dont test it or use it in any way at all! that can't be correct. __LIB__libodp_la_SOURCES = \ >------->------->------- odp_barrier.c \ >------->------->------- odp_buffer.c \ >------->------->------- odp_classification.c \ >------->------->------- odp_cpumask.c \ >------->------->------- odp_crypto.c \ >------->------->------- odp_errno.c \ >------->------->------- odp_event.c \ >------->------->------- odp_init.c \ >------->------->------- odp_impl.c \ >------->------->------- ../../helper/linux.c \ >------->------->------- odp_packet.c \ >------->------->------- odp_packet_flags.c \ >------->------->------- odp_packet_io.c \ >------->------->------- odp_packet_socket.c \ >------->------->------- odp_pool.c \ >------->------->------- odp_queue.c \ >------->------->------- ../../helper/ring.c \ To share implementations, thus far KS2 and DPDK have their make file include the src from linux-generic as appropriate, so they will reuse ring & IPC from there and all re-use is then uniformly handled. Let start by being clear what the helpers are before we start coupling them even more tightly with IPC. Previously stated and presumed still true is this description "Helpers are there to help tests, examples and applications, not implementations. Helpers are optional for applications" we need to establish this as fact before deciding much more. If helpers are not separate support code from the implementation then we need to state that all implementations should compile them all in so that customers of vendor implementations can run the validation tests. Mike > > Maxim. > > >> On 8 May 2015 at 05:57, Maxim Uvarov <[email protected] <mailto: >> [email protected]>> wrote: >> >> Signed-off-by: Maxim Uvarov <[email protected] >> <mailto:[email protected]>> >> >> --- >> helper/include/odp/helper/ring.h | 2 ++ >> helper/ring.c | 9 ++++++++- >> 2 files changed, 10 insertions(+), 1 deletion(-) >> >> diff --git a/helper/include/odp/helper/ring.h >> b/helper/include/odp/helper/ring.h >> index 65c32ad..5e640a7 100644 >> --- a/helper/include/odp/helper/ring.h >> +++ b/helper/include/odp/helper/ring.h >> @@ -158,6 +158,8 @@ typedef struct odph_ring { >> >> #define ODPH_RING_F_SP_ENQ 0x0001 /* The default enqueue is >> "single-producer".*/ >> #define ODPH_RING_F_SC_DEQ 0x0002 /* The default dequeue is >> "single-consumer".*/ >> +#define ODPH_RING_SHM_PROC 0x0004 /* If set - ring is visible >> from different >> + processes. Default is thread >> visible. */ >> #define ODPH_RING_QUOT_EXCEED (1 << 31) /* Quota exceed for >> burst ops */ >> #define ODPH_RING_SZ_MASK (unsigned)(0x0fffffff) /* Ring size >> mask */ >> >> diff --git a/helper/ring.c b/helper/ring.c >> index a24a020..0927a6c 100644 >> --- a/helper/ring.c >> +++ b/helper/ring.c >> @@ -159,8 +159,14 @@ odph_ring_create(const char *name, unsigned >> count, unsigned flags) >> char ring_name[ODPH_RING_NAMESIZE]; >> odph_ring_t *r; >> size_t ring_size; >> + uint32_t shm_flag; >> odp_shm_t shm; >> >> + if (flags & ODPH_RING_SHM_PROC) >> + shm_flag = ODP_SHM_PROC; >> + else >> + shm_flag = 0; >> + >> /* count must be a power of 2 */ >> if (!ODP_VAL_IS_POWER_2(count) || (count > >> ODPH_RING_SZ_MASK)) { >> ODP_ERR("Requested size is invalid, must be power >> of 2, and do not exceed the size limit %u\n", >> @@ -173,7 +179,8 @@ odph_ring_create(const char *name, unsigned >> count, unsigned flags) >> >> odp_rwlock_write_lock(&qlock); >> /* reserve a memory zone for this ring.*/ >> - shm = odp_shm_reserve(ring_name, ring_size, >> ODP_CACHE_LINE_SIZE, 0); >> + shm = odp_shm_reserve(ring_name, ring_size, >> ODP_CACHE_LINE_SIZE, >> + shm_flag); >> >> r = odp_shm_addr(shm); >> >> -- >> 1.9.1 >> >> _______________________________________________ >> lng-odp mailing list >> [email protected] <mailto:[email protected]> >> https://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> >> >> -- >> Mike Holmes >> Technical Manager - Linaro Networking Group >> Linaro.org <http://www.linaro.org/>***│ *Open source software for ARM >> SoCs >> >> > -- Mike Holmes Technical Manager - Linaro Networking Group Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs
_______________________________________________ lng-odp mailing list [email protected] https://lists.linaro.org/mailman/listinfo/lng-odp
