diff mbox series

[v5,7/7] app/bbdev: update offload test to dequeue full ring

Message ID 1603496581-35966-8-git-send-email-nicolas.chautru@intel.com (mailing list archive)
State Superseded, archived
Delegated to: akhil goyal
Headers
Series BBDEV test updates |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-broadcom-Performance fail Performance Testing issues
ci/iol-broadcom-Functional fail Functional Testing issues
ci/iol-testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-intel-Functional fail Functional Testing issues
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

Chautru, Nicolas Oct. 23, 2020, 11:43 p.m. UTC 
  update offload dequeue to retrieve the full ring to be
agnostic of implementation.

Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
Acked-by: Aidan Goddard <aidan.goddard@accelercomm.com>
Acked-by: Dave Burley <dave.burley@accelercomm.com>
---
 app/test-bbdev/test_bbdev_perf.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Comments

Tom Rix Oct. 26, 2020, 1:55 p.m. UTC | #1 
On 10/23/20 4:43 PM, Nicolas Chautru wrote:
> update offload dequeue to retrieve the full ring to be
> agnostic of implementation.
>
> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> Acked-by: Aidan Goddard <aidan.goddard@accelercomm.com>
> Acked-by: Dave Burley <dave.burley@accelercomm.com>
> ---
>  app/test-bbdev/test_bbdev_perf.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
> index b5dc536..a6884c5 100644
> --- a/app/test-bbdev/test_bbdev_perf.c
> +++ b/app/test-bbdev/test_bbdev_perf.c
> @@ -4463,8 +4463,8 @@ typedef int (test_case_function)(struct active_device *ad,
>  		/* Dequeue one operation */
This comment and similar need to change, not doing just 1 anymore
>  		do {
>  			deq += rte_bbdev_dequeue_dec_ops(dev_id, queue_id,
> -					&ops_deq[deq], 1);
> -		} while (unlikely(deq != 1));
> +					&ops_deq[deq], enq);
> +		} while (unlikely(deq == 0));

This check looks wrong, should likely be (deq != enq)

Similar below

Tom

>  
>  		deq_last_time = rte_rdtsc_precise() - deq_start_time;
>  		time_st->deq_max_time = RTE_MAX(time_st->deq_max_time,
> @@ -4554,8 +4554,8 @@ typedef int (test_case_function)(struct active_device *ad,
>  		/* Dequeue one operation */
>  		do {
>  			deq += rte_bbdev_dequeue_ldpc_dec_ops(dev_id, queue_id,
> -					&ops_deq[deq], 1);
> -		} while (unlikely(deq != 1));
> +					&ops_deq[deq], enq);
> +		} while (unlikely(deq == 0));
>  
>  		deq_last_time = rte_rdtsc_precise() - deq_start_time;
>  		time_st->deq_max_time = RTE_MAX(time_st->deq_max_time,
> @@ -4642,8 +4642,8 @@ typedef int (test_case_function)(struct active_device *ad,
>  		/* Dequeue one operation */
>  		do {
>  			deq += rte_bbdev_dequeue_enc_ops(dev_id, queue_id,
> -					&ops_deq[deq], 1);
> -		} while (unlikely(deq != 1));
> +					&ops_deq[deq], enq);
> +		} while (unlikely(deq == 0));
>  
>  		deq_last_time = rte_rdtsc_precise() - deq_start_time;
>  		time_st->deq_max_time = RTE_MAX(time_st->deq_max_time,
> @@ -4725,8 +4725,8 @@ typedef int (test_case_function)(struct active_device *ad,
>  		/* Dequeue one operation */
>  		do {
>  			deq += rte_bbdev_dequeue_ldpc_enc_ops(dev_id, queue_id,
> -					&ops_deq[deq], 1);
> -		} while (unlikely(deq != 1));
> +					&ops_deq[deq], enq);
> +		} while (unlikely(deq == 0));
>  
>  		deq_last_time = rte_rdtsc_precise() - deq_start_time;
>  		time_st->deq_max_time = RTE_MAX(time_st->deq_max_time,
Chautru, Nicolas Oct. 26, 2020, 4:27 p.m. UTC | #2 
> -----Original Message-----
> From: Tom Rix <trix@redhat.com>
> Sent: Monday, October 26, 2020 6:56 AM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
> akhil.goyal@nxp.com
> Cc: david.marchand@redhat.com
> Subject: Re: [PATCH v5 7/7] app/bbdev: update offload test to dequeue full
> ring
> 
> 
> On 10/23/20 4:43 PM, Nicolas Chautru wrote:
> > update offload dequeue to retrieve the full ring to be agnostic of
> > implementation.
> >
> > Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> > Acked-by: Aidan Goddard <aidan.goddard@accelercomm.com>
> > Acked-by: Dave Burley <dave.burley@accelercomm.com>
> > ---
> >  app/test-bbdev/test_bbdev_perf.c | 16 ++++++++--------
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/app/test-bbdev/test_bbdev_perf.c
> > b/app/test-bbdev/test_bbdev_perf.c
> > index b5dc536..a6884c5 100644
> > --- a/app/test-bbdev/test_bbdev_perf.c
> > +++ b/app/test-bbdev/test_bbdev_perf.c
> > @@ -4463,8 +4463,8 @@ typedef int (test_case_function)(struct
> active_device *ad,
> >  		/* Dequeue one operation */
> This comment and similar need to change, not doing just 1 anymore

We still just need one operation dequeued to be considered done. 

> >  		do {
> >  			deq += rte_bbdev_dequeue_dec_ops(dev_id,
> queue_id,
> > -					&ops_deq[deq], 1);
> > -		} while (unlikely(deq != 1));
> > +					&ops_deq[deq], enq);
> > +		} while (unlikely(deq == 0));
> 
> This check looks wrong, should likely be (deq != enq)
> 
> Similar below

