diff mbox series

[1/2] add queue size param and related capability

Message ID 1491538024-112904-1-git-send-email-honnappa.nagarahalli@linaro.org
State New
Headers show
Series [1/2] add queue size param and related capability | expand

Commit Message

Honnappa Nagarahalli April 7, 2017, 4:07 a.m. UTC
Added size parameter indicating the maximum number of events in the
queue and the corresponding queue capability changes.

Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@linaro.org>

---
 include/odp/api/spec/queue.h       | 12 ++++++++++++
 platform/linux-generic/odp_queue.c |  1 +
 2 files changed, 13 insertions(+)

-- 
2.7.4

Comments

Savolainen, Petri (Nokia - FI/Espoo) April 7, 2017, 7:54 a.m. UTC | #1
See my patch series: [PATCH v3 1/2] api: queue: added queue size param


> -----Original Message-----

> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

> Honnappa Nagarahalli

> Sent: Friday, April 07, 2017 7:07 AM

> To: lng-odp@lists.linaro.org

> Subject: [lng-odp] [PATCH 1/2] add queue size param and related capability

> 

> Added size parameter indicating the maximum number of events in the

> queue and the corresponding queue capability changes.

> 

> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@linaro.org>

> ---

>  include/odp/api/spec/queue.h       | 12 ++++++++++++

>  platform/linux-generic/odp_queue.c |  1 +

>  2 files changed, 13 insertions(+)

> 

> diff --git a/include/odp/api/spec/queue.h b/include/odp/api/spec/queue.h

> index 7972fea..ccb6fb8 100644

> --- a/include/odp/api/spec/queue.h

> +++ b/include/odp/api/spec/queue.h

> @@ -112,6 +112,12 @@ typedef struct odp_queue_capability_t {

>  	/** Number of scheduling priorities */

>  	unsigned sched_prios;

> 

> +	/** Maximum number of events in the queue.

> +	  *

> +	  * Value of zero indicates the size is limited only by the

> available

> +	  * memory in the system. */

> +	unsigned max_size;

> +

>  } odp_queue_capability_t;

> 

>  /**

> @@ -124,6 +130,12 @@ typedef struct odp_queue_param_t {

>  	  * the queue type. */

>  	odp_queue_type_t type;

> 

> +	/** Queue size

> +	  *

> +	  * Maximum number of events in the queue. Value of 0 chooses

> the

> +	  * default configuration of the implementation. */

> +	uint32_t size;

> +

>  	/** Enqueue mode

>  	  *

>  	  * Default value for both queue types is ODP_QUEUE_OP_MT.

> Application

> diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-

> generic/odp_queue.c

> index fcf4bf5..5a50a57 100644

> --- a/platform/linux-generic/odp_queue.c

> +++ b/platform/linux-generic/odp_queue.c

> @@ -175,6 +175,7 @@ int odp_queue_capability(odp_queue_capability_t *capa)

>  	capa->max_ordered_locks = sched_fn->max_ordered_locks();

>  	capa->max_sched_groups  = sched_fn->num_grps();

>  	capa->sched_prios       = odp_schedule_num_prio();

> +	capa->max_size          = 0;

> 

>  	return 0;

>  }

> --

> 2.7.4
Honnappa Nagarahalli April 10, 2017, 4:17 p.m. UTC | #2
Hi Bala,
    Continuing the discussion from the call, as I mentioned in the
call today, the queues need to hold all kinds of events and not just
packets. The events need not be defined by ODP (like timeout events)
also. The application may have its own events.

In such a case, queue size does not dependent on the capacity of
various pools supported in ODP. The size should depend on the
implementation.

If the queue is allocated out of memory, then the size should depend
on the available amount of memory at any point in time.

Thank you,
Honnappa

On 7 April 2017 at 02:54, Savolainen, Petri (Nokia - FI/Espoo)
<petri.savolainen@nokia-bell-labs.com> wrote:
> See my patch series: [PATCH v3 1/2] api: queue: added queue size param

>

>

>> -----Original Message-----

>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

>> Honnappa Nagarahalli

>> Sent: Friday, April 07, 2017 7:07 AM

>> To: lng-odp@lists.linaro.org

>> Subject: [lng-odp] [PATCH 1/2] add queue size param and related capability

>>

>> Added size parameter indicating the maximum number of events in the

>> queue and the corresponding queue capability changes.

>>

>> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@linaro.org>

>> ---

>>  include/odp/api/spec/queue.h       | 12 ++++++++++++

>>  platform/linux-generic/odp_queue.c |  1 +

>>  2 files changed, 13 insertions(+)

>>

>> diff --git a/include/odp/api/spec/queue.h b/include/odp/api/spec/queue.h

>> index 7972fea..ccb6fb8 100644

>> --- a/include/odp/api/spec/queue.h

>> +++ b/include/odp/api/spec/queue.h

>> @@ -112,6 +112,12 @@ typedef struct odp_queue_capability_t {

>>       /** Number of scheduling priorities */

>>       unsigned sched_prios;

>>

>> +     /** Maximum number of events in the queue.

>> +       *

>> +       * Value of zero indicates the size is limited only by the

>> available

>> +       * memory in the system. */

>> +     unsigned max_size;

>> +

>>  } odp_queue_capability_t;

>>

>>  /**

>> @@ -124,6 +130,12 @@ typedef struct odp_queue_param_t {

>>         * the queue type. */

>>       odp_queue_type_t type;

>>

>> +     /** Queue size

>> +       *

>> +       * Maximum number of events in the queue. Value of 0 chooses

>> the

>> +       * default configuration of the implementation. */

>> +     uint32_t size;

>> +

>>       /** Enqueue mode

>>         *

>>         * Default value for both queue types is ODP_QUEUE_OP_MT.

>> Application

>> diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-

>> generic/odp_queue.c

>> index fcf4bf5..5a50a57 100644

>> --- a/platform/linux-generic/odp_queue.c

>> +++ b/platform/linux-generic/odp_queue.c

>> @@ -175,6 +175,7 @@ int odp_queue_capability(odp_queue_capability_t *capa)

>>       capa->max_ordered_locks = sched_fn->max_ordered_locks();

>>       capa->max_sched_groups  = sched_fn->num_grps();

>>       capa->sched_prios       = odp_schedule_num_prio();

>> +     capa->max_size          = 0;

>>

>>       return 0;

>>  }

>> --

>> 2.7.4

>
Bill Fischofer April 10, 2017, 4:34 p.m. UTC | #3
On Thu, Apr 6, 2017 at 11:07 PM, Honnappa Nagarahalli
<honnappa.nagarahalli@linaro.org> wrote:
> Added size parameter indicating the maximum number of events in the

> queue and the corresponding queue capability changes.

>

> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@linaro.org>

> ---

>  include/odp/api/spec/queue.h       | 12 ++++++++++++

>  platform/linux-generic/odp_queue.c |  1 +

>  2 files changed, 13 insertions(+)

>

> diff --git a/include/odp/api/spec/queue.h b/include/odp/api/spec/queue.h

> index 7972fea..ccb6fb8 100644

> --- a/include/odp/api/spec/queue.h

> +++ b/include/odp/api/spec/queue.h

> @@ -112,6 +112,12 @@ typedef struct odp_queue_capability_t {

>         /** Number of scheduling priorities */

