Re: [lng-odp] ODP1.15 buffer alignment issue
Hi, See comments inline Regards, Liron -Original Message- From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of Maxim Uvarov Sent: Monday, August 07, 2017 23:21 To: lng-odp@lists.linaro.org Subject: Re: [lng-odp] ODP1.15 buffer alignment issue it's implementation specific. Full code is: + offset = pool->headroom; + + /* move to correct align */ + while (((uintptr_t)&data[offset]) % pool->align != 0) + offset++; + + /* Pointer to data start (of the first segment) */ + buf_hdr->addr[0] = &data[offset]; + /* Store buffer into the global pool */ + ring_enq(ring, mask, (uint32_t)(uintptr_t)buf_hdl); If I understood idea right there should be odp specific packet header with odp fields which is needed to implement api but missing in hardware buffer field. Then hw buffer which is aligned by default with pointer to data. Everything depends on implementation and it's not mandatory. [L.H.] As part of the linux-generic there is a 'ODP_CONFIG_BUFFER_ALIGN_MIN' which should be used for the buffer alignment. However, the code above use this value for the data section alignment in the buffer and not the buffer address alignment. On ODP1.11 the 'ODP_CONFIG_BUFFER_ALIGN_MIN' was used correctly for buffer alignment. In order to continue we need to understand what was the real intention here: 1. To make sure the data section is aligned? 2. To make sure the buffer address is aligned? If so, there is a bug in this code. A possible fix is as follow: - offset = pool->headroom; + offset = 0; /* move to correct align */ while (((uintptr_t)&data[offset]) % pool->align != 0) offset++; + offset += pool->headroom /* Pointer to data start (of the first segment) */ buf_hdr->addr[0] = &data[offset]; If 1# is required than maybe a 'ODP_CONFIG_DATA_ALIGN_MIN' should be define. If your platform reuses this linux-generic file and that alignment does not work for you then we rework it somehow. Bet regards. Maxim. On 08/07/17 21:58, Bill Fischofer wrote: > This is part of the latest pool restructure code contributed by Nokia. > If you'd like to join the ODP public call tomorrow at 15:00 UTC we can > discuss this then. > > On Mon, Aug 7, 2017 at 8:57 AM, Liron Himi wrote: > >> Hi, >> >> I'm trying to move to odp1.15 and encounter with an buffer alignment issue. >> It seems that linux-generic implementation make sure that the data >> address is align with the requested alignment and not the buffer >> address itself (look at the code sniped below). >> On ODP1.11 (the current version we use) the buffer was aligned correctly. >> >> Is the current behavior is the expected one? >> Our HW (and probably other HWs) rely on the fact that the buffers are >> aligned. >> >> From odp_pool.c, init_buffers: >> >> offset = pool->headroom; >> >> /* move to correct align */ >> while (((uintptr_t)&data[offset]) % >> pool->align != 0) >> offset++; >> >> >> Regards, >> Liron >>
Re: [lng-odp] ODP1.15 buffer alignment issue
it's implementation specific. Full code is: + offset = pool->headroom; + + /* move to correct align */ + while (((uintptr_t)&data[offset]) % pool->align != 0) + offset++; + + /* Pointer to data start (of the first segment) */ + buf_hdr->addr[0] = &data[offset]; + /* Store buffer into the global pool */ + ring_enq(ring, mask, (uint32_t)(uintptr_t)buf_hdl); If I understood idea right there should be odp specific packet header with odp fields which is needed to implement api but missing in hardware buffer field. Then hw buffer which is aligned by default with pointer to data. Everything depends on implementation and it's not mandatory. If your platform reuses this linux-generic file and that alignment does not work for you then we rework it somehow. Bet regards. Maxim. On 08/07/17 21:58, Bill Fischofer wrote: > This is part of the latest pool restructure code contributed by Nokia. If > you'd like to join the ODP public call tomorrow at 15:00 UTC we can discuss > this then. > > On Mon, Aug 7, 2017 at 8:57 AM, Liron Himi wrote: > >> Hi, >> >> I'm trying to move to odp1.15 and encounter with an buffer alignment issue. >> It seems that linux-generic implementation make sure that the data address >> is align with the requested alignment and not the buffer address itself >> (look at the code sniped below). >> On ODP1.11 (the current version we use) the buffer was aligned correctly. >> >> Is the current behavior is the expected one? >> Our HW (and probably other HWs) rely on the fact that the buffers are >> aligned. >> >> From odp_pool.c, init_buffers: >> >> offset = pool->headroom; >> >> /* move to correct align */ >> while (((uintptr_t)&data[offset]) % >> pool->align != 0) >> offset++; >> >> >> Regards, >> Liron >>
Re: [lng-odp] ODP1.15 buffer alignment issue
This is part of the latest pool restructure code contributed by Nokia. If you'd like to join the ODP public call tomorrow at 15:00 UTC we can discuss this then. On Mon, Aug 7, 2017 at 8:57 AM, Liron Himi wrote: > Hi, > > I'm trying to move to odp1.15 and encounter with an buffer alignment issue. > It seems that linux-generic implementation make sure that the data address > is align with the requested alignment and not the buffer address itself > (look at the code sniped below). > On ODP1.11 (the current version we use) the buffer was aligned correctly. > > Is the current behavior is the expected one? > Our HW (and probably other HWs) rely on the fact that the buffers are > aligned. > > From odp_pool.c, init_buffers: > > offset = pool->headroom; > > /* move to correct align */ > while (((uintptr_t)&data[offset]) % > pool->align != 0) > offset++; > > > Regards, > Liron >
[lng-odp] [PATCH API-NEXT v2 2/2] linux-gen: crypto: unify odp_crypto_session_create error path
From: Dmitry Eremin-Solenikov
Make sure that or error session is always returned as
ODP_CRYPTO_SESSION_INVALID.
Signed-off-by: Dmitry Eremin-Solenikov
---
/** Email created from pull request 95 (lumag:crypto-session-create)
** https://github.com/Linaro/odp/pull/95
** Patch: https://github.com/Linaro/odp/pull/95.patch
** Base sha: 2394317ab247fa14b4e239aec512afee7eac4524
** Merge commit sha: bc2a40cc94ced5ba8ec7b86fbdef99958ed37009
**/
platform/linux-generic/odp_crypto.c | 23 +--
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/platform/linux-generic/odp_crypto.c
b/platform/linux-generic/odp_crypto.c
index ca51bdf0..1f49261a 100644
--- a/platform/linux-generic/odp_crypto.c
+++ b/platform/linux-generic/odp_crypto.c
@@ -697,14 +697,11 @@ odp_crypto_session_create(odp_crypto_session_param_t
*param,
odp_crypto_generic_session_t *session;
int aes_gcm = 0;
- /* Default to successful result */
- *status = ODP_CRYPTO_SES_CREATE_ERR_NONE;
-
/* Allocate memory for this session */
session = alloc_session();
if (NULL == session) {
*status = ODP_CRYPTO_SES_CREATE_ERR_ENOMEM;
- return -1;
+ goto err;
}
/* Copy parameters */
@@ -712,8 +709,8 @@ odp_crypto_session_create(odp_crypto_session_param_t *param,
if (session->p.iv.length > MAX_IV_LEN) {
ODP_DBG("Maximum IV length exceeded\n");
- free_session(session);
- return -1;
+ *status = ODP_CRYPTO_SES_CREATE_ERR_INV_CIPHER;
+ goto err;
}
/* Copy IV data */
@@ -764,8 +761,7 @@ odp_crypto_session_create(odp_crypto_session_param_t *param,
/* Check result */
if (rc) {
*status = ODP_CRYPTO_SES_CREATE_ERR_INV_CIPHER;
- free_session(session);
- return -1;
+ goto err;
}
aes_gcm = 0;
@@ -825,13 +821,20 @@ odp_crypto_session_create(odp_crypto_session_param_t
*param,
/* Check result */
if (rc) {
*status = ODP_CRYPTO_SES_CREATE_ERR_INV_AUTH;
- free_session(session);
- return -1;
+ goto err;
}
/* We're happy */
*session_out = (intptr_t)session;
+ *status = ODP_CRYPTO_SES_CREATE_ERR_NONE;
return 0;
+
+err:
+ /* error status should be set at this moment */
+ if (session != NULL)
+ free_session(session);
+ *session_out = ODP_CRYPTO_SESSION_INVALID;
+ return -1;
}
int odp_crypto_session_destroy(odp_crypto_session_t session)
[lng-odp] [PATCH API-NEXT v2 1/2] api: crypto: clarify for odp_crypto_session_create
From: Dmitry Eremin-Solenikov Signed-off-by: Dmitry Eremin-Solenikov --- /** Email created from pull request 95 (lumag:crypto-session-create) ** https://github.com/Linaro/odp/pull/95 ** Patch: https://github.com/Linaro/odp/pull/95.patch ** Base sha: 2394317ab247fa14b4e239aec512afee7eac4524 ** Merge commit sha: bc2a40cc94ced5ba8ec7b86fbdef99958ed37009 **/ include/odp/api/spec/crypto.h | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/include/odp/api/spec/crypto.h b/include/odp/api/spec/crypto.h index 6d4807c4..715ba9a5 100644 --- a/include/odp/api/spec/crypto.h +++ b/include/odp/api/spec/crypto.h @@ -647,11 +647,12 @@ int odp_crypto_auth_capability(odp_auth_alg_t auth, * * Create a crypto session according to the session parameters. Use * odp_crypto_session_param_init() to initialize parameters into their - * default values. + * default values. If call ends up with an error no new session will be + * created. * - * @param param Session parameters - * @param session Created session else ODP_CRYPTO_SESSION_INVALID - * @param statusFailure code if unsuccessful + * @param paramSession parameters + * @param[out] session Created session else ODP_CRYPTO_SESSION_INVALID + * @param[out] status Failure code if unsuccessful * * @retval 0 on success * @retval <0 on failure
[lng-odp] [PATCH API-NEXT v2 0/2] api: crypto: clarify for odp_crypto_session_create
Signed-off-by: Dmitry Eremin-Solenikov dmitry.ereminsoleni...@linaro.org github /** Email created from pull request 95 (lumag:crypto-session-create) ** https://github.com/Linaro/odp/pull/95 ** Patch: https://github.com/Linaro/odp/pull/95.patch ** Base sha: 2394317ab247fa14b4e239aec512afee7eac4524 ** Merge commit sha: bc2a40cc94ced5ba8ec7b86fbdef99958ed37009 **/ /github checkpatch.pl total: 0 errors, 0 warnings, 0 checks, 16 lines checked to_send-p-000.patch has no obvious style problems and is ready for submission. total: 0 errors, 0 warnings, 0 checks, 56 lines checked to_send-p-001.patch has no obvious style problems and is ready for submission. /checkpatch.pl
[lng-odp] ODP1.15 buffer alignment issue
Hi, I'm trying to move to odp1.15 and encounter with an buffer alignment issue. It seems that linux-generic implementation make sure that the data address is align with the requested alignment and not the buffer address itself (look at the code sniped below). On ODP1.11 (the current version we use) the buffer was aligned correctly. Is the current behavior is the expected one? Our HW (and probably other HWs) rely on the fact that the buffers are aligned. >From odp_pool.c, init_buffers: offset = pool->headroom; /* move to correct align */ while (((uintptr_t)&data[offset]) % pool->align != 0) offset++; Regards, Liron
[lng-odp] [Linaro/odp] 239431: Revert "api: schedule: add thread removal correcti...
Branch: refs/heads/api-next Home: https://github.com/Linaro/odp Commit: 2394317ab247fa14b4e239aec512afee7eac4524 https://github.com/Linaro/odp/commit/2394317ab247fa14b4e239aec512afee7eac4524 Author: Maxim Uvarov Date: 2017-08-07 (Mon, 07 Aug 2017) Changed paths: M include/odp/api/spec/schedule.h Log Message: --- Revert "api: schedule: add thread removal correction" This reverts commit 09ce9406 api: schedule: add thread removal correction Needed some more discussion before merging this. Signed-off-by: Maxim Uvarov
Re: [lng-odp] [Linaro/odp] 09ce94: api: schedule: add thread removal correction
This is an API change, that I did oppose on June 16th. Why it's now merged?
What is the requirement ("All threads that have joined the group must leave
before destroying the group") ? A thread may be in a group due to automatic
group (no create/join called), being part of thread mask in a create call or
being part of thread mask in a join call. Also, thread itself may not call
create/join/leave, but another thread may do that for it.
In which cases a thread needs to be removed by explicit leave() call? Always,
when create() was called, only when join() was called ?
-Petri
> -Original Message-
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of
> GitHub
> Sent: Monday, August 07, 2017 1:59 PM
> To: lng-odp@lists.linaro.org
> Subject: [lng-odp] [Linaro/odp] 09ce94: api: schedule: add thread removal
> correction
>
> Branch: refs/heads/api-next
> Home: https://github.com/Linaro/odp
> Commit: 09ce9406e09eacadf8f739ae8499323ad1401d36
>
> https://github.com/Linaro/odp/commit/09ce9406e09eacadf8f739ae8499323ad1401
> d36
> Author: Honnappa Nagarahalli
> Date: 2017-08-07 (Mon, 07 Aug 2017)
>
> Changed paths:
> M include/odp/api/spec/schedule.h
>
> Log Message:
> ---
> api: schedule: add thread removal correction
>
> Clarify thread removal status from scheduler group
> before calling odp_schedule_group_destroy.
>
> Signed-off-by: Honnappa Nagarahalli
> Reviewed-by: Brian Brooks
> Reviewed-by: Ola Liljedahl
> Reviewed-by: Bill Fischofer
> Signed-off-by: Maxim Uvarov
>
[lng-odp] [Linaro/odp] 09ce94: api: schedule: add thread removal correction
Branch: refs/heads/api-next Home: https://github.com/Linaro/odp Commit: 09ce9406e09eacadf8f739ae8499323ad1401d36 https://github.com/Linaro/odp/commit/09ce9406e09eacadf8f739ae8499323ad1401d36 Author: Honnappa Nagarahalli Date: 2017-08-07 (Mon, 07 Aug 2017) Changed paths: M include/odp/api/spec/schedule.h Log Message: --- api: schedule: add thread removal correction Clarify thread removal status from scheduler group before calling odp_schedule_group_destroy. Signed-off-by: Honnappa Nagarahalli Reviewed-by: Brian Brooks Reviewed-by: Ola Liljedahl Reviewed-by: Bill Fischofer Signed-off-by: Maxim Uvarov
[lng-odp] [PATCH API-NEXT v2 1/1] linux-gen: timer: control timer pool polling frequency dynamically
From: Joyce Kong
Adjust frequency of timer pool polling by the duration
of timer. There needs to be 0 timer pool polling when
no timer pool created.
Change-Id: I54b7043411c2ec780070f727d0f452df57e1291b
Signed-off-by: Joyce Kong
---
/** Email created from pull request 107 (JoyceKong-arm:api-next-timer)
** https://github.com/Linaro/odp/pull/107
** Patch: https://github.com/Linaro/odp/pull/107.patch
** Base sha: a165ff4093f666e5481ae5113f70b41e3354520d
** Merge commit sha: c7b27e1863017508cbbf54d74ce9f39eb01c575f
**/
platform/linux-generic/include/odp_timer_internal.h | 3 ---
platform/linux-generic/odp_timer.c | 13 -
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/platform/linux-generic/include/odp_timer_internal.h
b/platform/linux-generic/include/odp_timer_internal.h
index 67ee9fef..0759f727 100644
--- a/platform/linux-generic/include/odp_timer_internal.h
+++ b/platform/linux-generic/include/odp_timer_internal.h
@@ -20,9 +20,6 @@
#include
#include
-/* Minimum number of nanoseconds between checking timer pools. */
-#define CONFIG_TIMER_RUN_RATELIMIT_NS 100
-
/* Minimum number of scheduling rounds between checking timer pools. */
#define CONFIG_TIMER_RUN_RATELIMIT_ROUNDS 1
diff --git a/platform/linux-generic/odp_timer.c
b/platform/linux-generic/odp_timer.c
index fdb48902..b359f7c6 100644
--- a/platform/linux-generic/odp_timer.c
+++ b/platform/linux-generic/odp_timer.c
@@ -75,6 +75,7 @@ static _odp_atomic_flag_t locks[NUM_LOCKS]; /* Multiple locks
per cache line! */
/* Max timer resolution in nanoseconds */
static uint64_t highest_res_ns;
+static uint64_t min_res_ns = INT64_MAX;
/**
* Translation between timeout buffer and timeout header
@@ -742,6 +743,9 @@ unsigned _timer_run(void)
CONFIG_TIMER_RUN_RATELIMIT_ROUNDS;
odp_time_t now;
+ if (odp_atomic_load_u32(&num_timer_pools) == 0)
+ return 0;
+
/* Rate limit how often this thread checks the timer pools. */
if (CONFIG_TIMER_RUN_RATELIMIT_ROUNDS > 1) {
@@ -984,6 +988,13 @@ odp_timer_pool_create(const char *name,
__odp_errno = EINVAL;
return ODP_TIMER_POOL_INVALID;
}
+
+ if (min_res_ns > param->res_ns) {
+ min_res_ns = param->res_ns;
+ time_per_ratelimit_period =
+ odp_time_global_from_ns(min_res_ns / 2);
+ }
+
return odp_timer_pool_new(name, param);
}
@@ -1188,7 +1199,7 @@ int odp_timer_init_global(const odp_init_t *params)
!params->not_used.feat.timer;
time_per_ratelimit_period =
- odp_time_global_from_ns(CONFIG_TIMER_RUN_RATELIMIT_NS);
+ odp_time_global_from_ns(min_res_ns / 2);
if (!inline_timers) {
timer_res_init();
[lng-odp] [PATCH API-NEXT v2 0/1] linux-gen: timer: control the timer pool polling frequency dynamically
The frequency of timer pool polling needs to be adjusted according to the duration of the timer started in the timer module. And there need to be 0 timer pool polling when no timer pools/timers created. At the same time, enable inline timers when passing NULL for odp_init_global. Signed-off-by: Joyce Kong joyce.k...@arm.com github /** Email created from pull request 107 (JoyceKong-arm:api-next-timer) ** https://github.com/Linaro/odp/pull/107 ** Patch: https://github.com/Linaro/odp/pull/107.patch ** Base sha: a165ff4093f666e5481ae5113f70b41e3354520d ** Merge commit sha: c7b27e1863017508cbbf54d74ce9f39eb01c575f **/ /github checkpatch.pl ERROR: Remove Gerrit Change-Id's before submitting upstream. #12: Change-Id: I54b7043411c2ec780070f727d0f452df57e1291b total: 1 errors, 0 warnings, 0 checks, 46 lines checked to_send-p-000.patch has style problems, please review. If any of these errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS. /checkpatch.pl
[lng-odp] [Bug 3182] Memory allocation checks
https://bugs.linaro.org/show_bug.cgi?id=3182 Ilias Apalodimas changed: What|Removed |Added Resolution|--- |FIXED Status|IN_PROGRESS |RESOLVED --- Comment #4 from Ilias Apalodimas --- Fixed by: https://github.com/Linaro/odp/commit/ab62d19d4811e2699ac0614bf4ba1d481bb3b2b1 -- You are receiving this mail because: You are on the CC list for the bug.
