Message ID | 20140730132639.GA27986@localhost |
---|---|
State | New |
Headers | show |
On 30 July 2014 18:56, Stuart Haslam <stuart.haslam@arm.com> wrote: > On Wed, Jul 30, 2014 at 12:10:24PM +0100, Santosh Shukla wrote: >> Hi, >> >> I noticed that few example odp application (l2fwd, generator, pktio >> and odp_timer_ping) likely to store wrong thread_id because of the way >> they were using odp_linux_pthread_create() api. >> >> Example: >> >> Refer file example/l2fwd/odp_l2fwd.c, odp_generator.c and odp_time_ping.c >> >> code snippet look like this >> >> for (i = 0; i < num_workers; ++i) { >> >> ............... >> ............. >> odp_linux_pthread_create(thread_tbl, 1, core, thr_run_func, >> &gbl_args->thread[i]); >> } >> The above will overwrite thread_tbl[0] for each iteration. >> >> So if you print thread_tbl[] array, you will find only thread_tbl[0] >> with valid value and rest with zero. >> >> Problem is inner for loop of odp_linux_pthread_create() refills entry >> in thread_tbl starting from index 0 till max number (arg3 above) >> >> In my isolation case as well other cases like generator, l2fwd app, >> don't want odp_linux_pthread_create to fill thread_tbl entry starting >> from 0 everytime. > > This looks like a bug in the application rather than in the API or the > implementation. Why not just pass the correct address from the > application? > > As far as I can tell, this (completely untested) patch would have the > same effect as yours; > > diff --git a/test/api_test/odp_timer_ping.c b/test/api_test/odp_timer_ping.c > index cd67e0d..88650d6 100644 > --- a/test/api_test/odp_timer_ping.c > +++ b/test/api_test/odp_timer_ping.c > @@ -367,7 +367,7 @@ int main(int argc ODP_UNUSED, char *argv[] ODP_UNUSED) > run_thread = send_ping; > > /* Create and launch worker threads */ > - odp_linux_pthread_create(thread_tbl, 1, i, > + odp_linux_pthread_create(&thread_tbl[i], 1, i, > run_thread, (pthrd_arg *)&pingarg); > } Yes and Its a better fix. Ignore other patches sent on this topic. I will send series of patch set where we want to fix this problem happening on other odp example application. Thanks. > > > -- > Stuart. >
On 07/30/2014 04:45 PM, Santosh Shukla wrote: > On 30 July 2014 18:56, Stuart Haslam <stuart.haslam@arm.com> wrote: >> As far as I can tell, this (completely untested) patch would have the >> same effect as yours; >> >> diff --git a/test/api_test/odp_timer_ping.c b/test/api_test/odp_timer_ping.c >> index cd67e0d..88650d6 100644 >> --- a/test/api_test/odp_timer_ping.c >> +++ b/test/api_test/odp_timer_ping.c >> @@ -367,7 +367,7 @@ int main(int argc ODP_UNUSED, char *argv[] ODP_UNUSED) >> run_thread = send_ping; >> >> /* Create and launch worker threads */ >> - odp_linux_pthread_create(thread_tbl, 1, i, >> + odp_linux_pthread_create(&thread_tbl[i], 1, i, >> run_thread, (pthrd_arg *)&pingarg); >> } > > Yes and Its a better fix. > > Ignore other patches sent on this topic. I will send series of patch > set where we want to fix this problem happening on other odp example > application. Hi Santosh, Will you send the fix?
Yup, tomorrow. On 3 August 2014 20:25, Taras Kondratiuk <taras.kondratiuk@linaro.org> wrote: > On 07/30/2014 04:45 PM, Santosh Shukla wrote: >> >> On 30 July 2014 18:56, Stuart Haslam <stuart.haslam@arm.com> wrote: >>> >>> As far as I can tell, this (completely untested) patch would have the >>> same effect as yours; >>> >>> diff --git a/test/api_test/odp_timer_ping.c >>> b/test/api_test/odp_timer_ping.c >>> index cd67e0d..88650d6 100644 >>> --- a/test/api_test/odp_timer_ping.c >>> +++ b/test/api_test/odp_timer_ping.c >>> @@ -367,7 +367,7 @@ int main(int argc ODP_UNUSED, char *argv[] >>> ODP_UNUSED) >>> run_thread = send_ping; >>> >>> /* Create and launch worker threads */ >>> - odp_linux_pthread_create(thread_tbl, 1, i, >>> + odp_linux_pthread_create(&thread_tbl[i], 1, i, >>> run_thread, (pthrd_arg >>> *)&pingarg); >>> } >> >> >> Yes and Its a better fix. >> >> Ignore other patches sent on this topic. I will send series of patch >> set where we want to fix this problem happening on other odp example >> application. > > > Hi Santosh, > > Will you send the fix?
diff --git a/test/api_test/odp_timer_ping.c b/test/api_test/odp_timer_ping.c index cd67e0d..88650d6 100644 --- a/test/api_test/odp_timer_ping.c +++ b/test/api_test/odp_timer_ping.c @@ -367,7 +367,7 @@ int main(int argc ODP_UNUSED, char *argv[] ODP_UNUSED) run_thread = send_ping; /* Create and launch worker threads */ - odp_linux_pthread_create(thread_tbl, 1, i, + odp_linux_pthread_create(&thread_tbl[i], 1, i, run_thread, (pthrd_arg *)&pingarg); }