On Thu, Apr 14, 2016 at 9:21 AM, Petri Savolainen <
[email protected]> wrote:

> Added queue context length parameter, which is a hint for
> the implementation for how much to prefetch. Added the same in
> to the set function. It's not needed for get function since
> application can store the same into context data and
> implementation may avoid to save the length if it does not
> need it (e.g. always prefetches a fixed number of bytes).
>

The code looks fine, but I'm a bit confused about this comment.  Is the
length the actual byte length of the context or is it a prefetch length
request?  If the latter (i.e., the actual struct pointed to by the user
context is may be longer than this field) then perhaps prefetch_len would
be a better name to make it clear what its intended purpose is.


>
> Signed-off-by: Petri Savolainen <[email protected]>
> ---
>  include/odp/api/spec/queue.h          |  9 ++++++++-
>  platform/linux-generic/odp_queue.c    |  3 ++-
>  test/validation/queue/queue.c         | 10 ++++++----
>  test/validation/scheduler/scheduler.c |  6 +++---
>  4 files changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/include/odp/api/spec/queue.h b/include/odp/api/spec/queue.h
> index 51d94a2..ecb1a6d 100644
> --- a/include/odp/api/spec/queue.h
> +++ b/include/odp/api/spec/queue.h
> @@ -139,6 +139,12 @@ typedef struct odp_queue_param_t {
>           * pointer for prefetching the context data. Default value of the
>           * pointer is NULL. */
>         void *context;
> +
> +       /** Queue context data length in bytes
> +         *
> +         * The implementation may use this value as a hint for the number
> of
> +         * context data bytes to prefetch. Default value is zero (no
> hint). */
> +       uint32_t context_len;
>  } odp_queue_param_t;
>
>  /**
> @@ -191,11 +197,12 @@ odp_queue_t odp_queue_lookup(const char *name);
>   *
>   * @param queue    Queue handle
>   * @param context  Address to the queue context
> + * @param len      Queue context data length in bytes
>   *
>   * @retval 0 on success
>   * @retval <0 on failure
>   */
> -int odp_queue_context_set(odp_queue_t queue, void *context);
> +int odp_queue_context_set(odp_queue_t queue, void *context, uint32_t len);
>
>  /**
>   * Get queue context
> diff --git a/platform/linux-generic/odp_queue.c
> b/platform/linux-generic/odp_queue.c
> index 342ffa2..9a9462d 100644
> --- a/platform/linux-generic/odp_queue.c
> +++ b/platform/linux-generic/odp_queue.c
> @@ -344,7 +344,8 @@ int odp_queue_destroy(odp_queue_t handle)
>         return 0;
>  }
>
> -int odp_queue_context_set(odp_queue_t handle, void *context)
> +int odp_queue_context_set(odp_queue_t handle, void *context,
> +                         uint32_t len ODP_UNUSED)
>  {
>         queue_entry_t *queue;
>         queue = queue_to_qentry(handle);
> diff --git a/test/validation/queue/queue.c b/test/validation/queue/queue.c
> index 50c6857..96d21fa 100644
> --- a/test/validation/queue/queue.c
> +++ b/test/validation/queue/queue.c
> @@ -12,7 +12,7 @@
>  #define MSG_POOL_SIZE           (4 * 1024 * 1024)
>  #define CONFIG_MAX_ITERATION    (100)
>
> -static int queue_contest = 0xff;
> +static int queue_context = 0xff;
>  static odp_pool_t pool;
>
>  int queue_suite_init(void)
> @@ -76,10 +76,11 @@ void queue_test_sunnydays(void)
>         CU_ASSERT_EQUAL(ODP_SCHED_SYNC_PARALLEL,
>                         odp_queue_sched_type(queue_id));
>
> -       CU_ASSERT(0 == odp_queue_context_set(queue_id, &queue_contest));
> +       CU_ASSERT(0 == odp_queue_context_set(queue_id, &queue_context,
> +                                            sizeof(queue_context)));
>
>         prtn = odp_queue_context(queue_id);
> -       CU_ASSERT(&queue_contest == (int *)prtn);
> +       CU_ASSERT(&queue_context == (int *)prtn);
>
>         msg_pool = odp_pool_lookup("msg_pool");
>         buf = odp_buffer_alloc(msg_pool);
> @@ -144,7 +145,8 @@ void queue_test_info(void)
>         /* Create a plain queue and set context */
>         q_plain = odp_queue_create(nq_plain, NULL);
>         CU_ASSERT(ODP_QUEUE_INVALID != q_plain);
> -       CU_ASSERT(odp_queue_context_set(q_plain, q_plain_ctx) == 0);
> +       CU_ASSERT(odp_queue_context_set(q_plain, q_plain_ctx,
> +                                       sizeof(q_plain_ctx)) == 0);
>
>         /* Create a scheduled ordered queue with explicitly set params */
>         odp_queue_param_init(&param);
> diff --git a/test/validation/scheduler/scheduler.c
> b/test/validation/scheduler/scheduler.c
> index ae98401..dae859e 100644
> --- a/test/validation/scheduler/scheduler.c
> +++ b/test/validation/scheduler/scheduler.c
> @@ -592,7 +592,7 @@ static void chaos_run(unsigned int qtype)
>                 CU_ASSERT_FATAL(globals->chaos_q[i].handle !=
>                                 ODP_QUEUE_INVALID);
>                 rc = odp_queue_context_set(globals->chaos_q[i].handle,
> -                                          CHAOS_NDX_TO_PTR(i));
> +                                          CHAOS_NDX_TO_PTR(i), 0);
>                 CU_ASSERT_FATAL(rc == 0);
>         }
>
> @@ -1374,7 +1374,7 @@ static int create_queues(void)
>                         pqctx->ctx_handle = queue_ctx_buf;
>                         pqctx->sequence = 0;
>
> -                       rc = odp_queue_context_set(pq, pqctx);
> +                       rc = odp_queue_context_set(pq, pqctx, 0);
>
>                         if (rc != 0) {
>                                 printf("Cannot set plain queue context\n");
> @@ -1419,7 +1419,7 @@ static int create_queues(void)
>                                 qctx->lock_sequence[ndx] = 0;
>                         }
>
> -                       rc = odp_queue_context_set(q, qctx);
> +                       rc = odp_queue_context_set(q, qctx, 0);
>
>                         if (rc != 0) {
>                                 printf("Cannot set queue context\n");
> --
> 2.7.2
>
> _______________________________________________
> lng-odp mailing list
> [email protected]
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to