diff mbox

[1/2] api: queue: add odp_queue_destroy()

Message ID 1417021388-15432-1-git-send-email-taras.kondratiuk@linaro.org
State New
Headers show

Commit Message

Taras Kondratiuk Nov. 26, 2014, 5:03 p.m. UTC
Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org>
---
 platform/linux-generic/include/api/odp_queue.h |    9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Mike Holmes Nov. 26, 2014, 5:11 p.m. UTC | #1
On 26 November 2014 at 12:03, Taras Kondratiuk <taras.kondratiuk@linaro.org>
wrote:

> Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org>
> ---
>  platform/linux-generic/include/api/odp_queue.h |    9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/platform/linux-generic/include/api/odp_queue.h
> b/platform/linux-generic/include/api/odp_queue.h
> index b8ac4bb..3321950 100644
> --- a/platform/linux-generic/include/api/odp_queue.h
> +++ b/platform/linux-generic/include/api/odp_queue.h
> @@ -124,6 +124,15 @@ odp_queue_t odp_queue_create(const char *name,
> odp_queue_type_t type,
>                              odp_queue_param_t *param);
>
>  /**
> + * Destroy ODP queue
> + *
>

Does it need the queue to be drained, will it drain the queue, will if free
buffers if it drains the queue.
does it wait if the queue is full
Are there any @warnings about its behaviour?


> + * @param queue    Queue handle
>

in or out ?


> + *
> + * @return 0 if successful
>

what cases cause a failure, what is returned value in those case?


> + */
> +int odp_queue_destroy(odp_queue_t queue);
> +
> +/**
>   * Find a queue by name
>   *
>   * @param name    Queue name
> --
> 1.7.9.5
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
Taras Kondratiuk Nov. 27, 2014, 9:35 a.m. UTC | #2
On 11/26/2014 07:11 PM, Mike Holmes wrote:
> 
> 
> On 26 November 2014 at 12:03, Taras Kondratiuk
> <taras.kondratiuk@linaro.org <mailto:taras.kondratiuk@linaro.org>> wrote:
> 
>     Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org
>     <mailto:taras.kondratiuk@linaro.org>>
>     ---
>      platform/linux-generic/include/api/odp_queue.h |    9 +++++++++
>      1 file changed, 9 insertions(+)
> 
>     diff --git a/platform/linux-generic/include/api/odp_queue.h
>     b/platform/linux-generic/include/api/odp_queue.h
>     index b8ac4bb..3321950 100644
>     --- a/platform/linux-generic/include/api/odp_queue.h
>     +++ b/platform/linux-generic/include/api/odp_queue.h
>     @@ -124,6 +124,15 @@ odp_queue_t odp_queue_create(const char *name,
>     odp_queue_type_t type,
>                                  odp_queue_param_t *param);
> 
>      /**
>     + * Destroy ODP queue
>     + *
> 
> 
> Does it need the queue to be drained, will it drain the queue, will if
> free buffers if it drains the queue.
> does it wait if the queue is full
> Are there any @warnings about its behaviour?

For v1.0 this function can have minimal functionality. Later we can
extend it to ensure correct teardown sequence and simplify operations
needed from application side.

/**
 * Destroy ODP queue
 *
 * Destroys ODP queue. The queue must be empty and detached from other
 * ODP API (crypto, pktio, etc). Application must ensure that no
 * other operations on this queue are invoked in parallel. Otherwise
 * behavior is undefined.
 */

>   Otherwise behavior is undefined.
> 
>     + * @param queue    Queue handle
> 
> 
> in or out ?

Will fix to [in]

>  
> 
>     + *
>     + * @return 0 if successful
> 
> 
> what cases cause a failure, what is returned value in those case?

I've added requirements to the description. Violating the requirements
will cause a failure. Some failure cases can be platform dependent. Do
you think there is a reason to enumerate all possible error cases?
Bill Fischofer Nov. 28, 2014, 4:37 a.m. UTC | #3
You cannot gracefully destroy a queue without some sort of quiesce function
that prohibits further enqueues to the queue while allowing items on the
queue to be dequeued until the queue is empty.  To position for this, I'd
include a check that rejects the destroy attempt if the queue is not
empty.  It's then up to the caller to ensure that the queue is empty by its
own means for v1.0.