>         unsigned sched_prios;

>

> +       /** Maximum number of events in the queue.

> +         *

> +         * Value of zero indicates the size is limited only by the available

> +         * memory in the system. */


This is not a specification, as memory may not be the controlling
factor. I'd rephrase this as:

"A value of zero indicates that there is no fixed upper bound to the
queue size."

> +       unsigned max_size;

> +

>  } odp_queue_capability_t;

>

>  /**

> @@ -124,6 +130,12 @@ typedef struct odp_queue_param_t {

>           * the queue type. */

>         odp_queue_type_t type;

>

> +       /** Queue size

> +         *

> +         * Maximum number of events in the queue. Value of 0 chooses the

> +         * default configuration of the implementation. */

> +       uint32_t size;

> +

>         /** Enqueue mode

>           *

>           * Default value for both queue types is ODP_QUEUE_OP_MT. Application

> diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-generic/odp_queue.c

> index fcf4bf5..5a50a57 100644

> --- a/platform/linux-generic/odp_queue.c

> +++ b/platform/linux-generic/odp_queue.c

> @@ -175,6 +175,7 @@ int odp_queue_capability(odp_queue_capability_t *capa)

>         capa->max_ordered_locks = sched_fn->max_ordered_locks();

>         capa->max_sched_groups  = sched_fn->num_grps();

>         capa->sched_prios       = odp_schedule_num_prio();

> +       capa->max_size          = 0;

>

>         return 0;

>  }

> --

> 2.7.4

>
Bill Fischofer April 10, 2017, 4:49 p.m. UTC | #4
On Mon, Apr 10, 2017 at 11:17 AM, Honnappa Nagarahalli
<honnappa.nagarahalli@linaro.org> wrote:
> Hi Bala,

>     Continuing the discussion from the call, as I mentioned in the

> call today, the queues need to hold all kinds of events and not just

> packets. The events need not be defined by ODP (like timeout events)

> also. The application may have its own events.

>

> In such a case, queue size does not dependent on the capacity of

> various pools supported in ODP. The size should depend on the

> implementation.

>

> If the queue is allocated out of memory, then the size should depend

> on the available amount of memory at any point in time.


The real distinction is whether the implementation imposes a fixed
upper bound to queue size. This may be, for example, because queues
are implemented in hardware and there are hardware-imposed limits to
queue size. Or it may be that the implementation preallocates
resources at configuration time and these have fixed numbers
associated with them.

ODP got along just fine before without having known queue sizes, so
the question is "what new information does the application want here"?
The only use-case we seem to have identified is the application
intends to use a queue as a storage mechanism and it wants to ensure
that the queue has a minimum storage capacity at queue create time. In
this case, the capability is reporting whether there is a fixed upper
bound that the application can use to compare to its minimum
requirements. If the answer is "yes", then the application can adjust
its needs (or refuse to run) based on that answer. If the answer is
"no" (a returned 0 as max_size) then the application can specify it's
requested size as input to odp_queue_create() and observe whether the
request succeeds or fails.

The alternative is to not add a max_size to odp_queue_capability() at
all and simply let odp_queue_create() report success or failure in
response to a requested size. That's effectively what we have today,
except that by default the requested "size" is forced to 0, meaning
that the implementation chooses what, if any, such limits exist. The
only way these limits are surfaced to applications is if
odp_queue_enq() fails because the queue is full and the implementation
doesn't use back pressure. If back pressure is used, then the
odp_queue_enq() call simply stalls until room is made available in the
queue to satisfy the request.

>

> Thank you,

> Honnappa

>

> On 7 April 2017 at 02:54, Savolainen, Petri (Nokia - FI/Espoo)

> <petri.savolainen@nokia-bell-labs.com> wrote:

>> See my patch series: [PATCH v3 1/2] api: queue: added queue size param

>>

>>

>>> -----Original Message-----

>>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

>>> Honnappa Nagarahalli

>>> Sent: Friday, April 07, 2017 7:07 AM

>>> To: lng-odp@lists.linaro.org

>>> Subject: [lng-odp] [PATCH 1/2] add queue size param and related capability

>>>

>>> Added size parameter indicating the maximum number of events in the

>>> queue and the corresponding queue capability changes.

>>>

>>> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@linaro.org>

>>> ---

>>>  include/odp/api/spec/queue.h       | 12 ++++++++++++

>>>  platform/linux-generic/odp_queue.c |  1 +

>>>  2 files changed, 13 insertions(+)

>>>

>>> diff --git a/include/odp/api/spec/queue.h b/include/odp/api/spec/queue.h

>>> index 7972fea..ccb6fb8 100644

>>> --- a/include/odp/api/spec/queue.h

>>> +++ b/include/odp/api/spec/queue.h

>>> @@ -112,6 +112,12 @@ typedef struct odp_queue_capability_t {

>>>       /** Number of scheduling priorities */

>>>       unsigned sched_prios;

>>>

>>> +     /** Maximum number of events in the queue.

>>> +       *

>>> +       * Value of zero indicates the size is limited only by the

>>> available

>>> +       * memory in the system. */

>>> +     unsigned max_size;

>>> +

>>>  } odp_queue_capability_t;

>>>

>>>  /**

>>> @@ -124,6 +130,12 @@ typedef struct odp_queue_param_t {

>>>         * the queue type. */

>>>       odp_queue_type_t type;

>>>

>>> +     /** Queue size

>>> +       *

>>> +       * Maximum number of events in the queue. Value of 0 chooses

>>> the

>>> +       * default configuration of the implementation. */

>>> +     uint32_t size;

>>> +