No that is intentional. Not waiting for everything to be done but purely the first dequeue. If not this would be run multiple times.
The rest of ring is dequeued below. 

> 
> Tom
> 
> >
> >  		deq_last_time = rte_rdtsc_precise() - deq_start_time;
> >  		time_st->deq_max_time = RTE_MAX(time_st-
> >deq_max_time, @@ -4554,8
> > +4554,8 @@ typedef int (test_case_function)(struct active_device *ad,
> >  		/* Dequeue one operation */
> >  		do {
> >  			deq += rte_bbdev_dequeue_ldpc_dec_ops(dev_id,
> queue_id,
> > -					&ops_deq[deq], 1);
> > -		} while (unlikely(deq != 1));
> > +					&ops_deq[deq], enq);
> > +		} while (unlikely(deq == 0));
> >
> >  		deq_last_time = rte_rdtsc_precise() - deq_start_time;
> >  		time_st->deq_max_time = RTE_MAX(time_st-
> >deq_max_time, @@ -4642,8
> > +4642,8 @@ typedef int (test_case_function)(struct active_device *ad,
> >  		/* Dequeue one operation */
> >  		do {
> >  			deq += rte_bbdev_dequeue_enc_ops(dev_id,
> queue_id,
> > -					&ops_deq[deq], 1);
> > -		} while (unlikely(deq != 1));
> > +					&ops_deq[deq], enq);
> > +		} while (unlikely(deq == 0));
> >
> >  		deq_last_time = rte_rdtsc_precise() - deq_start_time;
> >  		time_st->deq_max_time = RTE_MAX(time_st-
> >deq_max_time, @@ -4725,8
> > +4725,8 @@ typedef int (test_case_function)(struct active_device *ad,
> >  		/* Dequeue one operation */
> >  		do {
> >  			deq += rte_bbdev_dequeue_ldpc_enc_ops(dev_id,
> queue_id,
> > -					&ops_deq[deq], 1);
> > -		} while (unlikely(deq != 1));
> > +					&ops_deq[deq], enq);
> > +		} while (unlikely(deq == 0));
> >
> >  		deq_last_time = rte_rdtsc_precise() - deq_start_time;
> >  		time_st->deq_max_time = RTE_MAX(time_st-
> >deq_max_time,
Tom Rix Oct. 28, 2020, 8:28 p.m. UTC | #3 
On 10/26/20 9:27 AM, Chautru, Nicolas wrote:
>> -----Original Message-----
>> From: Tom Rix <trix@redhat.com>
>> Sent: Monday, October 26, 2020 6:56 AM
>> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
>> akhil.goyal@nxp.com
>> Cc: david.marchand@redhat.com
>> Subject: Re: [PATCH v5 7/7] app/bbdev: update offload test to dequeue full
>> ring
>>
>>
>> On 10/23/20 4:43 PM, Nicolas Chautru wrote:
>>> update offload dequeue to retrieve the full ring to be agnostic of
>>> implementation.
>>>
>>> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
>>> Acked-by: Aidan Goddard <aidan.goddard@accelercomm.com>
>>> Acked-by: Dave Burley <dave.burley@accelercomm.com>
>>> ---
>>>  app/test-bbdev/test_bbdev_perf.c | 16 ++++++++--------
>>>  1 file changed, 8 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/app/test-bbdev/test_bbdev_perf.c
>>> b/app/test-bbdev/test_bbdev_perf.c
>>> index b5dc536..a6884c5 100644
>>> --- a/app/test-bbdev/test_bbdev_perf.c
>>> +++ b/app/test-bbdev/test_bbdev_perf.c
>>> @@ -4463,8 +4463,8 @@ typedef int (test_case_function)(struct
>> active_device *ad,
>>>  		/* Dequeue one operation */
>> This comment and similar need to change, not doing just 1 anymore
> We still just need one operation dequeued to be considered done. 
>
>>>  		do {
>>>  			deq += rte_bbdev_dequeue_dec_ops(dev_id,
>> queue_id,
>>> -					&ops_deq[deq], 1);
>>> -		} while (unlikely(deq != 1));
>>> +					&ops_deq[deq], enq);
>>> +		} while (unlikely(deq == 0));
>> This check looks wrong, should likely be (deq != enq)
>>
>> Similar below
> No that is intentional. Not waiting for everything to be done but purely the first dequeue. If not this would be run multiple times.
> The rest of ring is dequeued below. 

