Re: [dpdk-dev] [PATCH v2 1/2] test/hash: use existing lcore API

[Dharmik Thakkar <Dharmik.Thakkar@arm.com>]

> On May 27, 2019, at 5:15 AM, David Marchand <david.marchand@redhat.com> wrote:
> 
> Hello Dharmik,
Hi David,
> 
> On Wed, May 22, 2019 at 9:38 PM Dharmik Thakkar <Dharmik.Thakkar@arm.com> wrote:
> > On May 22, 2019, at 12:17 PM, David Marchand <david.marchand@redhat.com> wrote:
> > On Wed, May 22, 2019 at 6:27 PM Dharmik Thakkar <Dharmik.Thakkar@arm.com> wrote:
> > > @@ -738,10 +736,9 @@ struct {
> > >                               rte_eal_remote_launch(test_rwc_reader,
> > >                                               (void *)(uintptr_t)read_type,
> > >                                                       enabled_core_ids[i]);
> > > -                     rte_eal_mp_wait_lcore();
> > > 
> > >                       for (i = 1; i <= rwc_core_cnt[n]; i++)
> > > -                             if (lcore_config[i].ret < 0)
> > > +                             if (rte_eal_wait_lcore(i) < 0)
> > if (rte_eal_wait_lcore(enabled_core_ids[i]) < 0)
> > 
> > (There are similar changes in other functions too.
> > I realize that this is a separate issue than what the patch is aimed for.
> > If you see fit, please integrate it, else I will put out a patch once your patch has been merged.)
> > 
> > Indeed, reproduced.
> > 
> 
> Did you have a chance to look at this ?
> These loops could be reworked by using RTE_LCORE_FOREACH_SLAVE() (so that it automatically skips the master lcore) removing the need for enabled_core_ids[].
> 
Yes, I have tried to use RTE_LCORE_FOREACH_SLAVE() and eliminate enabled_core_ids[]. However, the code becomes quite complicated to read with if statements
within RTE_LCORE_FOREACH_SLAVE(), to break the loop (since not all slave cores are checked for). This is a snip from test_hash_multi_add_lookup():

					/* snip */
				RTE_LCORE_FOREACH_SLAVE(core_id) {
					if (i < rwc_core_cnt[n])
						rte_eal_remote_launch(test_rwc_reader,
								(void *)(uintptr_t)read_type,
								core_id);
					else if (i >= rwc_core_cnt[n] &&
						 i < (rwc_core_cnt[n] +
						 rwc_core_cnt[m])) {
						rte_eal_remote_launch(test_rwc_multi_writer,
							(void *)(uintptr_t)pos_core,
							core_id);
						pos_core++;
					} else
						break;
					i++;
				}
> 
> -- 
> David Marchand