>>>       /** Enqueue mode

>>>         *

>>>         * Default value for both queue types is ODP_QUEUE_OP_MT.

>>> Application

>>> diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-

>>> generic/odp_queue.c

>>> index fcf4bf5..5a50a57 100644

>>> --- a/platform/linux-generic/odp_queue.c

>>> +++ b/platform/linux-generic/odp_queue.c

>>> @@ -175,6 +175,7 @@ int odp_queue_capability(odp_queue_capability_t *capa)

>>>       capa->max_ordered_locks = sched_fn->max_ordered_locks();

>>>       capa->max_sched_groups  = sched_fn->num_grps();

>>>       capa->sched_prios       = odp_schedule_num_prio();

>>> +     capa->max_size          = 0;

>>>

>>>       return 0;

>>>  }

>>> --

>>> 2.7.4

>>
Balasubramanian Manoharan April 11, 2017, 10:51 a.m. UTC | #5
On 10 April 2017 at 21:47, Honnappa Nagarahalli
<honnappa.nagarahalli@linaro.org> wrote:
> Hi Bala,

>     Continuing the discussion from the call, as I mentioned in the

> call today, the queues need to hold all kinds of events and not just

> packets. The events need not be defined by ODP (like timeout events)

> also. The application may have its own events.

>

> In such a case, queue size does not dependent on the capacity of

> various pools supported in ODP. The size should depend on the

> implementation.


ODP queues hold odp_event_t and these events needs to be allocated
from a pool even in case of a timer. Also since in your case you are returning
the maximum number of events across all the queues there needs to be a valid
use-case for this value.

Regards,
Bala

>

> If the queue is allocated out of memory, then the size should depend

> on the available amount of memory at any point in time.

>

> Thank you,

> Honnappa

>

> On 7 April 2017 at 02:54, Savolainen, Petri (Nokia - FI/Espoo)

> <petri.savolainen@nokia-bell-labs.com> wrote:

>> See my patch series: [PATCH v3 1/2] api: queue: added queue size param

>>

>>

>>> -----Original Message-----

>>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

>>> Honnappa Nagarahalli

>>> Sent: Friday, April 07, 2017 7:07 AM

>>> To: lng-odp@lists.linaro.org

>>> Subject: [lng-odp] [PATCH 1/2] add queue size param and related capability

>>>

>>> Added size parameter indicating the maximum number of events in the

>>> queue and the corresponding queue capability changes.

>>>

>>> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@linaro.org>

>>> ---

>>>  include/odp/api/spec/queue.h       | 12 ++++++++++++

>>>  platform/linux-generic/odp_queue.c |  1 +

>>>  2 files changed, 13 insertions(+)

>>>

>>> diff --git a/include/odp/api/spec/queue.h b/include/odp/api/spec/queue.h

>>> index 7972fea..ccb6fb8 100644

>>> --- a/include/odp/api/spec/queue.h

>>> +++ b/include/odp/api/spec/queue.h

>>> @@ -112,6 +112,12 @@ typedef struct odp_queue_capability_t {

>>>       /** Number of scheduling priorities */

>>>       unsigned sched_prios;

>>>

>>> +     /** Maximum number of events in the queue.

>>> +       *

>>> +       * Value of zero indicates the size is limited only by the

>>> available

>>> +       * memory in the system. */

>>> +     unsigned max_size;

>>> +

>>>  } odp_queue_capability_t;

>>>

>>>  /**

>>> @@ -124,6 +130,12 @@ typedef struct odp_queue_param_t {

>>>         * the queue type. */

>>>       odp_queue_type_t type;

>>>

>>> +     /** Queue size

>>> +       *

>>> +       * Maximum number of events in the queue. Value of 0 chooses

>>> the

>>> +       * default configuration of the implementation. */

>>> +     uint32_t size;

>>> +

>>>       /** Enqueue mode

>>>         *

>>>         * Default value for both queue types is ODP_QUEUE_OP_MT.

>>> Application

>>> diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-

>>> generic/odp_queue.c

>>> index fcf4bf5..5a50a57 100644

>>> --- a/platform/linux-generic/odp_queue.c

>>> +++ b/platform/linux-generic/odp_queue.c

>>> @@ -175,6 +175,7 @@ int odp_queue_capability(odp_queue_capability_t *capa)

>>>       capa->max_ordered_locks = sched_fn->max_ordered_locks();

>>>       capa->max_sched_groups  = sched_fn->num_grps();

>>>       capa->sched_prios       = odp_schedule_num_prio();

>>> +     capa->max_size          = 0;

>>>

>>>       return 0;

>>>  }

>>> --

>>> 2.7.4

>>
Honnappa Nagarahalli April 12, 2017, 4:04 a.m. UTC | #6
On 10 April 2017 at 11:34, Bill Fischofer <bill.fischofer@linaro.org> wrote:
> On Thu, Apr 6, 2017 at 11:07 PM, Honnappa Nagarahalli

> <honnappa.nagarahalli@linaro.org> wrote:

>> Added size parameter indicating the maximum number of events in the

>> queue and the corresponding queue capability changes.

>>

>> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@linaro.org>

>> ---

>>  include/odp/api/spec/queue.h       | 12 ++++++++++++

>>  platform/linux-generic/odp_queue.c |  1 +

>>  2 files changed, 13 insertions(+)

>>

>> diff --git a/include/odp/api/spec/queue.h b/include/odp/api/spec/queue.h

>> index 7972fea..ccb6fb8 100644

>> --- a/include/odp/api/spec/queue.h

>> +++ b/include/odp/api/spec/queue.h

>> @@ -112,6 +112,12 @@ typedef struct odp_queue_capability_t {

>>         /** Number of scheduling priorities */

>>         unsigned sched_prios;

>>

>> +       /** Maximum number of events in the queue.

>> +         *

>> +         * Value of zero indicates the size is limited only by the available

>> +         * memory in the system. */

>

> This is not a specification, as memory may not be the controlling

> factor. I'd rephrase this as:

>

> "A value of zero indicates that there is no fixed upper bound to the

> queue size."


In the case of pool's capability, the max sizes have been tied to the
available memory in the system. We need to be consistent.
There is a fixed upper bound (depending on memory or something else).
May be it can be stated as "Value of zero indicates that the size is
limited only by the system capabilities".

>

>> +       unsigned max_size;

>> +

>>  } odp_queue_capability_t;

>>

>>  /**

>> @@ -124,6 +130,12 @@ typedef struct odp_queue_param_t {

>>           * the queue type. */

>>         odp_queue_type_t type;

>>

>> +       /** Queue size

>> +         *

>> +         * Maximum number of events in the queue. Value of 0 chooses the

>> +         * default configuration of the implementation. */

>> +       uint32_t size;

>> +

