Re: [Qemu-devel] [PATCH 2/2] atomic-test: Add spinlock test case

[Andrew Jones <drjones@redhat.com>]

On Thu, May 07, 2015 at 01:31:42PM +0200, Alexander Spyridakis wrote:
> Sample spinlock test case with the option to implement the spinlock
> by means of GCC atomic instructions or unsafe memory operations.
> Additionally, printf is wrapped around a spinlock to avoid concurrent
> access to the serial device.
> 
> Suggested-by: Jani Kokkonen <jani.kokkonen@huawei.com>
> Suggested-by: Claudio Fontana <claudio.fontana@huawei.com>
> Signed-off-by: Alexander Spyridakis <a.spyridakis@virtualopensystems.com>
> ---
>  tests/atomic-test/Makefile  |  3 +++
>  tests/atomic-test/helpers.c | 21 +++++++++++++++++++++
>  tests/atomic-test/helpers.h | 12 ++++++++++++
>  tests/atomic-test/main.c    | 35 ++++++++++++++++++++++++++++++++++-
>  tests/atomic-test/printf.c  |  5 +++++
>  5 files changed, 75 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/atomic-test/Makefile b/tests/atomic-test/Makefile
> index 094e01a..d1e992d 100644
> --- a/tests/atomic-test/Makefile
> +++ b/tests/atomic-test/Makefile
> @@ -12,6 +12,9 @@ LD_SCRIPT = link.ld.S
>  LIBS      = $(shell $(CC) $(CCFLAGS) -print-libgcc-file-name)
>  CPPFLAGS  += -gdwarf-2 -fno-stack-protector -nostdinc -fno-builtin
>  
> +# Set to ATOMIC to implement spinlock test with real atomic instructions
> +TEST = ATOMIC
> +
>  #
>  # Target specific variables
>  #
> diff --git a/tests/atomic-test/helpers.c b/tests/atomic-test/helpers.c
> index 8ac8c2c..fd2d4cf 100644
> --- a/tests/atomic-test/helpers.c
> +++ b/tests/atomic-test/helpers.c
> @@ -82,3 +82,24 @@ void power_off(void)
>      /* Shut down system */
>      psci_call(PSCI_SYSTEM_OFF, 0, 0, 0);
>  }
> +
> +void atomic_lock(int *lock_var)
> +{
> +    while (__sync_lock_test_and_set(lock_var, 1));
> +}
> +
> +void atomic_unlock(int *lock_var)
> +{
> +    __sync_lock_release(lock_var);
> +}

Do these builtins actually do anything without enabling the
MMU first?

> +
> +void non_atomic_lock(int *lock_var)
> +{
> +    while (*lock_var != 0);
> +    *lock_var = 1;
> +}
> +
> +void non_atomic_unlock(int *lock_var)
> +{
> +    *lock_var = 0;
> +}
> diff --git a/tests/atomic-test/helpers.h b/tests/atomic-test/helpers.h
> index 66d440e..93036be 100644
> --- a/tests/atomic-test/helpers.h
> +++ b/tests/atomic-test/helpers.h
> @@ -29,6 +29,18 @@
>  #define PSCI_CPU_OFF       0x84000002
>  #define PSCI_SYSTEM_OFF    0x84000008
>  
> +#ifdef ATOMIC
> +#define LOCK   atomic_lock
> +#define UNLOCK atomic_unlock
> +#else
> +#define LOCK   non_atomic_lock
> +#define UNLOCK non_atomic_unlock
> +#endif
> +
> +int global_lock;
> +int global_a;
> +int global_b;
> +
>  int get_cpuid(void);
>  void power_secondary(void);
>  void power_off();
> diff --git a/tests/atomic-test/main.c b/tests/atomic-test/main.c
> index 72eaf59..3143f7c 100644
> --- a/tests/atomic-test/main.c
> +++ b/tests/atomic-test/main.c
> @@ -15,9 +15,42 @@
>   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> +#include "helpers.h"
> +
> +#define LOOP_SIZE 1000000
> +
> +void test_spinlock()
> +{
> +    int i, errors = 0;
> +    int cpu = get_cpuid();
> +
> +    for (i = 0; i < LOOP_SIZE; i++) {
> +        LOCK(&global_lock);
> +
> +        if (global_a == (cpu + 1) % 2) {
> +            global_a = 1;
> +            global_b = 0;
> +        } else {
> +            global_a = 0;
> +            global_b = 1;
> +        }
> +
> +        if (global_a == global_b) {
> +            errors++;
> +        }
> +        UNLOCK(&global_lock);
> +    }
> +
> +    printf("CPU%d: Done - Errors: %d\n", cpu, errors);
> +}
> +
>  void main(void)
>  {
> -    printf("CPU %d on\n", get_cpuid());
> +    if (!get_cpuid()) {
> +            printf("Starting test\n");
> +    }
> +
> +    test_spinlock();
>      power_off();
>  }

You could have saved a ton of time by just putting these 50
lines into a new kvm-unit-tests test. If you need the mmu
disabled for some reason, then we can add an mmu_disable()
to the API.

drew