[1/4] test/lpm: fix cycle calculation in rcu qsbr perf
Checks
Commit Message
Fix incorrect calculations for LPM adds, LPM deletes,
and average cycles in RCU QSBR perf tests
Fixes: eff30b59cc2e ("test/lpm: add RCU performance tests")
Cc: honnappa.nagarahalli@arm.com
Cc: stable@dpdk.org
Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
---
app/test/test_lpm_perf.c | 43 ++++++++++++++--------------------------
1 file changed, 15 insertions(+), 28 deletions(-)
Comments
On Thu, Oct 29, 2020 at 4:37 PM Dharmik Thakkar <dharmik.thakkar@arm.com> wrote:
>
> Fix incorrect calculations for LPM adds, LPM deletes,
> and average cycles in RCU QSBR perf tests
>
> Fixes: eff30b59cc2e ("test/lpm: add RCU performance tests")
> Cc: honnappa.nagarahalli@arm.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Bruce, Vladimir, reviews for this series please?
Thanks.
On Thu, Oct 29, 2020 at 10:36:31AM -0500, Dharmik Thakkar wrote:
> Fix incorrect calculations for LPM adds, LPM deletes,
> and average cycles in RCU QSBR perf tests
>
To help review this patch, could you provide some more details in the
commit log as to what exactly was wrong with the calculation and how this
patch fixes things?
> Fixes: eff30b59cc2e ("test/lpm: add RCU performance tests")
> Cc: honnappa.nagarahalli@arm.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> On Nov 2, 2020, at 9:11 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
>
> On Thu, Oct 29, 2020 at 10:36:31AM -0500, Dharmik Thakkar wrote:
>> Fix incorrect calculations for LPM adds, LPM deletes,
>> and average cycles in RCU QSBR perf tests
>>
>
> To help review this patch, could you provide some more details in the
> commit log as to what exactly was wrong with the calculation and how this
> patch fixes things?
>
I will update the commit message in the next version. Adding it here as well:
Since, rcu qsbr tests run for ‘RCU_ITERATIONS’ and not ‘ITERATIONS’,
replace ‘ITERATIONS’ with ‘RCU_ITERATIONS’ for calculating adds, deletes, and cycles.
Also, for multi-writer perf test, each writer only writes half of NUM_LDEPTH_ROUTE_ENTRIES.
For 2 writers, total adds (or deletes) should be (RCU_ITERATIONS * NUM_LDEPTH_ROUTE_ENTRIES) instead of
(2 * RCU_ITERATIONS * NUM_LDEPTH_ROUTE_ENTRIES).
Since, for both the single and multi writer tests, total adds/deletes is equal to (RCU_ITERATIONS * NUM_LDEPTH_ROUTE_ENTRIES),
this has been replaced with a macro ’TOTAL_WRITES’ and furthermore, ‘g_writes’ has been removed since it is always a fixed value
equal to TOTAL_WRITES.
>> Fixes: eff30b59cc2e ("test/lpm: add RCU performance tests")
>> Cc: honnappa.nagarahalli@arm.com
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Hi Dharmik,
On 02/11/2020 16:58, Dharmik Thakkar wrote:
>
>> On Nov 2, 2020, at 9:11 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
>>
>> On Thu, Oct 29, 2020 at 10:36:31AM -0500, Dharmik Thakkar wrote:
>>> Fix incorrect calculations for LPM adds, LPM deletes,
>>> and average cycles in RCU QSBR perf tests
>>>
>>
>> To help review this patch, could you provide some more details in the
>> commit log as to what exactly was wrong with the calculation and how this
>> patch fixes things?
>>
>
> I will update the commit message in the next version. Adding it here as well:
>
> Since, rcu qsbr tests run for ‘RCU_ITERATIONS’ and not ‘ITERATIONS’,
> replace ‘ITERATIONS’ with ‘RCU_ITERATIONS’ for calculating adds, deletes, and cycles.
>
> Also, for multi-writer perf test, each writer only writes half of NUM_LDEPTH_ROUTE_ENTRIES.
> For 2 writers, total adds (or deletes) should be (RCU_ITERATIONS * NUM_LDEPTH_ROUTE_ENTRIES) instead of
> (2 * RCU_ITERATIONS * NUM_LDEPTH_ROUTE_ENTRIES).
>
> Since, for both the single and multi writer tests, total adds/deletes is equal to (RCU_ITERATIONS * NUM_LDEPTH_ROUTE_ENTRIES),
> this has been replaced with a macro ’TOTAL_WRITES’ and furthermore, ‘g_writes’ has been removed since it is always a fixed value
> equal to TOTAL_WRITES.
>
Thanks for the clarification. I left a few comments regarding 4-th
patch. First 3 patches LGTM, just put more details in the commit message.
>>> Fixes: eff30b59cc2e ("test/lpm: add RCU performance tests")
>>> Cc: honnappa.nagarahalli@arm.com
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>>> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
>
On Mon, Nov 02, 2020 at 04:58:43PM +0000, Dharmik Thakkar wrote:
>
> > On Nov 2, 2020, at 9:11 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
> >
> > On Thu, Oct 29, 2020 at 10:36:31AM -0500, Dharmik Thakkar wrote:
> >> Fix incorrect calculations for LPM adds, LPM deletes,
> >> and average cycles in RCU QSBR perf tests
> >>
> >
> > To help review this patch, could you provide some more details in the
> > commit log as to what exactly was wrong with the calculation and how this
> > patch fixes things?
> >
>
> I will update the commit message in the next version. Adding it here as well:
>
> Since, rcu qsbr tests run for ‘RCU_ITERATIONS’ and not ‘ITERATIONS’,
> replace ‘ITERATIONS’ with ‘RCU_ITERATIONS’ for calculating adds, deletes, and cycles.
>
> Also, for multi-writer perf test, each writer only writes half of NUM_LDEPTH_ROUTE_ENTRIES.
> For 2 writers, total adds (or deletes) should be (RCU_ITERATIONS * NUM_LDEPTH_ROUTE_ENTRIES) instead of
> (2 * RCU_ITERATIONS * NUM_LDEPTH_ROUTE_ENTRIES).
>
> Since, for both the single and multi writer tests, total adds/deletes is equal to (RCU_ITERATIONS * NUM_LDEPTH_ROUTE_ENTRIES),
> this has been replaced with a macro ’TOTAL_WRITES’ and furthermore, ‘g_writes’ has been removed since it is always a fixed value
> equal to TOTAL_WRITES.
>
Thanks for the clear explanation.
@@ -23,7 +23,6 @@ static struct rte_rcu_qsbr *rv;
static volatile uint8_t writer_done;
static volatile uint32_t thr_id;
static uint64_t gwrite_cycles;
-static uint64_t gwrites;
/* LPM APIs are not thread safe, use mutex to provide thread safety */
static pthread_mutex_t lpm_mutex = PTHREAD_MUTEX_INITIALIZER;
@@ -60,6 +59,8 @@ static uint32_t num_ldepth_route_entries;
#define NUM_ROUTE_ENTRIES num_route_entries
#define NUM_LDEPTH_ROUTE_ENTRIES num_ldepth_route_entries
+#define TOTAL_WRITES (RCU_ITERATIONS * NUM_LDEPTH_ROUTE_ENTRIES)
+
enum {
IP_CLASS_A,
IP_CLASS_B,
@@ -432,7 +433,6 @@ test_lpm_rcu_qsbr_writer(void *arg)
uint8_t core_id = (uint8_t)((uintptr_t)arg);
uint32_t next_hop_add = 0xAA;
- RTE_SET_USED(arg);
/* 2 writer threads are used */
if (core_id % 2 == 0) {
si = 0;
@@ -472,9 +472,6 @@ test_lpm_rcu_qsbr_writer(void *arg)
total_cycles = rte_rdtsc_precise() - begin;
__atomic_fetch_add(&gwrite_cycles, total_cycles, __ATOMIC_RELAXED);
- __atomic_fetch_add(&gwrites,
- 2 * NUM_LDEPTH_ROUTE_ENTRIES * RCU_ITERATIONS,
- __ATOMIC_RELAXED);
return 0;
}
@@ -528,7 +525,6 @@ test_lpm_rcu_perf_multi_writer(void)
writer_done = 0;
__atomic_store_n(&gwrite_cycles, 0, __ATOMIC_RELAXED);
- __atomic_store_n(&gwrites, 0, __ATOMIC_RELAXED);
__atomic_store_n(&thr_id, 0, __ATOMIC_SEQ_CST);
@@ -548,13 +544,10 @@ test_lpm_rcu_perf_multi_writer(void)
if (rte_eal_wait_lcore(enabled_core_ids[i]) < 0)
goto error;
- printf("Total LPM Adds: %d\n",
- 2 * ITERATIONS * NUM_LDEPTH_ROUTE_ENTRIES);
- printf("Total LPM Deletes: %d\n",
- 2 * ITERATIONS * NUM_LDEPTH_ROUTE_ENTRIES);
+ printf("Total LPM Adds: %d\n", TOTAL_WRITES);
+ printf("Total LPM Deletes: %d\n", TOTAL_WRITES);
printf("Average LPM Add/Del: %"PRIu64" cycles\n",
- __atomic_load_n(&gwrite_cycles, __ATOMIC_RELAXED) /
- __atomic_load_n(&gwrites, __ATOMIC_RELAXED)
+ __atomic_load_n(&gwrite_cycles, __ATOMIC_RELAXED) / TOTAL_WRITES
);
/* Wait and check return value from reader threads */
@@ -581,7 +574,6 @@ test_lpm_rcu_perf_multi_writer(void)
writer_done = 0;
__atomic_store_n(&gwrite_cycles, 0, __ATOMIC_RELAXED);
- __atomic_store_n(&gwrites, 0, __ATOMIC_RELAXED);
__atomic_store_n(&thr_id, 0, __ATOMIC_SEQ_CST);
/* Launch reader threads */
@@ -600,14 +592,11 @@ test_lpm_rcu_perf_multi_writer(void)
if (rte_eal_wait_lcore(enabled_core_ids[i]) < 0)
goto error;
- printf("Total LPM Adds: %d\n",
- 2 * ITERATIONS * NUM_LDEPTH_ROUTE_ENTRIES);
- printf("Total LPM Deletes: %d\n",
- 2 * ITERATIONS * NUM_LDEPTH_ROUTE_ENTRIES);
+ printf("Total LPM Adds: %d\n", TOTAL_WRITES);
+ printf("Total LPM Deletes: %d\n", TOTAL_WRITES);
printf("Average LPM Add/Del: %"PRIu64" cycles\n",
- __atomic_load_n(&gwrite_cycles, __ATOMIC_RELAXED) /
- __atomic_load_n(&gwrites, __ATOMIC_RELAXED)
- );
+ __atomic_load_n(&gwrite_cycles, __ATOMIC_RELAXED)
+ / TOTAL_WRITES);
writer_done = 1;
/* Wait and check return value from reader threads */
@@ -711,11 +700,10 @@ test_lpm_rcu_perf(void)
}
total_cycles = rte_rdtsc_precise() - begin;
- printf("Total LPM Adds: %d\n", ITERATIONS * NUM_LDEPTH_ROUTE_ENTRIES);
- printf("Total LPM Deletes: %d\n",
- ITERATIONS * NUM_LDEPTH_ROUTE_ENTRIES);
+ printf("Total LPM Adds: %d\n", TOTAL_WRITES);
+ printf("Total LPM Deletes: %d\n", TOTAL_WRITES);
printf("Average LPM Add/Del: %g cycles\n",
- (double)total_cycles / (NUM_LDEPTH_ROUTE_ENTRIES * ITERATIONS));
+ (double)total_cycles / TOTAL_WRITES);
writer_done = 1;
/* Wait and check return value from reader threads */
@@ -771,11 +759,10 @@ test_lpm_rcu_perf(void)
}
total_cycles = rte_rdtsc_precise() - begin;
- printf("Total LPM Adds: %d\n", ITERATIONS * NUM_LDEPTH_ROUTE_ENTRIES);
- printf("Total LPM Deletes: %d\n",
- ITERATIONS * NUM_LDEPTH_ROUTE_ENTRIES);
+ printf("Total LPM Adds: %d\n", TOTAL_WRITES);
+ printf("Total LPM Deletes: %d\n", TOTAL_WRITES);
printf("Average LPM Add/Del: %g cycles\n",
- (double)total_cycles / (NUM_LDEPTH_ROUTE_ENTRIES * ITERATIONS));
+ (double)total_cycles / TOTAL_WRITES);
writer_done = 1;
/* Wait and check return value from reader threads */