Re: [lng-odp] [PATCH v2] validation: add odp_system test

2014-12-14 Thread Jerin Jacob 
On Fri, Dec 12, 2014 at 03:40:29PM -0500, Mike Holmes wrote:
> Add tests for ODP system_info interface
> 
> Signed-off-by: Mike Holmes 
> ---
> 
> This api has poor documentaion, these testis attempt to do something sensible
> against linux-generic in its present form.
> 
>  test/validation/.gitignore   |  1 +
>  test/validation/Makefile.am  |  4 ++-
>  test/validation/odp_system.c | 73 
> 
>  3 files changed, 77 insertions(+), 1 deletion(-)
>  create mode 100644 test/validation/odp_system.c
> 
> diff --git a/test/validation/.gitignore b/test/validation/.gitignore
> index 32834ae..a388488 100644
> --- a/test/validation/.gitignore
> +++ b/test/validation/.gitignore
> @@ -5,3 +5,4 @@ odp_queue
>  odp_crypto
>  odp_schedule
>  odp_shm
> +odp_system
> diff --git a/test/validation/Makefile.am b/test/validation/Makefile.am
> index d0b5426..98376b9 100644
> --- a/test/validation/Makefile.am
> +++ b/test/validation/Makefile.am
> @@ -6,7 +6,7 @@ AM_LDFLAGS += -static
>  if ODP_CUNIT_ENABLED
>  TESTS = ${bin_PROGRAMS}
>  check_PROGRAMS = ${bin_PROGRAMS}
> -bin_PROGRAMS = odp_init odp_queue odp_crypto odp_shm odp_schedule
> +bin_PROGRAMS = odp_init odp_queue odp_crypto odp_shm odp_schedule odp_system
>  odp_init_LDFLAGS = $(AM_LDFLAGS)
>  odp_queue_LDFLAGS = $(AM_LDFLAGS)
>  odp_crypto_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/crypto
> @@ -15,6 +15,7 @@ odp_shm_CFLAGS = $(AM_CFLAGS)
>  odp_shm_LDFLAGS = $(AM_LDFLAGS)
>  odp_schedule_CFLAGS = $(AM_CFLAGS)
>  odp_schedule_LDFLAGS = $(AM_LDFLAGS)
> +odp_system_LDFLAGS = $(AM_LDFLAGS)
>  endif
>  
>  dist_odp_init_SOURCES = odp_init.c
> @@ -25,6 +26,7 @@ dist_odp_crypto_SOURCES = 
> crypto/odp_crypto_test_async_inp.c \
> odp_crypto.c common/odp_cunit_common.c
>  dist_odp_shm_SOURCES = odp_shm.c common/odp_cunit_common.c
>  dist_odp_schedule_SOURCES = odp_schedule.c common/odp_cunit_common.c
> +dist_odp_system_SOURCES = odp_system.c common/odp_cunit_common.c
>  
>  #For Linux generic the unimplemented crypto API functions break the
>  #regression TODO: https://bugs.linaro.org/show_bug.cgi?id=975
> diff --git a/test/validation/odp_system.c b/test/validation/odp_system.c
> new file mode 100644
> index 000..dfad677
> --- /dev/null
> +++ b/test/validation/odp_system.c
> @@ -0,0 +1,73 @@
> +/* Copyright (c) 2014, Linaro Limited
> + * All rights reserved.
> + *
> + * SPDX-License-Identifier: BSD-3-Clause
> + */
> +
> +#include "odp.h"
> +#include "odp_cunit_common.h"
> +
> +static void test_odp_sys_core_count(void)
> +{
> + int cores;
> +
> + cores = odp_sys_core_count();
> + CU_ASSERT(0 < cores);
> +}
> +
> +static void test_odp_sys_cache_line_size(void)
> +{
> + int cache_size;
> +
> + cache_size = odp_sys_cache_line_size();
> + CU_ASSERT(0 < cache_size);

May be we can add one more assert here,
CU_ASSERT(ODP_CACHE_LINE_SIZE == cache_size)


> +}
> +
> +static void test_odp_sys_cpu_model_str(void)
> +{
> + char model[128];
> +
> + snprintf(model, 128, "%s", odp_sys_cpu_model_str());
> + CU_ASSERT(strlen(model) <= 127);
> + CU_ASSERT_PTR_NOT_NULL(odp_sys_cpu_model_str());
> +}
> +
> +static void test_odp_sys_page_size(void)
> +{
> + uint64_t page;
> +
> + page = odp_sys_page_size();
> + CU_ASSERT(0 < page);
> +}
> +
> +static void test_odp_sys_huge_page_size(void)
> +{
> + uint64_t page;
> +
> + page = odp_sys_huge_page_size();
> + CU_ASSERT(0 < page || 0 == page);
> +}
> +
> +static void test_odp_sys_cpu_hz(void)
> +{
> + uint64_t hz;
> +
> + hz = odp_sys_cpu_hz();
> + CU_ASSERT(0 < hz);
> +}
> +
> +CU_TestInfo test_odp_system[] = {
> + {"odp_sys_core_count",  test_odp_sys_core_count},
> + {"odp_sys_cache_line_size",  test_odp_sys_cache_line_size},
> + {"odp_sys_cpu_model_str",  test_odp_sys_cpu_model_str},
> + {"odp_sys_page_size",  test_odp_sys_page_size},
> + {"odp_sys_huge_page_size",  test_odp_sys_huge_page_size},
> + {"odp_sys_cpu_hz",  test_odp_sys_cpu_hz},
> + CU_TEST_INFO_NULL,
> +};
> +
> +CU_SuiteInfo odp_testsuites[] = {
> + {"System Info", NULL, NULL, NULL, NULL,
> +  test_odp_system},
> +  CU_SUITE_INFO_NULL,
> +};
> -- 
> 2.1.0
> 
> 
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp

