| Message ID | 20190628034406.5399-2-honnappa.nagarahalli@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] test/rcu: increase the size of num cores variable | expand |
On Fri, Jun 28, 2019 at 5:44 AM Honnappa Nagarahalli < honnappa.nagarahalli@arm.com> wrote: > Test case for rte_rcu_qsbr_get_memsize is written specifically > for 128 threads. Do not use RTE_MAX_LCORE as it changes for > different configurations. > Does it mean this test can only work on arm with 256 lcores? How many cores does this test require? -- David Marchand
On Fri, Jun 28, 2019 at 5:44 AM Honnappa Nagarahalli <honnappa.nagarahalli@arm.com<mailto:honnappa.nagarahalli@arm.com>> wrote: Test case for rte_rcu_qsbr_get_memsize is written specifically for 128 threads. Do not use RTE_MAX_LCORE as it changes for different configurations. Does it mean this test can only work on arm with 256 lcores? How many cores does this test require? [Honnappa] It tests the correctness of the calculation of the memory required. So, it uses the hand calculated number to verify. The hand calculated number is for 128 cores. So, it does not depend on the platform as such. -- David Marchand
On Fri, Jun 28, 2019 at 3:54 PM Honnappa Nagarahalli < Honnappa.Nagarahalli@arm.com> wrote: > > > On Fri, Jun 28, 2019 at 5:44 AM Honnappa Nagarahalli < > honnappa.nagarahalli@arm.com> wrote: > > Test case for rte_rcu_qsbr_get_memsize is written specifically > for 128 threads. Do not use RTE_MAX_LCORE as it changes for > different configurations. > > > > Does it mean this test can only work on arm with 256 lcores? > > How many cores does this test require? > > *[Honnappa] *It tests the correctness of the calculation of the memory > required. So, it uses the hand calculated number to verify. The hand > calculated number is for 128 cores. So, it does not depend on the platform > as such. > Ah ah, funny that the default RTE_MAX_LCORE for x86 is 128, and then I did not see the test failing. Then ok for this fix. Reviewed-by: David Marchand <david.marchand@redhat.com> How about the followup patch: - TEST_RCU_QSBR_RETURN_IF_ERROR((sz != 8384 && sz != 16768), - "Get Memsize for 128 threads"); + TEST_RCU_QSBR_RETURN_IF_ERROR( +#if RTE_CACHE_LINE_SIZE == 64 + sz != 8384 +#elif RTE_CACHE_LINE_SIZE == 128 + sz != 16768 +#endif + , "Get Memsize for 128 threads"); -- David Marchand
On Fri, Jun 28, 2019 at 5:44 AM Honnappa Nagarahalli <honnappa.nagarahalli@arm.com<mailto:honnappa.nagarahalli@arm.com>> wrote:
Test case for rte_rcu_qsbr_get_memsize is written specifically
for 128 threads. Do not use RTE_MAX_LCORE as it changes for
different configurations.
Does it mean this test can only work on arm with 256 lcores?
How many cores does this test require?
[Honnappa] It tests the correctness of the calculation of the memory required. So, it uses the hand calculated number to verify. The hand calculated number is for 128 cores. So, it does not depend on the platform as such.
Ah ah, funny that the default RTE_MAX_LCORE for x86 is 128, and then I did not see the test failing.
Then ok for this fix.
Reviewed-by: David Marchand <david.marchand@redhat.com<mailto:david.marchand@redhat.com>>
How about the followup patch:
- TEST_RCU_QSBR_RETURN_IF_ERROR((sz != 8384 && sz != 16768),
- "Get Memsize for 128 threads");
+ TEST_RCU_QSBR_RETURN_IF_ERROR(
+#if RTE_CACHE_LINE_SIZE == 64
+ sz != 8384
+#elif RTE_CACHE_LINE_SIZE == 128
+ sz != 16768
+#endif
+ , "Get Memsize for 128 threads");
[Honnappa] Added this change to V2, but slightly differently
--
David Marchand
On Fri, Jun 28, 2019 at 6:37 PM Honnappa Nagarahalli < Honnappa.Nagarahalli@arm.com> wrote: > On Fri, Jun 28, 2019 at 5:44 AM Honnappa Nagarahalli < > honnappa.nagarahalli@arm.com<mailto:honnappa.nagarahalli@arm.com>> wrote: > Test case for rte_rcu_qsbr_get_memsize is written specifically > for 128 threads. Do not use RTE_MAX_LCORE as it changes for > different configurations. > > Does it mean this test can only work on arm with 256 lcores? > How many cores does this test require? > [Honnappa] It tests the correctness of the calculation of the memory > required. So, it uses the hand calculated number to verify. The hand > calculated number is for 128 cores. So, it does not depend on the platform > as such. > > Ah ah, funny that the default RTE_MAX_LCORE for x86 is 128, and then I did > not see the test failing. > Then ok for this fix. > > Reviewed-by: David Marchand <david.marchand@redhat.com<mailto: > david.marchand@redhat.com>> > > > How about the followup patch: > > - TEST_RCU_QSBR_RETURN_IF_ERROR((sz != 8384 && sz != 16768), > - "Get Memsize for 128 threads"); > + TEST_RCU_QSBR_RETURN_IF_ERROR( > +#if RTE_CACHE_LINE_SIZE == 64 > + sz != 8384 > +#elif RTE_CACHE_LINE_SIZE == 128 > + sz != 16768 > +#endif > + , "Get Memsize for 128 threads"); > [Honnappa] Added this change to V2, but slightly differently > Yep saw it. Thanks. -- David Marchand
diff --git a/app/test/test_rcu_qsbr.c b/app/test/test_rcu_qsbr.c index 0c6267ee9..5d92fbee8 100644 --- a/app/test/test_rcu_qsbr.c +++ b/app/test/test_rcu_qsbr.c @@ -89,13 +89,13 @@ test_rcu_qsbr_get_memsize(void) sz = rte_rcu_qsbr_get_memsize(0); TEST_RCU_QSBR_RETURN_IF_ERROR((sz != 1), "Get Memsize for 0 threads"); - sz = rte_rcu_qsbr_get_memsize(RTE_MAX_LCORE); + sz = rte_rcu_qsbr_get_memsize(128); /* For 128 threads, * for machines with cache line size of 64B - 8384 * for machines with cache line size of 128 - 16768 */ TEST_RCU_QSBR_RETURN_IF_ERROR((sz != 8384 && sz != 16768), - "Get Memsize"); + "Get Memsize for 128 threads"); return 0; }
Test case for rte_rcu_qsbr_get_memsize is written specifically for 128 threads. Do not use RTE_MAX_LCORE as it changes for different configurations. Fixes: e6a14121f4ae ("test/rcu: remove arbitrary limit on max core count") Cc: stable@dpdk.org Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com> --- app/test/test_rcu_qsbr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- 2.17.1