>>         /** Enqueue mode

>>           *

>>           * Default value for both queue types is ODP_QUEUE_OP_MT. Application

>> diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-generic/odp_queue.c

>> index fcf4bf5..5a50a57 100644

>> --- a/platform/linux-generic/odp_queue.c

>> +++ b/platform/linux-generic/odp_queue.c

>> @@ -175,6 +175,7 @@ int odp_queue_capability(odp_queue_capability_t *capa)

>>         capa->max_ordered_locks = sched_fn->max_ordered_locks();

>>         capa->max_sched_groups  = sched_fn->num_grps();

>>         capa->sched_prios       = odp_schedule_num_prio();

>> +       capa->max_size          = 0;

>>

>>         return 0;

>>  }

>> --

>> 2.7.4

>>
Honnappa Nagarahalli April 12, 2017, 4:14 a.m. UTC | #7
On 10 April 2017 at 11:49, Bill Fischofer <bill.fischofer@linaro.org> wrote:
> On Mon, Apr 10, 2017 at 11:17 AM, Honnappa Nagarahalli

> <honnappa.nagarahalli@linaro.org> wrote:

>> Hi Bala,

>>     Continuing the discussion from the call, as I mentioned in the

>> call today, the queues need to hold all kinds of events and not just

>> packets. The events need not be defined by ODP (like timeout events)

>> also. The application may have its own events.

>>

>> In such a case, queue size does not dependent on the capacity of

>> various pools supported in ODP. The size should depend on the

>> implementation.

>>

>> If the queue is allocated out of memory, then the size should depend

>> on the available amount of memory at any point in time.

>

> The real distinction is whether the implementation imposes a fixed

> upper bound to queue size. This may be, for example, because queues

> are implemented in hardware and there are hardware-imposed limits to

> queue size. Or it may be that the implementation preallocates

> resources at configuration time and these have fixed numbers

> associated with them.

>

> ODP got along just fine before without having known queue sizes, so

> the question is "what new information does the application want here"?

> The only use-case we seem to have identified is the application

> intends to use a queue as a storage mechanism and it wants to ensure

> that the queue has a minimum storage capacity at queue create time. In

> this case, the capability is reporting whether there is a fixed upper

> bound that the application can use to compare to its minimum

> requirements. If the answer is "yes", then the application can adjust

> its needs (or refuse to run) based on that answer. If the answer is

> "no" (a returned 0 as max_size) then the application can specify it's

> requested size as input to odp_queue_create() and observe whether the

> request succeeds or fails.

>

> The alternative is to not add a max_size to odp_queue_capability() at

> all and simply let odp_queue_create() report success or failure in

> response to a requested size. That's effectively what we have today,

> except that by default the requested "size" is forced to 0, meaning

> that the implementation chooses what, if any, such limits exist. The

> only way these limits are surfaced to applications is if

> odp_queue_enq() fails because the queue is full and the implementation

> doesn't use back pressure. If back pressure is used, then the

> odp_queue_enq() call simply stalls until room is made available in the

> queue to satisfy the request.


I prefer to leave the back pressure implementation to the application.
This allows the application to have different algorithms to wait till
the queue has space. For ex: on ARM platforms, the implementation can
use WFE/SEV.

>

>>

>> Thank you,

>> Honnappa

>>

>> On 7 April 2017 at 02:54, Savolainen, Petri (Nokia - FI/Espoo)

>> <petri.savolainen@nokia-bell-labs.com> wrote:

>>> See my patch series: [PATCH v3 1/2] api: queue: added queue size param

>>>

>>>

>>>> -----Original Message-----

>>>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

>>>> Honnappa Nagarahalli

>>>> Sent: Friday, April 07, 2017 7:07 AM

>>>> To: lng-odp@lists.linaro.org

>>>> Subject: [lng-odp] [PATCH 1/2] add queue size param and related capability

>>>>

>>>> Added size parameter indicating the maximum number of events in the

>>>> queue and the corresponding queue capability changes.

>>>>

>>>> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@linaro.org>

>>>> ---

>>>>  include/odp/api/spec/queue.h       | 12 ++++++++++++

>>>>  platform/linux-generic/odp_queue.c |  1 +

>>>>  2 files changed, 13 insertions(+)

>>>>

>>>> diff --git a/include/odp/api/spec/queue.h b/include/odp/api/spec/queue.h

>>>> index 7972fea..ccb6fb8 100644

>>>> --- a/include/odp/api/spec/queue.h

>>>> +++ b/include/odp/api/spec/queue.h

>>>> @@ -112,6 +112,12 @@ typedef struct odp_queue_capability_t {

>>>>       /** Number of scheduling priorities */

>>>>       unsigned sched_prios;

>>>>

>>>> +     /** Maximum number of events in the queue.

>>>> +       *

>>>> +       * Value of zero indicates the size is limited only by the

>>>> available

>>>> +       * memory in the system. */

>>>> +     unsigned max_size;

>>>> +

>>>>  } odp_queue_capability_t;

>>>>

>>>>  /**

>>>> @@ -124,6 +130,12 @@ typedef struct odp_queue_param_t {

>>>>         * the queue type. */

>>>>       odp_queue_type_t type;

>>>>

>>>> +     /** Queue size

>>>> +       *

>>>> +       * Maximum number of events in the queue. Value of 0 chooses

>>>> the

>>>> +       * default configuration of the implementation. */

>>>> +     uint32_t size;

>>>> +

>>>>       /** Enqueue mode

>>>>         *

>>>>         * Default value for both queue types is ODP_QUEUE_OP_MT.

>>>> Application

>>>> diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-

>>>> generic/odp_queue.c

>>>> index fcf4bf5..5a50a57 100644

>>>> --- a/platform/linux-generic/odp_queue.c

>>>> +++ b/platform/linux-generic/odp_queue.c

>>>> @@ -175,6 +175,7 @@ int odp_queue_capability(odp_queue_capability_t *capa)

>>>>       capa->max_ordered_locks = sched_fn->max_ordered_locks();

>>>>       capa->max_sched_groups  = sched_fn->num_grps();

>>>>       capa->sched_prios       = odp_schedule_num_prio();

>>>> +     capa->max_size          = 0;

>>>>

>>>>       return 0;

>>>>  }

>>>> --

>>>> 2.7.4

>>>
Honnappa Nagarahalli April 12, 2017, 4:35 a.m. UTC | #8
On 11 April 2017 at 05:51, Bala Manoharan <bala.manoharan@linaro.org> wrote:
> On 10 April 2017 at 21:47, Honnappa Nagarahalli

> <honnappa.nagarahalli@linaro.org> wrote:

>> Hi Bala,

>>     Continuing the discussion from the call, as I mentioned in the

>> call today, the queues need to hold all kinds of events and not just

>> packets. The events need not be defined by ODP (like timeout events)

>> also. The application may have its own events.

>>

>> In such a case, queue size does not dependent on the capacity of

>> various pools supported in ODP. The size should depend on the

>> implementation.

>

> ODP queues hold odp_event_t and these events needs to be allocated

> from a pool even in case of a timer.


If I create a queue between two pipeline stages, the producer can
produce any 64b data and typecast it to odp_event_t and store it in
the queue. The consumer can dequeue from that queue and use that 64b
data according to the need of the application. It need not be
allocated from any pool.

Also since in your case you are returning
> the maximum number of events across all the queues there needs to be a valid

> use-case for this value.


The capability API would return the maximum size of a queue that the
application can create - if there is a fixed maximum queue size.
Otherwise, it would return 0, indicating that the max size is not
fixed - it is dependent on available resources at the time of
creation. One could create 10 queues of size 1K elements, but there
are not enough resources to create 11th queue with a size of 1K at
that point in time.

>

> Regards,

> Bala

>

>>

>> If the queue is allocated out of memory, then the size should depend

>> on the available amount of memory at any point in time.

>>

>> Thank you,

>> Honnappa

>>

>> On 7 April 2017 at 02:54, Savolainen, Petri (Nokia - FI/Espoo)

>> <petri.savolainen@nokia-bell-labs.com> wrote:

>>> See my patch series: [PATCH v3 1/2] api: queue: added queue size param

>>>

>>>

>>>> -----Original Message-----

>>>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

>>>> Honnappa Nagarahalli

>>>> Sent: Friday, April 07, 2017 7:07 AM

>>>> To: lng-odp@lists.linaro.org

>>>> Subject: [lng-odp] [PATCH 1/2] add queue size param and related capability

>>>>

>>>> Added size parameter indicating the maximum number of events in the

>>>> queue and the corresponding queue capability changes.

>>>>

>>>> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@linaro.org>

>>>> ---

>>>>  include/odp/api/spec/queue.h       | 12 ++++++++++++

>>>>  platform/linux-generic/odp_queue.c |  1 +

>>>>  2 files changed, 13 insertions(+)

>>>>

>>>> diff --git a/include/odp/api/spec/queue.h b/include/odp/api/spec/queue.h

>>>> index 7972fea..ccb6fb8 100644

>>>> --- a/include/odp/api/spec/queue.h

>>>> +++ b/include/odp/api/spec/queue.h

>>>> @@ -112,6 +112,12 @@ typedef struct odp_queue_capability_t {

>>>>       /** Number of scheduling priorities */

