[dpdk-dev,v6,01/21] eventdev: improve API docs for start function

Message ID 1490829963-106807-2-git-send-email-harry.van.haaren@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Jerin Jacob
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Van Haaren, Harry March 29, 2017, 11:25 p.m. UTC
  This commit documents two error return values for the
rte_event_dev_start() function.

-EINVAL  indicates not all ports are configured
-EDEADLK indicates that not all queues are linked to ports. If an
         application enqueues to such a queue it can lead to deadlock

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
---
 lib/librte_eventdev/rte_eventdev.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
  

Comments

Anatoly Burakov March 30, 2017, 10:56 a.m. UTC | #1
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Harry van Haaren
> Sent: Thursday, March 30, 2017 12:26 AM
> To: dev@dpdk.org
> Cc: jerin.jacob@caviumnetworks.com; Van Haaren, Harry
> <harry.van.haaren@intel.com>
> Subject: [dpdk-dev] [PATCH v6 01/21] eventdev: improve API docs for start
> function
> 
> This commit documents two error return values for the
> rte_event_dev_start() function.
> 
> -EINVAL  indicates not all ports are configured
> -EDEADLK indicates that not all queues are linked to ports. If an
>          application enqueues to such a queue it can lead to deadlock
> 
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>

Acked-by: Anatoly  Burakov <anatoly.burakov@intel.com>
  
Jerin Jacob March 30, 2017, 5:11 p.m. UTC | #2
On Thu, Mar 30, 2017 at 12:25:43AM +0100, Harry van Haaren wrote:
> This commit documents two error return values for the
> rte_event_dev_start() function.
> 
> -EINVAL  indicates not all ports are configured

-EINVAL returns in case of an invalid dev_id. How about -ESTALE  or
something like that?

> -EDEADLK indicates that not all queues are linked to ports. If an
>          application enqueues to such a queue it can lead to deadlock

IMO, Deadlock is an implementation detail all the PMD may not result in
deadlock. How about -ENOLINK ?

IMO, If you want to enforce this rule then the detection and
check has to be be in common code to avoid all PMD duplicating the same
code.

> 
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> ---
>  lib/librte_eventdev/rte_eventdev.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
> index 9971937..dc8dacb 100644
> --- a/lib/librte_eventdev/rte_eventdev.h
> +++ b/lib/librte_eventdev/rte_eventdev.h
> @@ -757,7 +757,8 @@ rte_event_port_count(uint8_t dev_id);
>   *   Event device identifier
>   * @return
>   *   - 0: Success, device started.
> - *   - <0: Error code of the driver device start function.
> + *   - -EINVAL : Not all ports of the device are configured
> + *   - -EDEADLK: Not all queues are linked, which could lead to deadlock.
>   */
>  int
>  rte_event_dev_start(uint8_t dev_id);
> -- 
> 2.7.4
>
  
Van Haaren, Harry March 30, 2017, 5:24 p.m. UTC | #3
Sure, will send an updated patch tomorrow, thanks!


> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Thursday, March 30, 2017 6:12 PM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v6 01/21] eventdev: improve API docs for start function
> 
> On Thu, Mar 30, 2017 at 12:25:43AM +0100, Harry van Haaren wrote:
> > This commit documents two error return values for the
> > rte_event_dev_start() function.
> >
> > -EINVAL  indicates not all ports are configured
> 
> -EINVAL returns in case of an invalid dev_id. How about -ESTALE  or
> something like that?
> 
> > -EDEADLK indicates that not all queues are linked to ports. If an
> >          application enqueues to such a queue it can lead to deadlock
> 
> IMO, Deadlock is an implementation detail all the PMD may not result in
> deadlock. How about -ENOLINK ?
> 
> IMO, If you want to enforce this rule then the detection and
> check has to be be in common code to avoid all PMD duplicating the same
> code.
> 
> >
> > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> > ---
> >  lib/librte_eventdev/rte_eventdev.h | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
> > index 9971937..dc8dacb 100644
> > --- a/lib/librte_eventdev/rte_eventdev.h
> > +++ b/lib/librte_eventdev/rte_eventdev.h
> > @@ -757,7 +757,8 @@ rte_event_port_count(uint8_t dev_id);
> >   *   Event device identifier
> >   * @return
> >   *   - 0: Success, device started.
> > - *   - <0: Error code of the driver device start function.
> > + *   - -EINVAL : Not all ports of the device are configured
> > + *   - -EDEADLK: Not all queues are linked, which could lead to deadlock.
> >   */
> >  int
> >  rte_event_dev_start(uint8_t dev_id);
> > --
> > 2.7.4
> >
  

Patch

diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
index 9971937..dc8dacb 100644
--- a/lib/librte_eventdev/rte_eventdev.h
+++ b/lib/librte_eventdev/rte_eventdev.h
@@ -757,7 +757,8 @@  rte_event_port_count(uint8_t dev_id);
  *   Event device identifier
  * @return
  *   - 0: Success, device started.
- *   - <0: Error code of the driver device start function.
+ *   - -EINVAL : Not all ports of the device are configured
+ *   - -EDEADLK: Not all queues are linked, which could lead to deadlock.
  */
 int
 rte_event_dev_start(uint8_t dev_id);