| Message ID | 1432222332-21289-4-git-send-email-maxim.uvarov@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Thu, May 21, 2015 at 6:32 PM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On init odp creates odp_sched_pool. Because of we can not > modify API to add new parameter to odp_pool_param_t this > pool should not be shared between different processes. To > make that add special value to shm provided to pool saying that > this pool should be in local memory only. This description is not very clear. > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > --- > platform/linux-generic/include/odp/plat/shared_memory_types.h | 4 ++++ > platform/linux-generic/odp_pool.c | 9 +++++++-- > platform/linux-generic/odp_schedule.c | 2 +- > 3 files changed, 12 insertions(+), 3 deletions(-) > > diff --git a/platform/linux-generic/include/odp/plat/shared_memory_types.h b/platform/linux-generic/include/odp/plat/shared_memory_types.h > index 4be7356..908bb2e 100644 > --- a/platform/linux-generic/include/odp/plat/shared_memory_types.h > +++ b/platform/linux-generic/include/odp/plat/shared_memory_types.h > @@ -30,6 +30,10 @@ typedef ODP_HANDLE_T(odp_shm_t); > > #define ODP_SHM_INVALID _odp_cast_scalar(odp_shm_t, 0) > #define ODP_SHM_NULL ODP_SHM_INVALID > +/** NULL shared memory but do not create IPC object for it. > + * Platfrom specific flag. > + */ > +#define _ODP_SHM_NULL_LOCAL _odp_cast_scalar(odp_shm_t, 0xffffffffULL - 1) I don't think we should have this visible to ODP applications, because it's a workaround. > > /** Get printable format of odp_shm_t */ > static inline uint64_t odp_shm_to_u64(odp_shm_t hdl) > diff --git a/platform/linux-generic/odp_pool.c b/platform/linux-generic/odp_pool.c > index cd2c449..b2f30dc 100644 > --- a/platform/linux-generic/odp_pool.c > +++ b/platform/linux-generic/odp_pool.c > @@ -151,10 +151,14 @@ odp_pool_t odp_pool_create(const char *name, > odp_pool_t pool_hdl = ODP_POOL_INVALID; > pool_entry_t *pool; > uint32_t i, headroom = 0, tailroom = 0; > + uint32_t shm_flags = 0; > > if (params == NULL) > return ODP_POOL_INVALID; > > + if (shm == ODP_SHM_NULL) > + shm_flags = ODP_SHM_PROC; > + > /* Default size and align for timeouts */ > if (params->type == ODP_POOL_TIMEOUT) { > params->buf.size = 0; /* tmo.__res1 */ > @@ -289,10 +293,11 @@ odp_pool_t odp_pool_create(const char *name, > mdata_size + > udata_size); > > - if (shm == ODP_SHM_NULL) { > + if (shm == ODP_SHM_NULL || shm == _ODP_SHM_NULL_LOCAL) { > shm = odp_shm_reserve(pool->s.name, > pool->s.pool_size, > - ODP_PAGE_SIZE, 0); > + ODP_PAGE_SIZE, > + shm_flags); > if (shm == ODP_SHM_INVALID) { > POOL_UNLOCK(&pool->s.lock); > return ODP_POOL_INVALID; > diff --git a/platform/linux-generic/odp_schedule.c b/platform/linux-generic/odp_schedule.c > index a63f97a..07422bd 100644 > --- a/platform/linux-generic/odp_schedule.c > +++ b/platform/linux-generic/odp_schedule.c > @@ -129,7 +129,7 @@ int odp_schedule_init_global(void) > params.buf.num = NUM_SCHED_CMD; > params.type = ODP_POOL_BUFFER; > > - pool = odp_pool_create("odp_sched_pool", ODP_SHM_NULL, ¶ms); > + pool = odp_pool_create("odp_sched_pool", _ODP_SHM_NULL_LOCAL, ¶ms); I think you're going about this the wrong way. ODP_SHM_NULL is a convenient way to let the implementation chose the memory where it will create buffers / packets / tmo. It was introduced because most hardware platforms don't use shared memory, since they have their own buffer management. But linux-generic can only use shared memory, and it's ok to allocate an shm here because it's inside the implementation, there is no visibility towards the application. So I think you should allocated the shm here with the right flags and remove the _ODP_SHM_NULL_LOCAL hack. > > if (pool == ODP_POOL_INVALID) { > ODP_ERR("Schedule init: Pool create failed.\n"); > -- > 1.9.1 > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp
On 05/22/15 13:25, Ciprian Barbu wrote: > On Thu, May 21, 2015 at 6:32 PM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: >> On init odp creates odp_sched_pool. Because of we can not >> modify API to add new parameter to odp_pool_param_t this >> pool should not be shared between different processes. To >> make that add special value to shm provided to pool saying that >> this pool should be in local memory only. > This description is not very clear. > >> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> >> --- >> platform/linux-generic/include/odp/plat/shared_memory_types.h | 4 ++++ >> platform/linux-generic/odp_pool.c | 9 +++++++-- >> platform/linux-generic/odp_schedule.c | 2 +- >> 3 files changed, 12 insertions(+), 3 deletions(-) >> >> diff --git a/platform/linux-generic/include/odp/plat/shared_memory_types.h b/platform/linux-generic/include/odp/plat/shared_memory_types.h >> index 4be7356..908bb2e 100644 >> --- a/platform/linux-generic/include/odp/plat/shared_memory_types.h >> +++ b/platform/linux-generic/include/odp/plat/shared_memory_types.h >> @@ -30,6 +30,10 @@ typedef ODP_HANDLE_T(odp_shm_t); >> >> #define ODP_SHM_INVALID _odp_cast_scalar(odp_shm_t, 0) >> #define ODP_SHM_NULL ODP_SHM_INVALID >> +/** NULL shared memory but do not create IPC object for it. >> + * Platfrom specific flag. >> + */ >> +#define _ODP_SHM_NULL_LOCAL _odp_cast_scalar(odp_shm_t, 0xffffffffULL - 1) > I don't think we should have this visible to ODP applications, because > it's a workaround. > >> /** Get printable format of odp_shm_t */ >> static inline uint64_t odp_shm_to_u64(odp_shm_t hdl) >> diff --git a/platform/linux-generic/odp_pool.c b/platform/linux-generic/odp_pool.c >> index cd2c449..b2f30dc 100644 >> --- a/platform/linux-generic/odp_pool.c >> +++ b/platform/linux-generic/odp_pool.c >> @@ -151,10 +151,14 @@ odp_pool_t odp_pool_create(const char *name, >> odp_pool_t pool_hdl = ODP_POOL_INVALID; >> pool_entry_t *pool; >> uint32_t i, headroom = 0, tailroom = 0; >> + uint32_t shm_flags = 0; >> >> if (params == NULL) >> return ODP_POOL_INVALID; >> >> + if (shm == ODP_SHM_NULL) >> + shm_flags = ODP_SHM_PROC; >> + >> /* Default size and align for timeouts */ >> if (params->type == ODP_POOL_TIMEOUT) { >> params->buf.size = 0; /* tmo.__res1 */ >> @@ -289,10 +293,11 @@ odp_pool_t odp_pool_create(const char *name, >> mdata_size + >> udata_size); >> >> - if (shm == ODP_SHM_NULL) { >> + if (shm == ODP_SHM_NULL || shm == _ODP_SHM_NULL_LOCAL) { >> shm = odp_shm_reserve(pool->s.name, >> pool->s.pool_size, >> - ODP_PAGE_SIZE, 0); >> + ODP_PAGE_SIZE, >> + shm_flags); >> if (shm == ODP_SHM_INVALID) { >> POOL_UNLOCK(&pool->s.lock); >> return ODP_POOL_INVALID; >> diff --git a/platform/linux-generic/odp_schedule.c b/platform/linux-generic/odp_schedule.c >> index a63f97a..07422bd 100644 >> --- a/platform/linux-generic/odp_schedule.c >> +++ b/platform/linux-generic/odp_schedule.c >> @@ -129,7 +129,7 @@ int odp_schedule_init_global(void) >> params.buf.num = NUM_SCHED_CMD; >> params.type = ODP_POOL_BUFFER; >> >> - pool = odp_pool_create("odp_sched_pool", ODP_SHM_NULL, ¶ms); >> + pool = odp_pool_create("odp_sched_pool", _ODP_SHM_NULL_LOCAL, ¶ms); > I think you're going about this the wrong way. ODP_SHM_NULL is a > convenient way to let the implementation chose the memory where it > will create buffers / packets / tmo. It was introduced because most > hardware platforms don't use shared memory, since they have their own > buffer management. But linux-generic can only use shared memory, and > it's ok to allocate an shm here because it's inside the > implementation, there is no visibility towards the application. So I > think you should allocated the shm here with the right flags and > remove the _ODP_SHM_NULL_LOCAL hack. we were planned to remove shm argument from pool_create() in future. For not it will work, but later I'm not sure what is the good solution for that. Maxim. >> if (pool == ODP_POOL_INVALID) { >> ODP_ERR("Schedule init: Pool create failed.\n"); >> -- >> 1.9.1 >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> https://lists.linaro.org/mailman/listinfo/lng-odp
On Fri, May 22, 2015 at 2:04 PM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 05/22/15 13:25, Ciprian Barbu wrote: >> >> On Thu, May 21, 2015 at 6:32 PM, Maxim Uvarov <maxim.uvarov@linaro.org> >> wrote: >>> >>> On init odp creates odp_sched_pool. Because of we can not >>> modify API to add new parameter to odp_pool_param_t this >>> pool should not be shared between different processes. To >>> make that add special value to shm provided to pool saying that >>> this pool should be in local memory only. >> >> This description is not very clear. >> >>> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> >>> --- >>> platform/linux-generic/include/odp/plat/shared_memory_types.h | 4 ++++ >>> platform/linux-generic/odp_pool.c | 9 >>> +++++++-- >>> platform/linux-generic/odp_schedule.c | 2 +- >>> 3 files changed, 12 insertions(+), 3 deletions(-) >>> >>> diff --git >>> a/platform/linux-generic/include/odp/plat/shared_memory_types.h >>> b/platform/linux-generic/include/odp/plat/shared_memory_types.h >>> index 4be7356..908bb2e 100644 >>> --- a/platform/linux-generic/include/odp/plat/shared_memory_types.h >>> +++ b/platform/linux-generic/include/odp/plat/shared_memory_types.h >>> @@ -30,6 +30,10 @@ typedef ODP_HANDLE_T(odp_shm_t); >>> >>> #define ODP_SHM_INVALID _odp_cast_scalar(odp_shm_t, 0) >>> #define ODP_SHM_NULL ODP_SHM_INVALID >>> +/** NULL shared memory but do not create IPC object for it. >>> + * Platfrom specific flag. >>> + */ >>> +#define _ODP_SHM_NULL_LOCAL _odp_cast_scalar(odp_shm_t, 0xffffffffULL - >>> 1) >> >> I don't think we should have this visible to ODP applications, because >> it's a workaround. >> >>> /** Get printable format of odp_shm_t */ >>> static inline uint64_t odp_shm_to_u64(odp_shm_t hdl) >>> diff --git a/platform/linux-generic/odp_pool.c >>> b/platform/linux-generic/odp_pool.c >>> index cd2c449..b2f30dc 100644 >>> --- a/platform/linux-generic/odp_pool.c >>> +++ b/platform/linux-generic/odp_pool.c >>> @@ -151,10 +151,14 @@ odp_pool_t odp_pool_create(const char *name, >>> odp_pool_t pool_hdl = ODP_POOL_INVALID; >>> pool_entry_t *pool; >>> uint32_t i, headroom = 0, tailroom = 0; >>> + uint32_t shm_flags = 0; >>> >>> if (params == NULL) >>> return ODP_POOL_INVALID; >>> >>> + if (shm == ODP_SHM_NULL) >>> + shm_flags = ODP_SHM_PROC; >>> + >>> /* Default size and align for timeouts */ >>> if (params->type == ODP_POOL_TIMEOUT) { >>> params->buf.size = 0; /* tmo.__res1 */ >>> @@ -289,10 +293,11 @@ odp_pool_t odp_pool_create(const char *name, >>> mdata_size + >>> udata_size); >>> >>> - if (shm == ODP_SHM_NULL) { >>> + if (shm == ODP_SHM_NULL || shm == _ODP_SHM_NULL_LOCAL) { >>> shm = odp_shm_reserve(pool->s.name, >>> pool->s.pool_size, >>> - ODP_PAGE_SIZE, 0); >>> + ODP_PAGE_SIZE, >>> + shm_flags); >>> if (shm == ODP_SHM_INVALID) { >>> POOL_UNLOCK(&pool->s.lock); >>> return ODP_POOL_INVALID; >>> diff --git a/platform/linux-generic/odp_schedule.c >>> b/platform/linux-generic/odp_schedule.c >>> index a63f97a..07422bd 100644 >>> --- a/platform/linux-generic/odp_schedule.c >>> +++ b/platform/linux-generic/odp_schedule.c >>> @@ -129,7 +129,7 @@ int odp_schedule_init_global(void) >>> params.buf.num = NUM_SCHED_CMD; >>> params.type = ODP_POOL_BUFFER; >>> >>> - pool = odp_pool_create("odp_sched_pool", ODP_SHM_NULL, ¶ms); >>> + pool = odp_pool_create("odp_sched_pool", _ODP_SHM_NULL_LOCAL, >>> ¶ms); >> >> I think you're going about this the wrong way. ODP_SHM_NULL is a >> convenient way to let the implementation chose the memory where it >> will create buffers / packets / tmo. It was introduced because most >> hardware platforms don't use shared memory, since they have their own >> buffer management. But linux-generic can only use shared memory, and >> it's ok to allocate an shm here because it's inside the >> implementation, there is no visibility towards the application. So I >> think you should allocated the shm here with the right flags and >> remove the _ODP_SHM_NULL_LOCAL hack. > > > we were planned to remove shm argument from pool_create() in future. For not > it will work, but later > I'm not sure what is the good solution for that. So why try to workaround it when we haven't even decided when to remove the shm param from odp_pktio_open? > > Maxim. > > >>> if (pool == ODP_POOL_INVALID) { >>> ODP_ERR("Schedule init: Pool create failed.\n"); >>> -- >>> 1.9.1 >>> >>> _______________________________________________ >>> lng-odp mailing list >>> lng-odp@lists.linaro.org >>> https://lists.linaro.org/mailman/listinfo/lng-odp > >
On 05/22/15 14:09, Ciprian Barbu wrote: > On Fri, May 22, 2015 at 2:04 PM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: >> On 05/22/15 13:25, Ciprian Barbu wrote: >>> On Thu, May 21, 2015 at 6:32 PM, Maxim Uvarov <maxim.uvarov@linaro.org> >>> wrote: >>>> On init odp creates odp_sched_pool. Because of we can not >>>> modify API to add new parameter to odp_pool_param_t this >>>> pool should not be shared between different processes. To >>>> make that add special value to shm provided to pool saying that >>>> this pool should be in local memory only. >>> This description is not very clear. >>> >>>> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> >>>> --- >>>> platform/linux-generic/include/odp/plat/shared_memory_types.h | 4 ++++ >>>> platform/linux-generic/odp_pool.c | 9 >>>> +++++++-- >>>> platform/linux-generic/odp_schedule.c | 2 +- >>>> 3 files changed, 12 insertions(+), 3 deletions(-) >>>> >>>> diff --git >>>> a/platform/linux-generic/include/odp/plat/shared_memory_types.h >>>> b/platform/linux-generic/include/odp/plat/shared_memory_types.h >>>> index 4be7356..908bb2e 100644 >>>> --- a/platform/linux-generic/include/odp/plat/shared_memory_types.h >>>> +++ b/platform/linux-generic/include/odp/plat/shared_memory_types.h >>>> @@ -30,6 +30,10 @@ typedef ODP_HANDLE_T(odp_shm_t); >>>> >>>> #define ODP_SHM_INVALID _odp_cast_scalar(odp_shm_t, 0) >>>> #define ODP_SHM_NULL ODP_SHM_INVALID >>>> +/** NULL shared memory but do not create IPC object for it. >>>> + * Platfrom specific flag. >>>> + */ >>>> +#define _ODP_SHM_NULL_LOCAL _odp_cast_scalar(odp_shm_t, 0xffffffffULL - >>>> 1) >>> I don't think we should have this visible to ODP applications, because >>> it's a workaround. >>> >>>> /** Get printable format of odp_shm_t */ >>>> static inline uint64_t odp_shm_to_u64(odp_shm_t hdl) >>>> diff --git a/platform/linux-generic/odp_pool.c >>>> b/platform/linux-generic/odp_pool.c >>>> index cd2c449..b2f30dc 100644 >>>> --- a/platform/linux-generic/odp_pool.c >>>> +++ b/platform/linux-generic/odp_pool.c >>>> @@ -151,10 +151,14 @@ odp_pool_t odp_pool_create(const char *name, >>>> odp_pool_t pool_hdl = ODP_POOL_INVALID; >>>> pool_entry_t *pool; >>>> uint32_t i, headroom = 0, tailroom = 0; >>>> + uint32_t shm_flags = 0; >>>> >>>> if (params == NULL) >>>> return ODP_POOL_INVALID; >>>> >>>> + if (shm == ODP_SHM_NULL) >>>> + shm_flags = ODP_SHM_PROC; >>>> + >>>> /* Default size and align for timeouts */ >>>> if (params->type == ODP_POOL_TIMEOUT) { >>>> params->buf.size = 0; /* tmo.__res1 */ >>>> @@ -289,10 +293,11 @@ odp_pool_t odp_pool_create(const char *name, >>>> mdata_size + >>>> udata_size); >>>> >>>> - if (shm == ODP_SHM_NULL) { >>>> + if (shm == ODP_SHM_NULL || shm == _ODP_SHM_NULL_LOCAL) { >>>> shm = odp_shm_reserve(pool->s.name, >>>> pool->s.pool_size, >>>> - ODP_PAGE_SIZE, 0); >>>> + ODP_PAGE_SIZE, >>>> + shm_flags); >>>> if (shm == ODP_SHM_INVALID) { >>>> POOL_UNLOCK(&pool->s.lock); >>>> return ODP_POOL_INVALID; >>>> diff --git a/platform/linux-generic/odp_schedule.c >>>> b/platform/linux-generic/odp_schedule.c >>>> index a63f97a..07422bd 100644 >>>> --- a/platform/linux-generic/odp_schedule.c >>>> +++ b/platform/linux-generic/odp_schedule.c >>>> @@ -129,7 +129,7 @@ int odp_schedule_init_global(void) >>>> params.buf.num = NUM_SCHED_CMD; >>>> params.type = ODP_POOL_BUFFER; >>>> >>>> - pool = odp_pool_create("odp_sched_pool", ODP_SHM_NULL, ¶ms); >>>> + pool = odp_pool_create("odp_sched_pool", _ODP_SHM_NULL_LOCAL, >>>> ¶ms); >>> I think you're going about this the wrong way. ODP_SHM_NULL is a >>> convenient way to let the implementation chose the memory where it >>> will create buffers / packets / tmo. It was introduced because most >>> hardware platforms don't use shared memory, since they have their own >>> buffer management. But linux-generic can only use shared memory, and >>> it's ok to allocate an shm here because it's inside the >>> implementation, there is no visibility towards the application. So I >>> think you should allocated the shm here with the right flags and >>> remove the _ODP_SHM_NULL_LOCAL hack. >> >> we were planned to remove shm argument from pool_create() in future. For not >> it will work, but later >> I'm not sure what is the good solution for that. > So why try to workaround it when we haven't even decided when to > remove the shm param from odp_pktio_open? From odp_pool_create(). But it's good suggestion I will do it. Maxim. >> Maxim. >> >> >>>> if (pool == ODP_POOL_INVALID) { >>>> ODP_ERR("Schedule init: Pool create failed.\n"); >>>> -- >>>> 1.9.1 >>>> >>>> _______________________________________________ >>>> lng-odp mailing list >>>> lng-odp@lists.linaro.org >>>> https://lists.linaro.org/mailman/listinfo/lng-odp >>
diff --git a/platform/linux-generic/include/odp/plat/shared_memory_types.h b/platform/linux-generic/include/odp/plat/shared_memory_types.h index 4be7356..908bb2e 100644 --- a/platform/linux-generic/include/odp/plat/shared_memory_types.h +++ b/platform/linux-generic/include/odp/plat/shared_memory_types.h @@ -30,6 +30,10 @@ typedef ODP_HANDLE_T(odp_shm_t); #define ODP_SHM_INVALID _odp_cast_scalar(odp_shm_t, 0) #define ODP_SHM_NULL ODP_SHM_INVALID +/** NULL shared memory but do not create IPC object for it. + * Platfrom specific flag. + */ +#define _ODP_SHM_NULL_LOCAL _odp_cast_scalar(odp_shm_t, 0xffffffffULL - 1) /** Get printable format of odp_shm_t */ static inline uint64_t odp_shm_to_u64(odp_shm_t hdl) diff --git a/platform/linux-generic/odp_pool.c b/platform/linux-generic/odp_pool.c index cd2c449..b2f30dc 100644 --- a/platform/linux-generic/odp_pool.c +++ b/platform/linux-generic/odp_pool.c @@ -151,10 +151,14 @@ odp_pool_t odp_pool_create(const char *name, odp_pool_t pool_hdl = ODP_POOL_INVALID; pool_entry_t *pool; uint32_t i, headroom = 0, tailroom = 0; + uint32_t shm_flags = 0; if (params == NULL) return ODP_POOL_INVALID; + if (shm == ODP_SHM_NULL) + shm_flags = ODP_SHM_PROC; + /* Default size and align for timeouts */ if (params->type == ODP_POOL_TIMEOUT) { params->buf.size = 0; /* tmo.__res1 */ @@ -289,10 +293,11 @@ odp_pool_t odp_pool_create(const char *name, mdata_size + udata_size); - if (shm == ODP_SHM_NULL) { + if (shm == ODP_SHM_NULL || shm == _ODP_SHM_NULL_LOCAL) { shm = odp_shm_reserve(pool->s.name, pool->s.pool_size, - ODP_PAGE_SIZE, 0); + ODP_PAGE_SIZE, + shm_flags); if (shm == ODP_SHM_INVALID) { POOL_UNLOCK(&pool->s.lock); return ODP_POOL_INVALID; diff --git a/platform/linux-generic/odp_schedule.c b/platform/linux-generic/odp_schedule.c index a63f97a..07422bd 100644 --- a/platform/linux-generic/odp_schedule.c +++ b/platform/linux-generic/odp_schedule.c @@ -129,7 +129,7 @@ int odp_schedule_init_global(void) params.buf.num = NUM_SCHED_CMD; params.type = ODP_POOL_BUFFER; - pool = odp_pool_create("odp_sched_pool", ODP_SHM_NULL, ¶ms); + pool = odp_pool_create("odp_sched_pool", _ODP_SHM_NULL_LOCAL, ¶ms); if (pool == ODP_POOL_INVALID) { ODP_ERR("Schedule init: Pool create failed.\n");
On init odp creates odp_sched_pool. Because of we can not modify API to add new parameter to odp_pool_param_t this pool should not be shared between different processes. To make that add special value to shm provided to pool saying that this pool should be in local memory only. Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> --- platform/linux-generic/include/odp/plat/shared_memory_types.h | 4 ++++ platform/linux-generic/odp_pool.c | 9 +++++++-- platform/linux-generic/odp_schedule.c | 2 +- 3 files changed, 12 insertions(+), 3 deletions(-)