>>>>       unsigned sched_prios;

>>>>

>>>> +     /** Maximum number of events in the queue.

>>>> +       *

>>>> +       * Value of zero indicates the size is limited only by the

>>>> available

>>>> +       * memory in the system. */

>>>> +     unsigned max_size;

>>>> +

>>>>  } odp_queue_capability_t;

>>>>

>>>>  /**

>>>> @@ -124,6 +130,12 @@ typedef struct odp_queue_param_t {

>>>>         * the queue type. */

>>>>       odp_queue_type_t type;

>>>>

>>>> +     /** Queue size

>>>> +       *

>>>> +       * Maximum number of events in the queue. Value of 0 chooses

>>>> the

>>>> +       * default configuration of the implementation. */

>>>> +     uint32_t size;

>>>> +

>>>>       /** Enqueue mode

>>>>         *

>>>>         * Default value for both queue types is ODP_QUEUE_OP_MT.

>>>> Application

>>>> diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-

>>>> generic/odp_queue.c

>>>> index fcf4bf5..5a50a57 100644

>>>> --- a/platform/linux-generic/odp_queue.c

>>>> +++ b/platform/linux-generic/odp_queue.c

>>>> @@ -175,6 +175,7 @@ int odp_queue_capability(odp_queue_capability_t *capa)

>>>>       capa->max_ordered_locks = sched_fn->max_ordered_locks();

>>>>       capa->max_sched_groups  = sched_fn->num_grps();

>>>>       capa->sched_prios       = odp_schedule_num_prio();

>>>> +     capa->max_size          = 0;

>>>>

>>>>       return 0;

>>>>  }

>>>> --

>>>> 2.7.4

>>>
Balasubramanian Manoharan April 12, 2017, 5:45 a.m. UTC | #9
Regards,
Bala


On 12 April 2017 at 10:05, Honnappa Nagarahalli
<honnappa.nagarahalli@linaro.org> wrote:
> On 11 April 2017 at 05:51, Bala Manoharan <bala.manoharan@linaro.org> wrote:

>> On 10 April 2017 at 21:47, Honnappa Nagarahalli

>> <honnappa.nagarahalli@linaro.org> wrote:

>>> Hi Bala,

>>>     Continuing the discussion from the call, as I mentioned in the

>>> call today, the queues need to hold all kinds of events and not just

>>> packets. The events need not be defined by ODP (like timeout events)

>>> also. The application may have its own events.

>>>

>>> In such a case, queue size does not dependent on the capacity of

>>> various pools supported in ODP. The size should depend on the

>>> implementation.

>>

>> ODP queues hold odp_event_t and these events needs to be allocated

>> from a pool even in case of a timer.

>

> If I create a queue between two pipeline stages, the producer can

> produce any 64b data and typecast it to odp_event_t and store it in

> the queue. The consumer can dequeue from that queue and use that 64b

> data according to the need of the application. It need not be

> allocated from any pool.


The idea of storing it in the queue is the concern there are
implementations where queue is a virtual
concept and even in the use-case you have defined the data has to be
allocated from a memory region
and the pointer is only handled by the queue in my implementation.
so even if I return 0 for the capability it does not mean there is
unlimited possibility, there is a hidden limitation
either by the pool or by any memory region.

>

> Also since in your case you are returning

>> the maximum number of events across all the queues there needs to be a valid

>> use-case for this value.

>

> The capability API would return the maximum size of a queue that the

> application can create - if there is a fixed maximum queue size.

> Otherwise, it would return 0, indicating that the max size is not

> fixed - it is dependent on available resources at the time of

> creation. One could create 10 queues of size 1K elements, but there

> are not enough resources to create 11th queue with a size of 1K at

> that point in time.

>

>>

>> Regards,

>> Bala

>>

>>>

>>> If the queue is allocated out of memory, then the size should depend

>>> on the available amount of memory at any point in time.

>>>

>>> Thank you,

>>> Honnappa

>>>

>>> On 7 April 2017 at 02:54, Savolainen, Petri (Nokia - FI/Espoo)

>>> <petri.savolainen@nokia-bell-labs.com> wrote:

>>>> See my patch series: [PATCH v3 1/2] api: queue: added queue size param

>>>>

>>>>

>>>>> -----Original Message-----

>>>>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

>>>>> Honnappa Nagarahalli

>>>>> Sent: Friday, April 07, 2017 7:07 AM

>>>>> To: lng-odp@lists.linaro.org

>>>>> Subject: [lng-odp] [PATCH 1/2] add queue size param and related capability

>>>>>

>>>>> Added size parameter indicating the maximum number of events in the

>>>>> queue and the corresponding queue capability changes.

>>>>>

>>>>> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@linaro.org>

>>>>> ---

>>>>>  include/odp/api/spec/queue.h       | 12 ++++++++++++

>>>>>  platform/linux-generic/odp_queue.c |  1 +

>>>>>  2 files changed, 13 insertions(+)

>>>>>

>>>>> diff --git a/include/odp/api/spec/queue.h b/include/odp/api/spec/queue.h

>>>>> index 7972fea..ccb6fb8 100644

>>>>> --- a/include/odp/api/spec/queue.h

>>>>> +++ b/include/odp/api/spec/queue.h

>>>>> @@ -112,6 +112,12 @@ typedef struct odp_queue_capability_t {

>>>>>       /** Number of scheduling priorities */

>>>>>       unsigned sched_prios;

>>>>>

>>>>> +     /** Maximum number of events in the queue.

>>>>> +       *

>>>>> +       * Value of zero indicates the size is limited only by the

>>>>> available

>>>>> +       * memory in the system. */

>>>>> +     unsigned max_size;

>>>>> +

>>>>>  } odp_queue_capability_t;

>>>>>

>>>>>  /**

>>>>> @@ -124,6 +130,12 @@ typedef struct odp_queue_param_t {

>>>>>         * the queue type. */

>>>>>       odp_queue_type_t type;

>>>>>

>>>>> +     /** Queue size

>>>>> +       *

>>>>> +       * Maximum number of events in the queue. Value of 0 chooses

>>>>> the

>>>>> +       * default configuration of the implementation. */

>>>>> +     uint32_t size;

>>>>> +