___
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCH] configure.ac check for atomic operations support

2014-12-14 Thread Jerin Jacob 
On Fri, Dec 12, 2014 at 10:56:26PM +0100, Ola Liljedahl wrote:
> On 12 December 2014 at 16:59, Mike Holmes  wrote:
> >
> >
> > On 12 December 2014 at 10:51, Maxim Uvarov  wrote:
> >>
> >> On 12/12/2014 06:47 PM, Taras Kondratiuk wrote:
> >>>
> >>> On 12/12/2014 05:03 PM, Maxim Uvarov wrote:
> 
>  Odp atomic operations based on compiler build-ins. Make
>  sure that compiler supports such operation at configure
>  stage.
> >>>
> >>> This check should be limited to platforms that use gcc atomics.
> >>
> >> Do we have such platforms?
> >
> >
> > __OCTEON__ directly swappes out gcc, infact that pre processor switch
> > affects other files in linux-generic too.
> >
> > static inline uint32_t odp_atomic_fetch_inc_u32(odp_atomic_u32_t *atom)
> > {
> > #if defined __OCTEON__
> >>---uint32_t ret;
> >>---__asm__ __volatile__ ("syncws");
> >>---__asm__ __volatile__ ("lai %0,(%2)" : "=r" (ret), "+m" (atom) :
> >>--->--->---  "r" (atom));
> >>---return ret;
> > #else
> >>---return __atomic_fetch_add(&atom->v, 1, __ATOMIC_RELAXED);
> > #endif
> > }
> This is an OCTEON-special for just one function. I didn't want to
> remove it. But I am not sure it is actually needed. Shouldn't the
> compiler be able to generate the appropriate OCTEON-specific
> instructions (e.g. laa(d), saa(d), lai(d), lad(d) etc), gcc has
> supported OCTEON since version 4.4? Also I don't understand the reason
> for the syncw *before* the lai (load-atomic-increment) but perhaps the
> load-atomic instructions (which shouldn't linger in the write buffer
> like stores) need to be treated differently.

I generated the dis-assembly of octeon gcc generated output,
Compiler does generate appropriate instruction for __atomic_fetch* instructions.
and if the contract is "__ATOMIC_RELAXED" then "syncws" can be removed.

Typical use case for putting "syncw" before the lai operation to make sure 
cpu does not change the order of "lai" and "instructions before it"
as octeon is weakly ordered cpu for better performance.

> 
> I think this OCTEON-fix should either be removed or there should be
> specials for most if not all of the functions in odp_atomic.h

Yes, It can be removed if the contract is "__ATOMIC_RELAXED".

Do we have any specific reason to keep the API as odp_atomic_*, if its
an "__ATOMIC_RELAXED" memory model then its better to rename as counters.


> >
> >
> >>
> >>
> >>
> >> Maxim.
> >>
> >>
> >> ___
> >> lng-odp mailing list
> >> lng-odp@lists.linaro.org
> >> http://lists.linaro.org/mailman/listinfo/lng-odp
> >
> >
> >
> > --
> > Mike Holmes
> > Linaro  Sr Technical Manager
> > LNG - ODP
> >
> > ___
> > lng-odp mailing list
> > lng-odp@lists.linaro.org
> > http://lists.linaro.org/mailman/listinfo/lng-odp
> >

___
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCH] validation:add atomic test in odp syncronizers

2014-12-14 Thread Mario Torrecillas Rodriguez 
I have combined both patches into a single one using Barry¹s as the
baseline. I need to fix a couple of minor issues that I have and do some
cleanup but I¹ll most likely send the patch tomorrow so that we can continue
working on this unified set of synchroniser tests from now on.
Like Mike said, there¹s basically one test set per lock, one for barrier,
and one for atomics, and they are all executed from a single application.

We also need tests that exercise the atomic primitives using more than one
thread so that race conditions can actually exist (in the presence of bugs
in the library).

Mario.