We have the same issue in buffer pools.  We have an
odp_buffer_pool_destroy() but unless the application can somehow ensure
that the pool is empty the call will fail.  We can talk about adding a pool
quiesce function to facilitate this post-v1.0.

Bill

On Thu, Nov 27, 2014 at 4:35 AM, Taras Kondratiuk <
taras.kondratiuk@linaro.org> wrote:

> On 11/26/2014 07:11 PM, Mike Holmes wrote:
> >
> >
> > On 26 November 2014 at 12:03, Taras Kondratiuk
> > <taras.kondratiuk@linaro.org <mailto:taras.kondratiuk@linaro.org>>
> wrote:
> >
> >     Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org
> >     <mailto:taras.kondratiuk@linaro.org>>
> >     ---
> >      platform/linux-generic/include/api/odp_queue.h |    9 +++++++++
> >      1 file changed, 9 insertions(+)
> >
> >     diff --git a/platform/linux-generic/include/api/odp_queue.h
> >     b/platform/linux-generic/include/api/odp_queue.h
> >     index b8ac4bb..3321950 100644
> >     --- a/platform/linux-generic/include/api/odp_queue.h
> >     +++ b/platform/linux-generic/include/api/odp_queue.h
> >     @@ -124,6 +124,15 @@ odp_queue_t odp_queue_create(const char *name,
> >     odp_queue_type_t type,
> >                                  odp_queue_param_t *param);
> >
> >      /**
> >     + * Destroy ODP queue
> >     + *
> >
> >
> > Does it need the queue to be drained, will it drain the queue, will if
> > free buffers if it drains the queue.
> > does it wait if the queue is full
> > Are there any @warnings about its behaviour?
>
> For v1.0 this function can have minimal functionality. Later we can
> extend it to ensure correct teardown sequence and simplify operations
> needed from application side.
>
> /**
>  * Destroy ODP queue
>  *
>  * Destroys ODP queue. The queue must be empty and detached from other
>  * ODP API (crypto, pktio, etc). Application must ensure that no
>  * other operations on this queue are invoked in parallel. Otherwise
>  * behavior is undefined.
>  */
>
> >   Otherwise behavior is undefined.
> >
> >     + * @param queue    Queue handle
> >
> >
> > in or out ?
>
> Will fix to [in]
>
> >
> >
> >     + *
> >     + * @return 0 if successful
> >
> >
> > what cases cause a failure, what is returned value in those case?
>
> I've added requirements to the description. Violating the requirements
> will cause a failure. Some failure cases can be platform dependent. Do
> you think there is a reason to enumerate all possible error cases?
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
Taras Kondratiuk Nov. 28, 2014, 11 a.m. UTC | #4
On 11/28/2014 06:37 AM, Bill Fischofer wrote:
> You cannot gracefully destroy a queue without some sort of quiesce
> function that prohibits further enqueues to the queue while allowing
> items on the queue to be dequeued until the queue is empty.  To position
> for this, I'd include a check that rejects the destroy attempt if the
> queue is not empty.  It's then up to the caller to ensure that the queue
> is empty by its own means for v1.0.

There is an issue with quiesce functions. If queue is used by some HW
block like pktio or crypto in may not be possible to prevent enqueues.
At least on Keystone queue should be detached from HW first (i.e.
replaced by another one). This should be coordinated from application
side.

I've mentioned in the description that behavior is undefined if queue
is not empty or used by some HW block.

>
> We have the same issue in buffer pools.  We have an
> odp_buffer_pool_destroy() but unless the application can somehow ensure
> that the pool is empty the call will fail.  We can talk about adding a
> pool quiesce function to facilitate this post-v1.0.
>
diff mbox

Patch

diff --git a/platform/linux-generic/include/api/odp_queue.h b/platform/linux-generic/include/api/odp_queue.h
index b8ac4bb..3321950 100644
--- a/platform/linux-generic/include/api/odp_queue.h
+++ b/platform/linux-generic/include/api/odp_queue.h
@@ -124,6 +124,15 @@  odp_queue_t odp_queue_create(const char *name, odp_queue_type_t type,
 			     odp_queue_param_t *param);
 
 /**
+ * Destroy ODP queue
+ *
+ * @param queue    Queue handle
+ *
+ * @return 0 if successful
+ */
+int odp_queue_destroy(odp_queue_t queue);
+
+/**
  * Find a queue by name
  *
  * @param name    Queue name