>>>>>       /** Enqueue mode

>>>>>         *

>>>>>         * Default value for both queue types is ODP_QUEUE_OP_MT.

>>>>> Application

>>>>> diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-

>>>>> generic/odp_queue.c

>>>>> index fcf4bf5..5a50a57 100644

>>>>> --- a/platform/linux-generic/odp_queue.c

>>>>> +++ b/platform/linux-generic/odp_queue.c

>>>>> @@ -175,6 +175,7 @@ int odp_queue_capability(odp_queue_capability_t *capa)

>>>>>       capa->max_ordered_locks = sched_fn->max_ordered_locks();

>>>>>       capa->max_sched_groups  = sched_fn->num_grps();

>>>>>       capa->sched_prios       = odp_schedule_num_prio();

>>>>> +     capa->max_size          = 0;

>>>>>

>>>>>       return 0;

>>>>>  }

>>>>> --

>>>>> 2.7.4

>>>>
Bill Fischofer April 12, 2017, 12:32 p.m. UTC | #10
On Tue, Apr 11, 2017 at 11:14 PM, Honnappa Nagarahalli
<honnappa.nagarahalli@linaro.org> wrote:
> On 10 April 2017 at 11:49, Bill Fischofer <bill.fischofer@linaro.org> wrote:

>> On Mon, Apr 10, 2017 at 11:17 AM, Honnappa Nagarahalli

>> <honnappa.nagarahalli@linaro.org> wrote:

>>> Hi Bala,

>>>     Continuing the discussion from the call, as I mentioned in the

>>> call today, the queues need to hold all kinds of events and not just

>>> packets. The events need not be defined by ODP (like timeout events)

>>> also. The application may have its own events.

>>>

>>> In such a case, queue size does not dependent on the capacity of

>>> various pools supported in ODP. The size should depend on the

>>> implementation.

>>>

>>> If the queue is allocated out of memory, then the size should depend

>>> on the available amount of memory at any point in time.

>>

>> The real distinction is whether the implementation imposes a fixed

>> upper bound to queue size. This may be, for example, because queues

>> are implemented in hardware and there are hardware-imposed limits to

>> queue size. Or it may be that the implementation preallocates

>> resources at configuration time and these have fixed numbers

>> associated with them.

>>

>> ODP got along just fine before without having known queue sizes, so

>> the question is "what new information does the application want here"?

>> The only use-case we seem to have identified is the application

>> intends to use a queue as a storage mechanism and it wants to ensure

>> that the queue has a minimum storage capacity at queue create time. In

>> this case, the capability is reporting whether there is a fixed upper

>> bound that the application can use to compare to its minimum

>> requirements. If the answer is "yes", then the application can adjust

>> its needs (or refuse to run) based on that answer. If the answer is

>> "no" (a returned 0 as max_size) then the application can specify it's

>> requested size as input to odp_queue_create() and observe whether the

>> request succeeds or fails.

>>

>> The alternative is to not add a max_size to odp_queue_capability() at

>> all and simply let odp_queue_create() report success or failure in

>> response to a requested size. That's effectively what we have today,

>> except that by default the requested "size" is forced to 0, meaning

>> that the implementation chooses what, if any, such limits exist. The

>> only way these limits are surfaced to applications is if

>> odp_queue_enq() fails because the queue is full and the implementation

>> doesn't use back pressure. If back pressure is used, then the

>> odp_queue_enq() call simply stalls until room is made available in the

>> queue to satisfy the request.

>

> I prefer to leave the back pressure implementation to the application.

> This allows the application to have different algorithms to wait till

> the queue has space. For ex: on ARM platforms, the implementation can

> use WFE/SEV.


Back pressure is normally an implementation area because it is a
feature of hardware. If we expect applications to use
platform-specific features to implement this themselves, then this
defeats the portability goal of ODP. If the implementation doesn't use
back pressure, then odp_queue_enq() will fail when a queue is full and
applications can take whatever recovery action is appropriate, same as
the would on an allocation failure.

>

>>

>>>

>>> Thank you,

>>> Honnappa

>>>

>>> On 7 April 2017 at 02:54, Savolainen, Petri (Nokia - FI/Espoo)

>>> <petri.savolainen@nokia-bell-labs.com> wrote:

>>>> See my patch series: [PATCH v3 1/2] api: queue: added queue size param

>>>>

>>>>

>>>>> -----Original Message-----

>>>>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

>>>>> Honnappa Nagarahalli

>>>>> Sent: Friday, April 07, 2017 7:07 AM

>>>>> To: lng-odp@lists.linaro.org

>>>>> Subject: [lng-odp] [PATCH 1/2] add queue size param and related capability

>>>>>

>>>>> Added size parameter indicating the maximum number of events in the

>>>>> queue and the corresponding queue capability changes.

>>>>>

>>>>> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@linaro.org>

>>>>> ---

>>>>>  include/odp/api/spec/queue.h       | 12 ++++++++++++

>>>>>  platform/linux-generic/odp_queue.c |  1 +

>>>>>  2 files changed, 13 insertions(+)

>>>>>

>>>>> diff --git a/include/odp/api/spec/queue.h b/include/odp/api/spec/queue.h

>>>>> index 7972fea..ccb6fb8 100644

>>>>> --- a/include/odp/api/spec/queue.h

>>>>> +++ b/include/odp/api/spec/queue.h

>>>>> @@ -112,6 +112,12 @@ typedef struct odp_queue_capability_t {

>>>>>       /** Number of scheduling priorities */

>>>>>       unsigned sched_prios;

>>>>>

>>>>> +     /** Maximum number of events in the queue.

>>>>> +       *

>>>>> +       * Value of zero indicates the size is limited only by the

>>>>> available

>>>>> +       * memory in the system. */

>>>>> +     unsigned max_size;

>>>>> +

>>>>>  } odp_queue_capability_t;

>>>>>

>>>>>  /**

>>>>> @@ -124,6 +130,12 @@ typedef struct odp_queue_param_t {

>>>>>         * the queue type. */

>>>>>       odp_queue_type_t type;

>>>>>

>>>>> +     /** Queue size

>>>>> +       *

>>>>> +       * Maximum number of events in the queue. Value of 0 chooses

>>>>> the

>>>>> +       * default configuration of the implementation. */

>>>>> +     uint32_t size;

>>>>> +