So is > 1 an error condition or ok?

Maybe add a comment that it is really ok because the call logic is not setup for 1 but for enq

Tom

>
>> Tom
>>
>>>  		deq_last_time = rte_rdtsc_precise() - deq_start_time;
>>>  		time_st->deq_max_time = RTE_MAX(time_st-
>>> deq_max_time, @@ -4554,8
>>> +4554,8 @@ typedef int (test_case_function)(struct active_device *ad,
>>>  		/* Dequeue one operation */
>>>  		do {
>>>  			deq += rte_bbdev_dequeue_ldpc_dec_ops(dev_id,
>> queue_id,
>>> -					&ops_deq[deq], 1);
>>> -		} while (unlikely(deq != 1));
>>> +					&ops_deq[deq], enq);
>>> +		} while (unlikely(deq == 0));
>>>
>>>  		deq_last_time = rte_rdtsc_precise() - deq_start_time;
>>>  		time_st->deq_max_time = RTE_MAX(time_st-
>>> deq_max_time, @@ -4642,8
>>> +4642,8 @@ typedef int (test_case_function)(struct active_device *ad,
>>>  		/* Dequeue one operation */
>>>  		do {
>>>  			deq += rte_bbdev_dequeue_enc_ops(dev_id,
>> queue_id,
>>> -					&ops_deq[deq], 1);
>>> -		} while (unlikely(deq != 1));
>>> +					&ops_deq[deq], enq);
>>> +		} while (unlikely(deq == 0));
>>>
>>>  		deq_last_time = rte_rdtsc_precise() - deq_start_time;
>>>  		time_st->deq_max_time = RTE_MAX(time_st-
>>> deq_max_time, @@ -4725,8
>>> +4725,8 @@ typedef int (test_case_function)(struct active_device *ad,
>>>  		/* Dequeue one operation */
>>>  		do {
>>>  			deq += rte_bbdev_dequeue_ldpc_enc_ops(dev_id,
>> queue_id,
>>> -					&ops_deq[deq], 1);
>>> -		} while (unlikely(deq != 1));
>>> +					&ops_deq[deq], enq);
>>> +		} while (unlikely(deq == 0));
>>>
>>>  		deq_last_time = rte_rdtsc_precise() - deq_start_time;
>>>  		time_st->deq_max_time = RTE_MAX(time_st-
>>> deq_max_time,
diff mbox series

Patch

diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
index b5dc536..a6884c5 100644
--- a/app/test-bbdev/test_bbdev_perf.c
+++ b/app/test-bbdev/test_bbdev_perf.c
@@ -4463,8 +4463,8 @@  typedef int (test_case_function)(struct active_device *ad,
 		/* Dequeue one operation */
 		do {
 			deq += rte_bbdev_dequeue_dec_ops(dev_id, queue_id,
-					&ops_deq[deq], 1);
-		} while (unlikely(deq != 1));
+					&ops_deq[deq], enq);
+		} while (unlikely(deq == 0));
 
 		deq_last_time = rte_rdtsc_precise() - deq_start_time;
 		time_st->deq_max_time = RTE_MAX(time_st->deq_max_time,
@@ -4554,8 +4554,8 @@  typedef int (test_case_function)(struct active_device *ad,
 		/* Dequeue one operation */
 		do {
 			deq += rte_bbdev_dequeue_ldpc_dec_ops(dev_id, queue_id,
-					&ops_deq[deq], 1);
-		} while (unlikely(deq != 1));
+					&ops_deq[deq], enq);
+		} while (unlikely(deq == 0));
 
 		deq_last_time = rte_rdtsc_precise() - deq_start_time;
 		time_st->deq_max_time = RTE_MAX(time_st->deq_max_time,
@@ -4642,8 +4642,8 @@  typedef int (test_case_function)(struct active_device *ad,
 		/* Dequeue one operation */
 		do {
 			deq += rte_bbdev_dequeue_enc_ops(dev_id, queue_id,
-					&ops_deq[deq], 1);
-		} while (unlikely(deq != 1));
+					&ops_deq[deq], enq);
+		} while (unlikely(deq == 0));
 
 		deq_last_time = rte_rdtsc_precise() - deq_start_time;
 		time_st->deq_max_time = RTE_MAX(time_st->deq_max_time,
@@ -4725,8 +4725,8 @@  typedef int (test_case_function)(struct active_device *ad,
 		/* Dequeue one operation */
 		do {
 			deq += rte_bbdev_dequeue_ldpc_enc_ops(dev_id, queue_id,
-					&ops_deq[deq], 1);
-		} while (unlikely(deq != 1));
+					&ops_deq[deq], enq);
+		} while (unlikely(deq == 0));
 
 		deq_last_time = rte_rdtsc_precise() - deq_start_time;
 		time_st->deq_max_time = RTE_MAX(time_st->deq_max_time,