From:  Mike Holmes 
Date:  Friday, 12 December 2014 18:41
To:  "yan.songm...@linaro.org" 
Cc:  spinney , lng-odp 
Subject:  Re: [lng-odp] [PATCH] validation:add atomic test in odp
syncronizers

Synchronize with Mario, but we don't want to lose any extra functionality in
combining all the work.

I assume that test/validation/sync  (synchronizers)  will contain one test
suite each for for atomics, rwlock, spin lock and ticket lock and all of
those test suites will be called from a top level test/validation/odp_sync
test.

Mike

On 12 December 2014 at 03:43, yan.songm...@linaro.org
 wrote:
> Mike,
> I think Barry's patch is cover the lock, and my patch cover the atomic.
> 
> But I still have some cards about the rwlock, spin lock and ticket lock.
> 
> So ,should  I write a new one accord Barry's patch or just use his code and
> change it suitable for our  sunny day test if necessary  ?
> I prefer the second.
> 
> Thanks,
> Yan
> 
> yan.songm...@linaro.org
>>  
>> From: Mike Holmes 
>> Date: 2014-12-12 03:09
>> To: Ola Liljedahl 
>> CC: Yan Sonming  ; Barry Spinney
>>  ; lng-odp 
>> Subject: Re: [lng-odp] [PATCH] validation:add atomic test in odp syncronizers
>> 
>> 
>> On 11 December 2014 at 04:36, Ola Liljedahl  wrote:
>>> Are these cunit tests for odp_atomic.h only?
>>> Are there any cunit tests for plain spin locks, ticket locks, rwlocks etc?
>> 
>> Yes, Mario has Barrys code as the basis.
>>  
>>> 
>>> -- Ola
>>> 
>>> 
>>> On 10 December 2014 at 20:42, Mike Holmes  wrote:
 > I think odp_syne would be better as odp_sync  throughout the patch
 >
 > Does Barrys patch cover atomics in an overlapping way, or do we have both
 as
 > separate suites ? I think this one is much more sunny day as we had
 > originally planned.
 > Mario were you able to make the final changes to create a patch for tip
 from
 > Barrys files, do we need to wait until we can see that patch along side
 this
 > one ?
 >
 > Mike
 >
 >
 > On 9 December 2014 at 22:19, Yan Sonming  wrote:
> >>
> >> Remove odp_atomic_test in test/api_test and  add the odp_atomic_test
> >> to test/validation as one part of odp syncronizers test
 >
 >
 > syncronizers
 >
> >>
> >>
> >> Signed-off-by: Yan Songming 
> >> ---
> >>  test/api_test/Makefile.am  |   5 +-
> >>  test/api_test/odp_atomic_test.c| 299
> >> -
> >>  test/api_test/odp_atomic_test.h|  51 --
> >>  test/api_test/odp_common.c |   1 -
> >>  test/validation/.gitignore |   1 +
> >>  test/validation/Makefile.am|   6 +-
> >>  test/validation/odp_syne.c |  15 ++
> >>  test/validation/syne/odp_test_atomic.c | 258
> 
> >>  test/validation/syne/odp_test_atomic.h |  14 ++
> >>  9 files changed, 294 insertions(+), 356 deletions(-)
> >>  delete mode 100644 test/api_test/odp_atomic_test.c
> >>  delete mode 100644 test/api_test/odp_atomic_test.h
> >>  create mode 100644 test/validation/odp_syne.c
> >>  create mode 100644 test/validation/syne/odp_test_atomic.c
> >>  create mode 100644 test/validation/syne/odp_test_atomic.h
> >>
> >> diff --git a/test/api_test/Makefile.am b/test/api_test/Makefile.am
> >> index 74e8612..ce301ce 100644
> >> --- a/test/api_test/Makefile.am
> >> +++ b/test/api_test/Makefile.am
> >> @@ -1,18 +1,15 @@
> >>  include $(top_srcdir)/test/Makefile.inc
> >>
> >> -bin_PROGRAMS = odp_atomic odp_shm odp_ring odp_timer_ping
> >> +bin_PROGRAMS = odp_shm odp_ring odp_timer_ping
> >>
> >> -odp_atomic_CFLAGS = $(AM_CFLAGS)
> >>  odp_shm_CFLAGS = $(AM_CFLAGS)
> >>  odp_ring_CFLAGS = $(AM_CFLAGS)
> >>  odp_timer_ping_CFLAGS = $(AM_CFLAGS)
> >>
> >> -odp_atomic_LDFLAGS = $(AM_LDFLAGS) -static
> >>  odp_shm_LDFLAGS = $(AM_LDFLAGS) -static
> >>  odp_ring_LDFLAGS = $(AM_LDFLAGS) -static
> >>  odp_timer_ping_LDFLAGS = $(AM_LDFLAGS) -static
> >>
> >> -dist_odp_atomic_SOURCES = odp_atomic_test.c odp_common.c
> >>  dist_odp_shm_SOURCES = odp_shm_test.c odp_common.