>>>>>       /** Enqueue mode

>>>>>         *

>>>>>         * Default value for both queue types is ODP_QUEUE_OP_MT.

>>>>> Application

>>>>> diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-

>>>>> generic/odp_queue.c

>>>>> index fcf4bf5..5a50a57 100644

>>>>> --- a/platform/linux-generic/odp_queue.c

>>>>> +++ b/platform/linux-generic/odp_queue.c

>>>>> @@ -175,6 +175,7 @@ int odp_queue_capability(odp_queue_capability_t *capa)

>>>>>       capa->max_ordered_locks = sched_fn->max_ordered_locks();

>>>>>       capa->max_sched_groups  = sched_fn->num_grps();

>>>>>       capa->sched_prios       = odp_schedule_num_prio();

>>>>> +     capa->max_size          = 0;

>>>>>

>>>>>       return 0;

>>>>>  }

>>>>> --

>>>>> 2.7.4

>>>>
Honnappa Nagarahalli April 12, 2017, 5:28 p.m. UTC | #11
On 12 April 2017 at 00:45, Bala Manoharan <bala.manoharan@linaro.org> wrote:
> Regards,

> Bala

>

>

> On 12 April 2017 at 10:05, Honnappa Nagarahalli

> <honnappa.nagarahalli@linaro.org> wrote:

>> On 11 April 2017 at 05:51, Bala Manoharan <bala.manoharan@linaro.org> wrote:

>>> On 10 April 2017 at 21:47, Honnappa Nagarahalli

>>> <honnappa.nagarahalli@linaro.org> wrote:

>>>> Hi Bala,

>>>>     Continuing the discussion from the call, as I mentioned in the

>>>> call today, the queues need to hold all kinds of events and not just

>>>> packets. The events need not be defined by ODP (like timeout events)

>>>> also. The application may have its own events.

>>>>

>>>> In such a case, queue size does not dependent on the capacity of

>>>> various pools supported in ODP. The size should depend on the

>>>> implementation.

>>>

>>> ODP queues hold odp_event_t and these events needs to be allocated

>>> from a pool even in case of a timer.

>>

>> If I create a queue between two pipeline stages, the producer can

>> produce any 64b data and typecast it to odp_event_t and store it in

>> the queue. The consumer can dequeue from that queue and use that 64b

>> data according to the need of the application. It need not be

>> allocated from any pool.

>

> The idea of storing it in the queue is the concern there are

> implementations where queue is a virtual

> concept and even in the use-case you have defined the data has to be

> allocated from a memory region

> and the pointer is only handled by the queue in my implementation.

> so even if I return 0 for the capability it does not mean there is

> unlimited possibility,


Agree. There is no true unlimited possibility. I am fine to change it
to "Value of zero indicates the size is limited only by the available
resources in the system at the time of creation."

> there is a hidden limitation

> either by the pool or by any memory region.


In a software implementation of the pools, the size of the pool is
again dependent on the available resources in the system. The size of
the queue need not depend on the pool size. Application should decide
how that dependency should be - it might want to configure the pool
size depending on the queue size capability or vice versa.

>

>>

>> Also since in your case you are returning

>>> the maximum number of events across all the queues there needs to be a valid

>>> use-case for this value.

>>

>> The capability API would return the maximum size of a queue that the

>> application can create - if there is a fixed maximum queue size.

>> Otherwise, it would return 0, indicating that the max size is not

>> fixed - it is dependent on available resources at the time of

>> creation. One could create 10 queues of size 1K elements, but there

>> are not enough resources to create 11th queue with a size of 1K at

>> that point in time.

>>

>>>

>>> Regards,

>>> Bala

>>>

>>>>

>>>> If the queue is allocated out of memory, then the size should depend

>>>> on the available amount of memory at any point in time.

>>>>

>>>> Thank you,

>>>> Honnappa

>>>>

>>>> On 7 April 2017 at 02:54, Savolainen, Petri (Nokia - FI/Espoo)

>>>> <petri.savolainen@nokia-bell-labs.com> wrote:

>>>>> See my patch series: [PATCH v3 1/2] api: queue: added queue size param

>>>>>

>>>>>

>>>>>> -----Original Message-----

>>>>>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

>>>>>> Honnappa Nagarahalli

>>>>>> Sent: Friday, April 07, 2017 7:07 AM

>>>>>> To: lng-odp@lists.linaro.org

>>>>>> Subject: [lng-odp] [PATCH 1/2] add queue size param and related capability

>>>>>>

>>>>>> Added size parameter indicating the maximum number of events in the

>>>>>> queue and the corresponding queue capability changes.

>>>>>>

>>>>>> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@linaro.org>

>>>>>> ---

>>>>>>  include/odp/api/spec/queue.h       | 12 ++++++++++++

>>>>>>  platform/linux-generic/odp_queue.c |  1 +

>>>>>>  2 files changed, 13 insertions(+)

>>>>>>

>>>>>> diff --git a/include/odp/api/spec/queue.h b/include/odp/api/spec/queue.h

>>>>>> index 7972fea..ccb6fb8 100644

>>>>>> --- a/include/odp/api/spec/queue.h

>>>>>> +++ b/include/odp/api/spec/queue.h

>>>>>> @@ -112,6 +112,12 @@ typedef struct odp_queue_capability_t {

>>>>>>       /** Number of scheduling priorities */

>>>>>>       unsigned sched_prios;

>>>>>>

>>>>>> +     /** Maximum number of events in the queue.

>>>>>> +       *

>>>>>> +       * Value of zero indicates the size is limited only by the

>>>>>> available

>>>>>> +       * memory in the system. */

>>>>>> +     unsigned max_size;

>>>>>> +

>>>>>>  } odp_queue_capability_t;

>>>>>>

>>>>>>  /**

>>>>>> @@ -124,6 +130,12 @@ typedef struct odp_queue_param_t {

>>>>>>         * the queue type. */

>>>>>>       odp_queue_type_t type;

>>>>>>

>>>>>> +     /** Queue size

>>>>>> +       *

>>>>>> +       * Maximum number of events in the queue. Value of 0 chooses

>>>>>> the

>>>>>> +       * default configuration of the implementation. */

>>>>>> +     uint32_t size;

>>>>>> +

>>>>>>       /** Enqueue mode

>>>>>>         *

>>>>>>         * Default value for both queue types is ODP_QUEUE_OP_MT.

>>>>>> Application

>>>>>> diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-

>>>>>> generic/odp_queue.c

>>>>>> index fcf4bf5..5a50a57 100644

>>>>>> --- a/platform/linux-generic/odp_queue.c

>>>>>> +++ b/platform/linux-generic/odp_queue.c

>>>>>> @@ -175,6 +175,7 @@ int odp_queue_capability(odp_queue_capability_t *capa)

>>>>>>       capa->max_ordered_locks = sched_fn->max_ordered_locks();

>>>>>>       capa->max_sched_groups  = sched_fn->num_grps();

>>>>>>       capa->sched_prios       = odp_schedule_num_prio();

>>>>>> +     capa->max_size          = 0;

>>>>>>

>>>>>>       return 0;

>>>>>>  }

>>>>>> --

>>>>>> 2.7.4

>>>>>
Bill Fischofer April 12, 2017, 5:51 p.m. UTC | #12
On Wed, Apr 12, 2017 at 12:28 PM, Honnappa Nagarahalli
<honnappa.nagarahalli@linaro.org> wrote:
> On 12 April 2017 at 00:45, Bala Manoharan <bala.manoharan@linaro.org> wrote:

>> Regards,

>> Bala

>>

>>

>> On 12 April 2017 at 10:05, Honnappa Nagarahalli

>> <honnappa.nagarahalli@linaro.org> wrote:

>>> On 11 April 2017 at 05:51, Bala Manoharan <bala.manoharan@linaro.org> wrote:

>>>> On 10 April 2017 at 21:47, Honnappa Nagarahalli

>>>> <honnappa.nagarahalli@linaro.org> wrote:

>>>>> Hi Bala,

>>>>>     Continuing the discussion from the call, as I mentioned in the

>>>>> call today, the queues need to hold all kinds of events and not just

>>>>> packets. The events need not be defined by ODP (like timeout events)

>>>>> also. The application may have its own events.

>>>>>

>>>>> In such a case, queue size does not dependent on the capacity of

>>>>> various pools supported in ODP. The size should depend on the

>>>>> implementation.

>>>>

>>>> ODP queues hold odp_event_t and these events needs to be allocated

>>>> from a pool even in case of a timer.

>>>

>>> If I create a queue between two pipeline stages, the producer can

>>> produce any 64b data and typecast it to odp_event_t and store it in

>>> the queue. The consumer can dequeue from that queue and use that 64b

>>> data according to the need of the application. It need not be

>>> allocated from any pool.

>>

>> The idea of storing it in the queue is the concern there are

>> implementations where queue is a virtual

>> concept and even in the use-case you have defined the data has to be

>> allocated from a memory region

>> and the pointer is only handled by the queue in my implementation.

>> so even if I return 0 for the capability it does not mean there is

>> unlimited possibility,

>

> Agree. There is no true unlimited possibility. I am fine to change it

> to "Value of zero indicates the size is limited only by the available

> resources in the system at the time of creation."


Petri's argument earlier today to me seemed convincing that this is
not a useful piece of information for applications looking to check if
they have sufficient resources available to operate successfully. What
the application wants to know is "what is the minimum number of queues
this ODP instance is guaranteed to be able to create"?

Please see my earlier comments at
https://lists.linaro.org/pipermail/lng-odp/2017-April/029844.html

I'd appreciate your thoughts on this.

>

>> there is a hidden limitation

>> either by the pool or by any memory region.

>

> In a software implementation of the pools, the size of the pool is

> again dependent on the available resources in the system. The size of

> the queue need not depend on the pool size. Application should decide

> how that dependency should be - it might want to configure the pool

> size depending on the queue size capability or vice versa.

>

>>

>>>

>>> Also since in your case you are returning

>>>> the maximum number of events across all the queues there needs to be a valid

>>>> use-case for this value.

>>>

>>> The capability API would return the maximum size of a queue that the

>>> application can create - if there is a fixed maximum queue size.

>>> Otherwise, it would return 0, indicating that the max size is not

>>> fixed - it is dependent on available resources at the time of

>>> creation. One could create 10 queues of size 1K elements, but there

>>> are not enough resources to create 11th queue with a size of 1K at

>>> that point in time.

>>>

>>>>

>>>> Regards,

>>>> Bala

>>>>

>>>>>

>>>>> If the queue is allocated out of memory, then the size should depend

>>>>> on the available amount of memory at any point in time.

>>>>>

>>>>> Thank you,

>>>>> Honnappa

>>>>>

>>>>> On 7 April 2017 at 02:54, Savolainen, Petri (Nokia - FI/Espoo)

>>>>> <petri.savolainen@nokia-bell-labs.com> wrote:

>>>>>> See my patch series: [PATCH v3 1/2] api: queue: added queue size param

>>>>>>

>>>>>>

>>>>>>> -----Original Message-----

>>>>>>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

>>>>>>> Honnappa Nagarahalli

>>>>>>> Sent: Friday, April 07, 2017 7:07 AM

>>>>>>> To: lng-odp@lists.linaro.org

>>>>>>> Subject: [lng-odp] [PATCH 1/2] add queue size param and related capability

>>>>>>>

>>>>>>> Added size parameter indicating the maximum number of events in the

>>>>>>> queue and the corresponding queue capability changes.

>>>>>>>

>>>>>>> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@linaro.org>

>>>>>>> ---

>>>>>>>  include/odp/api/spec/queue.h       | 12 ++++++++++++

>>>>>>>  platform/linux-generic/odp_queue.c |  1 +

>>>>>>>  2 files changed, 13 insertions(+)

>>>>>>>

>>>>>>> diff --git a/include/odp/api/spec/queue.h b/include/odp/api/spec/queue.h

>>>>>>> index 7972fea..ccb6fb8 100644

>>>>>>> --- a/include/odp/api/spec/queue.h

>>>>>>> +++ b/include/odp/api/spec/queue.h

>>>>>>> @@ -112,6 +112,12 @@ typedef struct odp_queue_capability_t {

>>>>>>>       /** Number of scheduling priorities */

>>>>>>>       unsigned sched_prios;

>>>>>>>

>>>>>>> +     /** Maximum number of events in the queue.

>>>>>>> +       *

>>>>>>> +       * Value of zero indicates the size is limited only by the

>>>>>>> available

>>>>>>> +       * memory in the system. */

>>>>>>> +     unsigned max_size;

>>>>>>> +

>>>>>>>  } odp_queue_capability_t;

>>>>>>>

>>>>>>>  /**

>>>>>>> @@ -124,6 +130,12 @@ typedef struct odp_queue_param_t {

>>>>>>>         * the queue type. */

>>>>>>>       odp_queue_type_t type;

>>>>>>>

>>>>>>> +     /** Queue size

>>>>>>> +       *

>>>>>>> +       * Maximum number of events in the queue. Value of 0 chooses

>>>>>>> the

>>>>>>> +       * default configuration of the implementation. */

>>>>>>> +     uint32_t size;

>>>>>>> +

>>>>>>>       /** Enqueue mode

>>>>>>>         *

>>>>>>>         * Default value for both queue types is ODP_QUEUE_OP_MT.

>>>>>>> Application

>>>>>>> diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-

>>>>>>> generic/odp_queue.c

>>>>>>> index fcf4bf5..5a50a57 100644

>>>>>>> --- a/platform/linux-generic/odp_queue.c

>>>>>>> +++ b/platform/linux-generic/odp_queue.c

>>>>>>> @@ -175,6 +175,7 @@ int odp_queue_capability(odp_queue_capability_t *capa)

>>>>>>>       capa->max_ordered_locks = sched_fn->max_ordered_locks();

>>>>>>>       capa->max_sched_groups  = sched_fn->num_grps();

>>>>>>>       capa->sched_prios       = odp_schedule_num_prio();

>>>>>>> +     capa->max_size          = 0;

>>>>>>>

>>>>>>>       return 0;

>>>>>>>  }

>>>>>>> --

>>>>>>> 2.7.4

>>>>>>
diff mbox series

Patch

diff --git a/include/odp/api/spec/queue.h b/include/odp/api/spec/queue.h
index 7972fea..ccb6fb8 100644
--- a/include/odp/api/spec/queue.h
+++ b/include/odp/api/spec/queue.h
@@ -112,6 +112,12 @@  typedef struct odp_queue_capability_t {
 	/** Number of scheduling priorities */
 	unsigned sched_prios;
 
+	/** Maximum number of events in the queue.
+	  *
+	  * Value of zero indicates the size is limited only by the available
+	  * memory in the system. */
+	unsigned max_size;
+
 } odp_queue_capability_t;
 
 /**
@@ -124,6 +130,12 @@  typedef struct odp_queue_param_t {
 	  * the queue type. */
 	odp_queue_type_t type;
 
+	/** Queue size
+	  *
+	  * Maximum number of events in the queue. Value of 0 chooses the
+	  * default configuration of the implementation. */
+	uint32_t size;
+
 	/** Enqueue mode
 	  *
 	  * Default value for both queue types is ODP_QUEUE_OP_MT. Application
diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-generic/odp_queue.c
index fcf4bf5..5a50a57 100644
--- a/platform/linux-generic/odp_queue.c
+++ b/platform/linux-generic/odp_queue.c
@@ -175,6 +175,7 @@  int odp_queue_capability(odp_queue_capability_t *capa)
 	capa->max_ordered_locks = sched_fn->max_ordered_locks();
 	capa->max_sched_groups  = sched_fn->num_grps();
 	capa->sched_prios       = odp_schedule_num_prio();
+	capa->max_size          = 0;
 
 	return 0;
 }